Don't block SSH keep alive probes when a period keep alive is in progress

psiphon/tunnel.go

@@ -1071,20 +1071,39 @@ func (tunnel *Tunnel) operateTunnel(tunnelOwner TunnelOwner) {
 	}()
 
 	requestsWaitGroup.Add(1)
-	signalSshKeepAlive := make(chan time.Duration)
+	signalPeriodicSshKeepAlive := make(chan time.Duration)
 	sshKeepAliveError := make(chan error, 1)
 	go func() {
 		defer requestsWaitGroup.Done()
-		isFirstKeepAlive := true
-		for timeout := range signalSshKeepAlive {
-			err := tunnel.sendSshKeepAlive(isFirstKeepAlive, timeout)
+		isFirstPeriodicKeepAlive := true
+		for timeout := range signalPeriodicSshKeepAlive {
+			err := tunnel.sendSshKeepAlive(isFirstPeriodicKeepAlive, timeout)
+			if err != nil {
+				select {
+				case sshKeepAliveError <- err:
+				default:
+				}
+			}
+			isFirstPeriodicKeepAlive = false
+		}
+	}()
+
+	// Probe-type SSH keep alives have a distinct send worker and may be sent
+	// concurrently, to ensure a long period keep alive timeout doesn't delay
+	// failed tunnel detection.
+
+	requestsWaitGroup.Add(1)
+	signalProbeSshKeepAlive := make(chan time.Duration)
+	go func() {
+		defer requestsWaitGroup.Done()
+		for timeout := range signalProbeSshKeepAlive {
+			err := tunnel.sendSshKeepAlive(false, timeout)
 			if err != nil {
 				select {
 				case sshKeepAliveError <- err:
 				default:
 				}
 			}
-			isFirstKeepAlive = false
 		}
 	}()
 
@@ -1152,7 +1171,7 @@ func (tunnel *Tunnel) operateTunnel(tunnelOwner TunnelOwner) {
 			if lastBytesReceivedTime.Add(inactivePeriod).Before(time.Now()) {
 				timeout := p.Duration(parameters.SSHKeepAlivePeriodicTimeout)
 				select {
-				case signalSshKeepAlive <- timeout:
+				case signalPeriodicSshKeepAlive <- timeout:
 				default:
 				}
 			}
@@ -1177,7 +1196,7 @@ func (tunnel *Tunnel) operateTunnel(tunnelOwner TunnelOwner) {
 				if lastBytesReceivedTime.Add(inactivePeriod).Before(time.Now()) {
 					timeout := p.Duration(parameters.SSHKeepAliveProbeTimeout)
 					select {
-					case signalSshKeepAlive <- timeout:
+					case signalProbeSshKeepAlive <- timeout:
 					default:
 					}
 				}
@@ -1206,7 +1225,8 @@ func (tunnel *Tunnel) operateTunnel(tunnelOwner TunnelOwner) {
 		}
 	}
 
-	close(signalSshKeepAlive)
+	close(signalPeriodicSshKeepAlive)
+	close(signalProbeSshKeepAlive)
 	close(signalStatusRequest)
 	requestsWaitGroup.Wait()
 
@@ -1241,7 +1261,7 @@ func (tunnel *Tunnel) operateTunnel(tunnelOwner TunnelOwner) {
 // on the specified SSH connections and returns true of the request succeeds
 // within a specified timeout. If the request fails, the associated conn is
 // closed, which will terminate the associated tunnel.
-func (tunnel *Tunnel) sendSshKeepAlive(isFirstKeepAlive bool, timeout time.Duration) error {
+func (tunnel *Tunnel) sendSshKeepAlive(isFirstPeriodicKeepAlive bool, timeout time.Duration) error {
 
 	// Note: there is no request context since SSH requests cannot be
 	// interrupted directly. Closing the tunnel will interrupt the request.
@@ -1277,14 +1297,14 @@ func (tunnel *Tunnel) sendSshKeepAlive(isFirstKeepAlive bool, timeout time.Durat
 		errChannel <- err
 
 		// Record the keep alive round trip as a speed test sample. The first
-		// keep alive is always recorded, as many tunnels are short-lived and
-		// we want to ensure that some data is gathered. Subsequent keep
-		// alives are recorded with some configurable probability, which,
-		// considering that only the last SpeedTestMaxSampleCount samples are
-		// retained, enables tuning the sampling frequency.
+		// periodic keep alive is always recorded, as many tunnels are short-lived
+		// and we want to ensure that some data is gathered. Subsequent keep alives
+		// are recorded with some configurable probability, which, considering that
+		// only the last SpeedTestMaxSampleCount samples are retained, enables
+		// tuning the sampling frequency.
 
 		if err == nil && requestOk &&
-			(isFirstKeepAlive ||
+			(isFirstPeriodicKeepAlive ||
 				tunnel.getCustomClientParameters().WeightedCoinFlip(
 					parameters.SSHKeepAliveSpeedTestSampleProbability)) {