ssh: fix spinloop in mux SendRequest drain on closed channel

The drain loop added in 4e7a738 ("ssh: fix deadlock on unexpected
global responses") receives from m.globalResponses without checking
the comma-ok flag. Once mux.loop closes globalResponses on connection
teardown, the loop spins forever on the zero value, pinning a CPU
core for every concurrent SendRequest caller.

Use the comma-ok idiom to detect the closed channel and break out.

Change-Id: Icf0a7419fe5b206b7a4f70429fa9f25d0b5af731
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/783681
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Neal Patel <nealpatel@google.com>
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Neal Patel <neal@golang.org>
Reviewed-by: Junyang Shao <shaojunyang@google.com>

ssh/mux.go

@@ -155,7 +155,10 @@ func (m *mux) SendRequest(name string, wantReply bool, payload []byte) (bool, []
 	drain:
 		for {
 			select {
-			case <-m.globalResponses:
+			case _, ok := <-m.globalResponses:
+				if !ok {
+					break drain
+				}
 			default:
 				break drain
 			}

ssh/mux_test.go

@@ -1405,3 +1405,41 @@ closed:
 		t.Fatal("SendRequest spinloop on closed ch.msg")
 	}
 }
+
+func TestMuxSendRequestAfterCloseNoSpinloop(t *testing.T) {
+	a, b := muxPair()
+	defer a.Close()
+
+	// Closing the underlying conn causes mux.loop to exit and close
+	// b.globalResponses.
+	if err := b.Close(); err != nil {
+		t.Fatalf("Close: %v", err)
+	}
+	// Wait for globalResponses to actually be closed by loop().
+	for {
+		select {
+		case _, ok := <-b.globalResponses:
+			if !ok {
+				goto closed
+			}
+		case <-time.After(2 * time.Second):
+			t.Fatal("globalResponses was not closed")
+		}
+	}
+closed:
+
+	done := make(chan error, 1)
+	go func() {
+		_, _, err := b.SendRequest("ping", true, nil)
+		done <- err
+	}()
+
+	select {
+	case err := <-done:
+		if err == nil {
+			t.Fatal("SendRequest after close: got nil error")
+		}
+	case <-time.After(2 * time.Second):
+		t.Fatal("SendRequest spinloop on closed globalResponses")
+	}
+}