diff options
author | Jamie Liu <jamieliu@google.com> | 2021-02-24 18:14:32 -0800 |
---|---|---|
committer | gVisor bot <gvisor-bot@google.com> | 2021-02-24 18:16:51 -0800 |
commit | 0462dfe9f8bfbe482912caeb2bd636ec005b2b9e (patch) | |
tree | c768b59f8f99a11d3e0585337eea1a608d5d355e | |
parent | 1d2975ffbe0e32ebcd2fe9307544799b2f9ae632 (diff) |
Use sync.Gate in p9.connState.
sync.WaitGroup.Add(positive delta) is illegal if the WaitGroup counter is zero
and WaitGroup.Wait() may be called concurrently. This is problematic for
p9.connState.pendingWg, which counts inflight requests (so transitions from
zero are normal) and is waited-upon when receiving from the underlying Unix
domain socket returns an error, e.g. during connection shutdown. (Even if the
socket has been closed, new requests can still be concurrently received via
flipcall channels.)
PiperOrigin-RevId: 359416057
-rw-r--r-- | pkg/p9/server.go | 21 |
1 files changed, 13 insertions, 8 deletions
diff --git a/pkg/p9/server.go b/pkg/p9/server.go index 8c5c434fd..290d5b9ce 100644 --- a/pkg/p9/server.go +++ b/pkg/p9/server.go @@ -81,8 +81,8 @@ type connState struct { // version 0 implies 9P2000.L. version uint32 - // pendingWg counts requests that are still being handled. - pendingWg sync.WaitGroup + // reqGate counts requests that are still being handled. + reqGate sync.Gate // -- below relates to the legacy handler -- @@ -481,9 +481,13 @@ func (cs *connState) lookupChannel(id uint32) *channel { // handle handles a single message. func (cs *connState) handle(m message) (r message) { - cs.pendingWg.Add(1) + if !cs.reqGate.Enter() { + // connState.stop() has been called; the connection is shutting down. + r = newErr(syscall.ECONNRESET) + return + } defer func() { - cs.pendingWg.Done() + cs.reqGate.Leave() if r == nil { // Don't allow a panic to propagate. err := recover() @@ -594,10 +598,11 @@ func (cs *connState) handleRequests() { } func (cs *connState) stop() { - // Wait for completion of all inflight requests. This is mostly so that if - // a request is stuck, the sandbox supervisor has the opportunity to kill - // us with SIGABRT to get a stack dump of the offending handler. - cs.pendingWg.Wait() + // Stop new requests from proceeding, and wait for completion of all + // inflight requests. This is mostly so that if a request is stuck, the + // sandbox supervisor has the opportunity to kill us with SIGABRT to get a + // stack dump of the offending handler. + cs.reqGate.Close() // Free the channels. cs.channelMu.Lock() |