Skip to content

Commit

Permalink
internal/poll: avoid race between SetDeadline and timer expiry in Plan 9
Browse files Browse the repository at this point in the history
The mutexes added by CL 235820 aren't sufficient to prevent a race when
an i/o deadline timer fires just as the deadline is being reset to zero.

Consider this possible sequence when goroutine S is clearing the
deadline and goroutine T has been started by the timer:

1. S locks the mutex
2. T blocks on the mutex
3. S sets the timedout flag to false
4. S calls Stop on the timer (and fails, because the timer has fired)
5. S unlocks the mutex
6. T locks the mutex
7. T sets the timedout flag to true

Now all subsequent I/O will timeout, although the deadline has been
cleared.

The fix is for the timeout goroutine to skip setting the timedout
flag if the timer pointer has been cleared, or reassigned by
another SetDeadline operation.

Fixes #57114

Change-Id: I4a45d19c3b4b66cdf151dcc3f70536deaa8216a7
Reviewed-on: https://go-review.googlesource.com/c/go/+/470215
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David du Colombier <0intro@gmail.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
  • Loading branch information
millerresearch authored and Bryan Mills committed Feb 27, 2023
1 parent e153905 commit 8538477
Showing 1 changed file with 25 additions and 19 deletions.
44 changes: 25 additions & 19 deletions src/internal/poll/fd_plan9.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,48 +122,54 @@ func setDeadlineImpl(fd *FD, t time.Time, mode int) error {
if mode == 'r' || mode == 'r'+'w' {
fd.rmu.Lock()
defer fd.rmu.Unlock()
if fd.rtimer != nil {
fd.rtimer.Stop()
fd.rtimer = nil
}
fd.rtimedout.Store(false)
}
if mode == 'w' || mode == 'r'+'w' {
fd.wmu.Lock()
defer fd.wmu.Unlock()
fd.wtimedout.Store(false)
}
if t.IsZero() || d < 0 {
// Stop timer
if mode == 'r' || mode == 'r'+'w' {
if fd.rtimer != nil {
fd.rtimer.Stop()
}
fd.rtimer = nil
}
if mode == 'w' || mode == 'r'+'w' {
if fd.wtimer != nil {
fd.wtimer.Stop()
}
if fd.wtimer != nil {
fd.wtimer.Stop()
fd.wtimer = nil
}
} else {
fd.wtimedout.Store(false)
}
if !t.IsZero() && d > 0 {
// Interrupt I/O operation once timer has expired
if mode == 'r' || mode == 'r'+'w' {
fd.rtimer = time.AfterFunc(d, func() {
var timer *time.Timer
timer = time.AfterFunc(d, func() {
fd.rmu.Lock()
defer fd.rmu.Unlock()
if fd.rtimer != timer {
// deadline was changed
return
}
fd.rtimedout.Store(true)
if fd.raio != nil {
fd.raio.Cancel()
}
fd.rmu.Unlock()
})
fd.rtimer = timer
}
if mode == 'w' || mode == 'r'+'w' {
fd.wtimer = time.AfterFunc(d, func() {
var timer *time.Timer
timer = time.AfterFunc(d, func() {
fd.wmu.Lock()
defer fd.wmu.Unlock()
if fd.wtimer != timer {
// deadline was changed
return
}
fd.wtimedout.Store(true)
if fd.waio != nil {
fd.waio.Cancel()
}
fd.wmu.Unlock()
})
fd.wtimer = timer
}
}
if !t.IsZero() && d < 0 {
Expand Down

0 comments on commit 8538477

Please sign in to comment.