From 782041509f4130e8e795b22379368239d5091c8f Mon Sep 17 00:00:00 2001 From: Ghanan Gowripalan Date: Mon, 20 Apr 2020 16:31:39 -0700 Subject: 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 --- pkg/tcpip/timer.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) (limited to 'pkg/tcpip/timer.go') 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 -- cgit v1.2.3