]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: handle timer overflow in tsleep
authorAnthony Martin <ality@pbrane.org>
Fri, 6 Sep 2013 19:47:39 +0000 (15:47 -0400)
committerRuss Cox <rsc@golang.org>
Fri, 6 Sep 2013 19:47:39 +0000 (15:47 -0400)
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
src/pkg/time/internal_test.go
src/pkg/time/sleep_test.go

index 1101ad068a0ba235c61f852d00880cabf4b13e1c..b575696f715c5e7e7e44e2abd417d3b68047e295 100644 (file)
@@ -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");
+}
index 918a9f33be6f8629088337e9099876be2263aa70..4e5557d6a0bd6288c286b83825a42759bb668bb5 100644 (file)
@@ -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()
+               }
+       }
+}
index d21b9cca44ad3d9eafee179d213fd4903e33c3d1..46872595094e97a4d06a8b93c6a63ab751932005 100644 (file)
@@ -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())
+       }
+}