[release-branch.go1.1] time: prevent a panic from leaving the timer mutex held

««« CL 10373047 / 974a69ed9fcf
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

»»»

Update #5928

R=golang-dev, dave
CC=golang-dev
https://golang.org/cl/11666046

2 files changed, 25 insertions, 0 deletions

diff --git a/src/pkg/runtime/time.goc b/src/pkg/runtime/time.goc
index 6de989f515..be0c1f83d4 100644
--- 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 1322f06114..603adc9b89 100644
--- 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.")
+}