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
This commit is contained in:
Ghanan Gowripalan
2020-04-20 16:32:44 -07:00
committed by gVisor bot
parent 1a597e01be
commit 782041509f
2 changed files with 31 additions and 2 deletions
+6 -2
View File
@@ -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
+25
View File
@@ -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()