From: Jeff R. Allen Date: Tue, 2 Jul 2013 01:42:29 +0000 (-0400) Subject: time: prevent a panic from leaving the timer mutex held X-Git-Tag: go1.2rc2~1124 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=0286b4738e33c5a043d454b23af88fb95127bf13;p=gostls13.git 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 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.") +}