Skip to content

Commit

Permalink
fix: remove data race when resetting ticker
Browse files Browse the repository at this point in the history
Currently there is a data race in the Ticker that is triggered when
Ticker.Reset and Ticker.IsDurationReached are called concurrently.
IsDurationReached is called by the clock as part of Clock.SetTimestamp and
Ticker.Reset is called by user-code.

This commit fixes the data race by protecting `duration` field of Ticker
with the mutex. This also forces inlining of some calls in both methods
as the called methods will also attempt to lock the mutex.
  • Loading branch information
ericwenn committed Oct 23, 2023
1 parent e1212cb commit dbd5807
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 2 deletions.
31 changes: 31 additions & 0 deletions externalclock/clock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,37 @@ func TestExternalClock_TestLooper(t *testing.T) {
assert.Equal(t, looper.Target, looper.counter)
}

func TestTicker_ResetRace(t *testing.T) {
// This test does not have any assertions, and instead is intended to catch
// data races with `-race` flag set.
const (
ticks = 100
tickInterval = time.Millisecond
)
clock := newTestFixture(t)
done := make(chan struct{}, 2)
go func() {
initialTime := time.Unix(0, 0)
for i := 0; i < ticks; i++ {
clock.SetTimestamp(initialTime.Add(time.Second))
time.Sleep(tickInterval)
}
done <- struct{}{}
}()
go func() {
ticker := clock.NewTicker(time.Second)
for i := 0; i < ticks; i++ {
ticker.Reset(time.Second)
time.Sleep(tickInterval)
}
ticker.Stop()
done <- struct{}{}
}()

<-done
<-done
}

func TestExternalClock_TestLooper_AddTicker(t *testing.T) {
externalClock := newTestFixture(t)
ctx, cancel := context.WithCancel(context.Background())
Expand Down
10 changes: 8 additions & 2 deletions externalclock/ticker.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,18 @@ func (t *ticker) Stop() {

func (t *ticker) Reset(duration time.Duration) {
now := t.getTimeFunc()
t.mutex.Lock()
t.duration = duration
t.SetLastTimestamp(now)
t.lastTimeStamp = now
t.mutex.Unlock()
}

func (t *ticker) IsDurationReached(currentTime time.Time) bool {
return t.duration <= currentTime.Sub(t.GetLastTimestamp())
t.mutex.Lock()
dur := t.duration
ts := t.lastTimeStamp
t.mutex.Unlock()
return dur <= currentTime.Sub(ts)
}

func (t *ticker) GetLastTimestamp() time.Time {
Expand Down

0 comments on commit dbd5807

Please sign in to comment.