time: prevent a panic from leaving the timer mutex held

When deleting a timer, a panic due to nil deref
would leave a lock held, possibly leading to a deadlock
in a defer. Instead return false on a nil timer.

Fixes #5745.

R=golang-dev, daniel.morsing, dvyukov, rsc, iant
CC=golang-dev
https://golang.org/cl/10373047

diff --git a/src/pkg/runtime/time.goc b/src/pkg/runtime/time.goc
index 6de989f5158c4d15d99bf9645657170c5add61bb..be0c1f83d4c6860c7c49a779fa1897f9bafc4d25 100644 (file)
--- a/src/pkg/runtime/time.goc
+++ b/src/pkg/runtime/time.goc
@@ -131,6 +131,11 @@ runtime·deltimer(Timer *t)
 {
        int32 i;
 
+       // Dereference t so that any panic happens before the lock is held.
+       // Discard result, because t might be moving in the heap.
+       i = t->i;
+       USED(i);
+
        runtime·lock(&timers);
 
        // t may not be registered anymore and may have

diff --git a/src/pkg/time/sleep_test.go b/src/pkg/time/sleep_test.go
index 1322f061149f577611c54c16a79e9403372d109a..603adc9b895894eea9b2319f8e7d7816647a467d 100644 (file)
--- a/src/pkg/time/sleep_test.go
+++ b/src/pkg/time/sleep_test.go
@@ -314,3 +314,23 @@ func TestOverflowSleep(t *testing.T) {
                t.Fatalf("negative timeout didn't fire")
        }
 }
+
+// Test that a panic while deleting a timer does not leave
+// the timers mutex held, deadlocking a ticker.Stop in a defer.
+func TestIssue5745(t *testing.T) {
+       ticker := NewTicker(Hour)
+       defer func() {
+               // would deadlock here before the fix due to
+               // lock taken before the segfault.
+               ticker.Stop()
+
+               if r := recover(); r == nil {
+                       t.Error("Expected panic, but none happened.")
+               }
+       }()
+
+       // cause a panic due to a segfault
+       var timer *Timer
+       timer.Stop()
+       t.Error("Should be unreachable.")
+}