internal/jsonrpc2_v2: fix a potential deadlock when (*Conn).Close is invoked during Bind

This fixes the goroutine leak reported in
https://build.golang.org/log/ae36d36843ca240e9e080886417a8798dd4c9618.

Fixes golang/go#46047 (hopefully for real this time).

Change-Id: I360e54d819849a35284c61d3a0655cc175d81f77
Reviewed-on: https://go-review.googlesource.com/c/tools/+/448095
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Reviewed-by: Alan Donovan <adonovan@google.com>
gopls-CI: kokoro <noreply+kokoro@google.com>
This commit is contained in:
Bryan C. Mills 2022-11-04 16:37:35 -04:00 committed by Bryan Mills
parent 3057465037
commit d41a43b94f
1 changed files with 14 additions and 5 deletions

View File

@ -82,6 +82,7 @@ type Connection struct {
// Connection.
type inFlightState struct {
connClosing bool // true when the Connection's Close method has been called
reading bool // true while the readIncoming goroutine is running
readErr error // non-nil when the readIncoming goroutine exits (typically io.EOF)
writeErr error // non-nil if a call to the Writer has failed with a non-canceled Context
@ -140,14 +141,13 @@ func (c *Connection) updateInFlight(f func(*inFlightState)) {
s.closeErr = s.closer.Close()
s.closer = nil // prevent duplicate Close calls
}
if s.readErr == nil {
if s.reading {
// The readIncoming goroutine is still running. Our call to Close should
// cause it to exit soon, at which point it will make another call to
// updateInFlight, set s.readErr to a non-nil error, and mark the
// Connection done.
// updateInFlight, set s.reading to false, and mark the Connection done.
} else {
// The readIncoming goroutine has exited. Since everything else is idle,
// we're completely done.
// The readIncoming goroutine has exited, or never started to begin with.
// Since everything else is idle, we're completely done.
if c.onDone != nil {
c.onDone()
}
@ -240,10 +240,18 @@ func newConnection(bindCtx context.Context, rwc io.ReadWriteCloser, binder Binde
reader := framer.Reader(rwc)
c.updateInFlight(func(s *inFlightState) {
select {
case <-c.done:
// Bind already closed the connection; don't start a goroutine to read it.
return
default:
}
// The goroutine started here will continue until the underlying stream is closed.
//
// (If the Binder closed the Connection already, this should error out and
// return almost immediately.)
s.reading = true
go c.readIncoming(ctx, reader, options.Preempter)
})
return c
@ -514,6 +522,7 @@ func (c *Connection) readIncoming(ctx context.Context, reader Reader, preempter
}
c.updateInFlight(func(s *inFlightState) {
s.reading = false
s.readErr = err
// Retire any outgoing requests that were still in flight: with the Reader no