Browse Source

Fix shutdown performance issues

* Don't close tunnels in sequence, close in parallel

* Enforce short timeout for operateTunnel shutdown (in
  particular, final status request)
Rod Hynes 11 years ago
parent
commit
182846c5ec
3 changed files with 23 additions and 1 deletions
  1. 1 0
      psiphon/config.go
  2. 11 1
      psiphon/controller.go
  3. 11 0
      psiphon/tunnel.go

+ 1 - 0
psiphon/config.go

@@ -36,6 +36,7 @@ const (
 	TUNNEL_CONNECT_TIMEOUT                       = 15 * time.Second
 	TUNNEL_READ_TIMEOUT                          = 0 * time.Second
 	TUNNEL_WRITE_TIMEOUT                         = 5 * time.Second
+	TUNNEL_OPERATE_SHUTDOWN_TIMEOUT              = 2 * time.Second
 	TUNNEL_SSH_KEEP_ALIVE_PAYLOAD_MAX_BYTES      = 256
 	TUNNEL_SSH_KEEP_ALIVE_PERIOD_MIN             = 60 * time.Second
 	TUNNEL_SSH_KEEP_ALIVE_PERIOD_MAX             = 120 * time.Second

+ 11 - 1
psiphon/controller.go

@@ -433,9 +433,19 @@ func (controller *Controller) terminateTunnel(tunnel *Tunnel) {
 func (controller *Controller) terminateAllTunnels() {
 	controller.tunnelMutex.Lock()
 	defer controller.tunnelMutex.Unlock()
+	// Closing all tunnels in parallel. In an orderly shutdown, each tunnel
+	// may take a few seconds to send a final status request. We only want
+	// to wait as long as the single slowest tunnel.
+	closeWaitGroup := new(sync.WaitGroup)
+	closeWaitGroup.Add(len(controller.tunnels))
 	for _, activeTunnel := range controller.tunnels {
-		activeTunnel.Close()
+		tunnel := activeTunnel
+		go func() {
+			defer closeWaitGroup.Done()
+			tunnel.Close()
+		}()
 	}
+	closeWaitGroup.Wait()
 	controller.tunnels = make([]*Tunnel, 0)
 	controller.nextTunnel = 0
 	NoticeTunnels(len(controller.tunnels))

+ 11 - 0
psiphon/tunnel.go

@@ -172,8 +172,19 @@ func EstablishTunnel(
 func (tunnel *Tunnel) Close() {
 	tunnel.mutex.Lock()
 	if !tunnel.isClosed {
+		// Signal operateTunnel to stop before closing the tunnel -- this
+		// allows a final status request to be made in the case of an orderly
+		// shutdown.
+		// A timer is set, so if operateTunnel takes too long to stop, the
+		// tunnel is closed, which will interrupt any slow final status request.
+		// In effect, the TUNNEL_OPERATE_SHUTDOWN_TIMEOUT value will take
+		// precedence over the PSIPHON_API_SERVER_TIMEOUT http.Client.Timeout
+		// value set in makePsiphonHttpsClient.
+		timer := time.AfterFunc(TUNNEL_OPERATE_SHUTDOWN_TIMEOUT, func() { tunnel.conn.Close() })
 		close(tunnel.shutdownOperateBroadcast)
 		tunnel.operateWaitGroup.Wait()
+		timer.Stop()
+		// tunnel.conn.Close() may get called twice, which is allowed.
 		tunnel.conn.Close()
 	}
 	tunnel.isClosed = true