runtime: don't crash holding locks on racy timer access

If we run into data corruption due to the program accessing timers in
a racy way, do a normal panic rather than a hard crash with "panic
holding locks". The hope is to make the problem less confusing for users.

Fixes #25686

Change-Id: I863417adf21f7f8c088675b67a3acf49a0cdef41
Reviewed-on: https://go-review.googlesource.com/115815
Reviewed-by: Austin Clements <austin@google.com>

1 files changed, 37 insertions, 0 deletions

diff --git a/src/time/time_test.go b/src/time/time_test.go
index 7778bf1f83..432a67dec3 100644
--- a/src/time/time_test.go
+++ b/src/time/time_test.go
@@ -9,11 +9,13 @@ import (
 	"encoding/gob"
 	"encoding/json"
 	"fmt"
+	"internal/race"
 	"math/big"
 	"math/rand"
 	"os"
 	"runtime"
 	"strings"
+	"sync"
 	"testing"
 	"testing/quick"
 	. "time"
@@ -1341,3 +1343,38 @@ func TestReadFileLimit(t *testing.T) {
 		t.Errorf("readFile(%q) error = %v; want error containing 'is too large'", zero, err)
 	}
 }
+
+// Issue 25686: hard crash on concurrent timer access.
+// This test deliberately invokes a race condition.
+// We are testing that we don't crash with "fatal error: panic holding locks".
+func TestConcurrentTimerReset(t *testing.T) {
+	if race.Enabled {
+		t.Skip("skipping test under race detector")
+	}
+
+	// We expect this code to panic rather than crash.
+	// Don't worry if it doesn't panic.
+	catch := func(i int) {
+		if e := recover(); e != nil {
+			t.Logf("panic in goroutine %d, as expected, with %q", i, e)
+		} else {
+			t.Logf("no panic in goroutine %d", i)
+		}
+	}
+
+	const goroutines = 8
+	const tries = 1000
+	var wg sync.WaitGroup
+	wg.Add(goroutines)
+	timer := NewTimer(Hour)
+	for i := 0; i < goroutines; i++ {
+		go func(i int) {
+			defer wg.Done()
+			defer catch(i)
+			for j := 0; j < tries; j++ {
+				timer.Reset(Hour + Duration(i*j))
+			}
+		}(i)
+	}
+	wg.Wait()
+}