From 3548ab5ebb9b729f8399693488d537e37688b0ef Mon Sep 17 00:00:00 2001 From: Anthony Martin Date: Fri, 6 Sep 2013 15:47:39 -0400 Subject: [PATCH] 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 --- src/pkg/runtime/time.goc | 29 ++++++++++++++++ src/pkg/time/internal_test.go | 63 +++++++++++++++++++++++++++++++++++ src/pkg/time/sleep_test.go | 6 ++++ 3 files changed, 98 insertions(+) diff --git a/src/pkg/runtime/time.goc b/src/pkg/runtime/time.goc index 1101ad068a0..b575696f715 100644 --- a/src/pkg/runtime/time.goc +++ b/src/pkg/runtime/time.goc @@ -13,8 +13,13 @@ package time #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 @@ addtimer(Timer *t) 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 @@ addtimer(Timer *t) 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 @@ runtime·deltimer(Timer *t) siftup(i); siftdown(i); } + if(debug) + dumptimers("deltimer"); runtime·unlock(&timers); return true; } @@ -285,3 +299,18 @@ siftdown(int32 i) 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 918a9f33be6..4e5557d6a0b 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 @@ func init() { 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 d21b9cca44a..46872595094 100644 --- a/src/pkg/time/sleep_test.go +++ b/src/pkg/time/sleep_test.go @@ -396,3 +396,9 @@ func TestIssue5745(t *testing.T) { timer.Stop() t.Error("Should be unreachable.") } + +func TestOverflowRuntimeTimer(t *testing.T) { + if err := CheckRuntimeTimerOverflow(); err != nil { + t.Fatalf(err.Error()) + } +}