Merge pull request #72 from rod-hynes/master

Fix shutdown performance issues

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

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))

psiphon/socksProxy.go

@@ -77,7 +77,7 @@ func (proxy *SocksProxy) socksConnectionHandler(localConn *socks.SocksConn) (err
 	defer localConn.Close()
 	defer proxy.openConns.Remove(localConn)
 	proxy.openConns.Add(localConn)
-	// Setting peerConn so localConn.Close() will be called when remoteConn.Close() is called.
+	// Using downstreamConn so localConn.Close() will be called when remoteConn.Close() is called.
 	// This ensures that the downstream client (e.g., web browser) doesn't keep waiting on the
 	// open connection for data which will never arrive.
 	remoteConn, err := proxy.tunneler.Dial(localConn.Req.Target, localConn)

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