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 <emm.odeke@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

diff --git a/src/runtime/time.go b/src/runtime/time.go
index b9454d6e2b9f7bd1791098256d07d752d147bd97..0e1763e0cd0e698912a7b6ba96239b009ec34921 100644 (file)
--- 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 9e40eb43740c796e89558cf6f1eaa06f96802622..dd17aab1b15364d53f91d25693938669156bf605 100644 (file)
--- 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