ssh: fix spinloop in channel SendRequest drain on closed channel
The drain loop added in 3c7c869 ("ssh: fix deadlock on unexpected
channel responses") receives from ch.msg without checking the comma-ok
flag. Once ch.msg is closed by channel.close(), receives succeed
immediately with the zero value, so the default arm is never taken
and the loop spins forever, pinning a CPU core for every concurrent
SendRequest caller on a torn-down channel.
Use the comma-ok idiom to detect the closed channel and break out.
Fixes golang/go#79658
Change-Id: Id80aa5d8429550c7a68e8759aed9ffba3aabb006
Reviewed-on: https://go-review.googlesource.com/c/crypto/+/783680
LUCI-TryBot-Result: golang-scoped@luci-project-accounts.iam.gserviceaccount.com <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Junyang Shao <shaojunyang@google.com>
Reviewed-by: Neal Patel <neal@golang.org>
Auto-Submit: Nicola Murino <nicola.murino@gmail.com>
Reviewed-by: Neal Patel <nealpatel@google.com>
This commit is contained in:
committed by
Gopher Robot
parent
5adb68b3e6
commit
e3e62d9601
+4
-1
@@ -634,7 +634,10 @@ func (ch *channel) SendRequest(name string, wantReply bool, payload []byte) (boo
|
||||
drain:
|
||||
for {
|
||||
select {
|
||||
case <-ch.msg:
|
||||
case _, ok := <-ch.msg:
|
||||
if !ok {
|
||||
break drain
|
||||
}
|
||||
default:
|
||||
break drain
|
||||
}
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"io"
|
||||
"sync"
|
||||
"testing"
|
||||
"time"
|
||||
)
|
||||
|
||||
func muxPair() (*mux, *mux) {
|
||||
@@ -1366,3 +1367,41 @@ func TestChannelResponseAcceptedWhilePending(t *testing.T) {
|
||||
writer.Close()
|
||||
<-serverDone
|
||||
}
|
||||
|
||||
func TestChannelSendRequestAfterCloseNoSpinloop(t *testing.T) {
|
||||
writer, reader, mux := channelPair(t)
|
||||
defer writer.Close()
|
||||
defer mux.Close()
|
||||
|
||||
// Close the channel, this closes reader.msg.
|
||||
if err := reader.Close(); err != nil {
|
||||
t.Fatalf("Close: %v", err)
|
||||
}
|
||||
// Wait for reader.msg to actually be closed by the mux loop.
|
||||
for {
|
||||
select {
|
||||
case _, ok := <-reader.msg:
|
||||
if !ok {
|
||||
goto closed
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("reader.msg was not closed")
|
||||
}
|
||||
}
|
||||
closed:
|
||||
|
||||
done := make(chan error, 1)
|
||||
go func() {
|
||||
_, err := reader.SendRequest("ping", true, nil)
|
||||
done <- err
|
||||
}()
|
||||
|
||||
select {
|
||||
case err := <-done:
|
||||
if !errors.Is(err, io.EOF) {
|
||||
t.Fatalf("SendRequest after close: got %v, want io.EOF", err)
|
||||
}
|
||||
case <-time.After(2 * time.Second):
|
||||
t.Fatal("SendRequest spinloop on closed ch.msg")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user