diff options
| author | Ghanan Gowripalan <ghanan@google.com> | 2020-04-20 16:31:39 -0700 |
|---|---|---|
| committer | gVisor bot <gvisor-bot@google.com> | 2020-04-20 16:32:44 -0700 |
| commit | 782041509f4130e8e795b22379368239d5091c8f (patch) | |
| tree | 81ed7df40421e1bce4f5c381ce91b79b5362ff11 /pkg/tcpip/timer.go | |
| parent | 1a597e01bed5d5fb30b3d444e0a23669c5587235 (diff) | |
Prevent race when reassigning CancellableTimer
Capture a timer's locker for each instance of a CancellableTimer so that
reassigning a tcpip.CancellableTimer does not cause a data race.
Reassigning a tcpip.CancellableTimer updates its underlying locker. When
a timer fires, it does a read of the timer's locker variable to lock it.
This read of the locker was not synchronized so a race existed where one
goroutine may reassign the timer (updating the locker) and another
handles the timer firing (attempts to lock the timer's locker).
Test: tcpip_test.TestCancellableTimerReassignment
PiperOrigin-RevId: 307499822
Diffstat (limited to 'pkg/tcpip/timer.go')
| -rw-r--r-- | pkg/tcpip/timer.go | 8 |
1 files changed, 6 insertions, 2 deletions
diff --git a/pkg/tcpip/timer.go b/pkg/tcpip/timer.go index f5f01f32f..67f66fc72 100644 --- a/pkg/tcpip/timer.go +++ b/pkg/tcpip/timer.go @@ -131,10 +131,14 @@ func (t *CancellableTimer) StopLocked() { func (t *CancellableTimer) Reset(d time.Duration) { // Create a new instance. earlyReturn := false + + // Capture the locker so that updating the timer does not cause a data race + // when a timer fires and tries to obtain the lock (read the timer's locker). + locker := t.locker t.instance = cancellableTimerInstance{ timer: time.AfterFunc(d, func() { - t.locker.Lock() - defer t.locker.Unlock() + locker.Lock() + defer locker.Unlock() if earlyReturn { // If we reach this point, it means that the timer fired while another |