time.Ticker: fix bug arising when all tickers are dead.

thanks to yglgogo for analysis.

Fixes issue 593.

R=rsc
CC=golang-dev
http://codereview.appspot.com/210044

2 files changed, 24 insertions, 6 deletions

diff --git a/src/pkg/time/tick.go b/src/pkg/time/tick.go
index bbbc46961..885a290bc 100644
--- a/src/pkg/time/tick.go
+++ b/src/pkg/time/tick.go
@@ -39,16 +39,23 @@ type alarmer struct {
 
 // Set alarm to go off at time ns, if not already set earlier.
 func (a *alarmer) set(ns int64) {
-	// If there's no wakeLoop or the next tick we expect is too late, start a new wakeLoop
-	if a.wakeMeAt == nil || a.wakeTime > ns {
-		// Stop previous wakeLoop.
-		if a.wakeMeAt != nil {
-			a.wakeMeAt <- -1
-		}
+	switch {
+	case a.wakeTime > ns:
+		// Next tick we expect is too late; shut down the late runner
+		// and (after fallthrough) start a new wakeLoop.
+		a.wakeMeAt <- -1
+		fallthrough
+	case a.wakeMeAt == nil:
+		// There's no wakeLoop, start one.
 		a.wakeMeAt = make(chan int64, 10)
 		go wakeLoop(a.wakeMeAt, a.wakeUp)
+		fallthrough
+	case a.wakeTime == 0:
+		// Nobody else is waiting; it's just us.
 		a.wakeTime = ns
 		a.wakeMeAt <- ns
+	default:
+		// There's already someone scheduled.
 	}
 }
 
@@ -141,6 +148,8 @@ func tickerLoop() {
 				// Please send wakeup at earliest required time.
 				// If there are no tickers, don't bother.
 				alarm.wakeMeAt <- wakeTime
+			} else {
+				alarm.wakeTime = 0
 			}
 		}
 		prevTime = now
diff --git a/src/pkg/time/tick_test.go b/src/pkg/time/tick_test.go
index e15793aea..d089a9b98 100644
--- a/src/pkg/time/tick_test.go
+++ b/src/pkg/time/tick_test.go
@@ -34,3 +34,12 @@ func TestTicker(t *testing.T) {
 		t.Fatalf("Ticker did not shut down")
 	}
 }
+
+// Test that a bug tearing down a ticker has been fixed.  This routine should not deadlock.
+func TestTeardown(t *testing.T) {
+	for i := 0; i < 3; i++ {
+		ticker := NewTicker(1e8)
+		<-ticker.C
+		ticker.Stop()
+	}
+}