]> Cypherpunks repositories - gostls13.git/commitdiff
time: avoid broken fix for buggy TestOverflowRuntimeTimer
authorRob Pike <r@golang.org>
Thu, 12 Jun 2014 18:44:55 +0000 (11:44 -0700)
committerRob Pike <r@golang.org>
Thu, 12 Jun 2014 18:44:55 +0000 (11:44 -0700)
The test requires that timerproc runs, but busy loops and starves
the scheduler so that, with high probability, timerproc doesn't run.
Avoid the issue by expecting the test to succeed; if not, a major
outside timeout will kill it and let us know.

As you can see from the diffs, there have been several attempts to
fix this with chicanery, but none has worked. Don't bother trying
any more.

Fixes #8136.

LGTM=rsc
R=rsc, josharian
CC=golang-codereviews
https://golang.org/cl/105140043

src/pkg/time/internal_test.go
src/pkg/time/sleep_test.go

index 2243d3668deaee97925971b977896d44b727a9a5..f09d30507fcadb170ad6f6ecfc860bee4387c7d8 100644 (file)
@@ -4,11 +4,6 @@
 
 package time
 
-import (
-       "errors"
-       "runtime"
-)
-
 func init() {
        // force US/Pacific for time zone tests
        ForceUSPacificForTesting()
@@ -24,7 +19,7 @@ func empty(now int64, arg interface{}) {}
 //
 // This test has to be in internal_test.go since it fiddles with
 // unexported data structures.
-func CheckRuntimeTimerOverflow() error {
+func CheckRuntimeTimerOverflow() {
        // We manually create a runtimeTimer to bypass the overflow
        // detection logic in NewTimer: we're testing the underlying
        // runtime.addtimer function.
@@ -35,17 +30,7 @@ func CheckRuntimeTimerOverflow() error {
        }
        startTimer(r)
 
-       timeout := 100 * Millisecond
-       switch runtime.GOOS {
-       // Allow more time for gobuilder to succeed.
-       case "windows":
-               timeout = Second
-       case "plan9":
-               // TODO(0intro): We don't know why it is needed.
-               timeout = 3 * Second
-       }
-
-       // Start a goroutine that should send on t.C before the timeout.
+       // Start a goroutine that should send on t.C right away.
        t := NewTimer(1)
 
        defer func() {
@@ -64,29 +49,11 @@ func CheckRuntimeTimerOverflow() error {
                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")
-                       }
-                       // Issue 6874. This test previously called runtime.Gosched to try to yield
-                       // to the goroutine servicing t, however the scheduler has a bias towards the
-                       // previously running goroutine in an idle system. Combined with high load due
-                       // to all CPUs busy running tests t's goroutine could be delayed beyond the
-                       // timeout window.
-                       //
-                       // Calling runtime.GC() reduces the worst case lantency for scheduling t by 20x
-                       // under the current Go 1.3 scheduler.
-                       runtime.GC()
-               }
-       }
+       // If the test fails, we will hang here until the timeout in the testing package
+       // fires, which is 10 minutes. It would be nice to catch the problem sooner,
+       // but there is no reliable way to guarantee that timerproc schedules without
+       // doing something involving timerproc itself. Previous failed attempts have
+       // tried calling runtime.Gosched and runtime.GC, but neither is reliable.
+       // So we fall back to hope: We hope we don't hang here.
+       <-t.C
 }
index 03f8e732c9e201e1467cba5e4b70dfa4b131cf40..d78490d4445fa97a56cf803585a28960540d50a5 100644 (file)
@@ -387,7 +387,7 @@ func TestOverflowRuntimeTimer(t *testing.T) {
        if testing.Short() {
                t.Skip("skipping in short mode, see issue 6874")
        }
-       if err := CheckRuntimeTimerOverflow(); err != nil {
-               t.Fatalf(err.Error())
-       }
+       // This may hang forever if timers are broken. See comment near
+       // the end of CheckRuntimeTimerOverflow in internal_test.go.
+       CheckRuntimeTimerOverflow()
 }