runtime: handle timer overflow in tsleep
Make sure we never pass a timer into timerproc with
a negative duration since it will cause other timers
to never expire.
Fixes #5321.
R=golang-dev, minux.ma, remyoudompheng, mikioh.mikioh, r, bradfitz, rsc, dvyukov
CC=golang-dev
https://golang.org/cl/9035047
diff --git a/src/pkg/runtime/time.goc b/src/pkg/runtime/time.goc
index 1101ad0..b575696 100644
--- a/src/pkg/runtime/time.goc
+++ b/src/pkg/runtime/time.goc
@@ -13,8 +13,13 @@
#include "malloc.h"
#include "race.h"
+enum {
+ debug = 0,
+};
+
static Timers timers;
static void addtimer(Timer*);
+static void dumptimers(int8*);
// Package time APIs.
// Godoc uses the comments in package time, not these.
@@ -92,6 +97,11 @@
int32 n;
Timer **nt;
+ // when must never be negative; otherwise timerproc will overflow
+ // during its delta calculation and never expire other timers.
+ if(t->when < 0)
+ t->when = (1LL<<63)-1;
+
if(timers.len >= timers.cap) {
// Grow slice.
n = 16;
@@ -121,6 +131,8 @@
timers.timerproc = runtime·newproc1(&timerprocv, nil, 0, 0, addtimer);
timers.timerproc->issystem = true;
}
+ if(debug)
+ dumptimers("addtimer");
}
// Delete timer t from the heap.
@@ -157,6 +169,8 @@
siftup(i);
siftdown(i);
}
+ if(debug)
+ dumptimers("deltimer");
runtime·unlock(&timers);
return true;
}
@@ -285,3 +299,18 @@
i = c;
}
}
+
+static void
+dumptimers(int8 *msg)
+{
+ Timer *t;
+ int32 i;
+
+ runtime·printf("timers: %s\n", msg);
+ for(i = 0; i < timers.len; i++) {
+ t = timers.t[i];
+ runtime·printf("\t%d\t%p:\ti %d when %D period %D fn %p\n",
+ i, t, t->i, t->when, t->period, t->fv->fn);
+ }
+ runtime·printf("\n");
+}
diff --git a/src/pkg/time/internal_test.go b/src/pkg/time/internal_test.go
index 918a9f3..4e5557d 100644
--- a/src/pkg/time/internal_test.go
+++ b/src/pkg/time/internal_test.go
@@ -4,6 +4,11 @@
package time
+import (
+ "errors"
+ "runtime"
+)
+
func init() {
// force US/Pacific for time zone tests
ForceUSPacificForTesting()
@@ -11,3 +16,61 @@
var Interrupt = interrupt
var DaysIn = daysIn
+
+func empty(now int64, arg interface{}) {}
+
+// Test that a runtimeTimer with a duration so large it overflows
+// does not cause other timers to hang.
+//
+// This test has to be in internal_test.go since it fiddles with
+// unexported data structures.
+func CheckRuntimeTimerOverflow() error {
+ // We manually create a runtimeTimer to bypass the overflow
+ // detection logic in NewTimer: we're testing the underlying
+ // runtime.addtimer function.
+ r := &runtimeTimer{
+ when: nano() + (1<<63 - 1),
+ f: empty,
+ arg: nil,
+ }
+ startTimer(r)
+
+ const timeout = 100 * Millisecond
+
+ // Start a goroutine that should send on t.C before the timeout.
+ t := NewTimer(1)
+
+ defer func() {
+ // Subsequent tests won't work correctly if we don't stop the
+ // overflow timer and kick the timer proc back into service.
+ //
+ // The timer proc is now sleeping and can only be awoken by
+ // adding a timer to the *beginning* of the heap. We can't
+ // wake it up by calling NewTimer since other tests may have
+ // left timers running that should have expired before ours.
+ // Instead we zero the overflow timer duration and start it
+ // once more.
+ stopTimer(r)
+ t.Stop()
+ r.when = 0
+ startTimer(r)
+ }()
+
+ // Try to receive from t.C before the timeout. It will succeed
+ // iff the previous sleep was able to finish. We're forced to
+ // spin and yield after trying to receive since we can't start
+ // any more timers (they might hang due to the same bug we're
+ // now testing).
+ stop := Now().Add(timeout)
+ for {
+ select {
+ case <-t.C:
+ return nil // It worked!
+ default:
+ if Now().After(stop) {
+ return errors.New("runtime timer stuck: overflow in addtimer")
+ }
+ runtime.Gosched()
+ }
+ }
+}
diff --git a/src/pkg/time/sleep_test.go b/src/pkg/time/sleep_test.go
index d21b9cc..4687259 100644
--- a/src/pkg/time/sleep_test.go
+++ b/src/pkg/time/sleep_test.go
@@ -396,3 +396,9 @@
timer.Stop()
t.Error("Should be unreachable.")
}
+
+func TestOverflowRuntimeTimer(t *testing.T) {
+ if err := CheckRuntimeTimerOverflow(); err != nil {
+ t.Fatalf(err.Error())
+ }
+}
