From a72e26f246a50b2d5226262420874b143d9d8b5a Mon Sep 17 00:00:00 2001 From: Emmanuel Odeke Date: Thu, 14 Sep 2017 19:12:26 -0600 Subject: [PATCH] runtime: return deltimer early if timer.timersBucket is unset Return early from deltimer, with false as the result, to indicate that we couldn't delete the timer since its timersBucket was nil(not set) in the first place. That happens in such a case where a user created the timer from a Ticker with: t := time.Ticker{C: c} The above usage skips the entire setup of assigning the appropriate underlying runtimeTimer and timersBucket, steps that are done for us by time.NewTicker. CL 34784 introduced this bug with an optimization, by changing stopTimer to retrieve the timersBucket from the timer itself (which is unset with the mentioned usage pattern above), whereas the old behavior relied on indexing by goroutine ID into the global slice of runtime timers, to retrieve the appropriate timersBucket. Fixes #21874 Change-Id: Ie9ccc6bdee685414b2430dc4aa74ef618cea2b33 Reviewed-on: https://go-review.googlesource.com/63970 Run-TryBot: Emmanuel Odeke TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- src/runtime/time.go | 9 +++++++++ src/time/tick_test.go | 7 +++++++ 2 files changed, 16 insertions(+) diff --git a/src/runtime/time.go b/src/runtime/time.go index b9454d6e2b..0e1763e0cd 100644 --- a/src/runtime/time.go +++ b/src/runtime/time.go @@ -163,6 +163,15 @@ func (tb *timersBucket) addtimerLocked(t *timer) { // Delete timer t from the heap. // Do not need to update the timerproc: if it wakes up early, no big deal. func deltimer(t *timer) bool { + if t.tb == nil { + // t.tb can be nil if the user created a timer + // directly, without invoking startTimer e.g + // time.Ticker{C: c} + // In this case, return early without any deletion. + // See Issue 21874. + return false + } + tb := t.tb lock(&tb.lock) diff --git a/src/time/tick_test.go b/src/time/tick_test.go index 9e40eb4374..dd17aab1b1 100644 --- a/src/time/tick_test.go +++ b/src/time/tick_test.go @@ -35,6 +35,13 @@ func TestTicker(t *testing.T) { } } +// Issue 21874 +func TestTickerStopWithDirectInitialization(t *testing.T) { + c := make(chan Time) + tk := &Ticker{C: c} + tk.Stop() +} + // Test that a bug tearing down a ticker has been fixed. This routine should not deadlock. func TestTeardown(t *testing.T) { Delta := 100 * Millisecond -- 2.48.1