diff options
| -rw-r--r-- | pkg/tcpip/timer.go | 8 | ||||
| -rw-r--r-- | pkg/tcpip/timer_test.go | 25 |
2 files changed, 31 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 diff --git a/pkg/tcpip/timer_test.go b/pkg/tcpip/timer_test.go index 2d20f7ef3..730134906 100644 --- a/pkg/tcpip/timer_test.go +++ b/pkg/tcpip/timer_test.go @@ -28,6 +28,31 @@ const ( longDuration = 1 * time.Second ) +func TestCancellableTimerReassignment(t *testing.T) { + var timer tcpip.CancellableTimer + var wg sync.WaitGroup + var lock sync.Mutex + + for i := 0; i < 2; i++ { + wg.Add(1) + + go func() { + lock.Lock() + // Assigning a new timer value updates the timer's locker and function. + // This test makes sure there is no data race when reassigning a timer + // that has an active timer (even if it has been stopped as a stopped + // timer may be blocked on a lock before it can check if it has been + // stopped while another goroutine holds the same lock). + timer = tcpip.MakeCancellableTimer(&lock, func() { + wg.Done() + }) + timer.Reset(shortDuration) + lock.Unlock() + }() + } + wg.Wait() +} + func TestCancellableTimerFire(t *testing.T) { t.Parallel() |