1
0
mirror of https://github.com/golang/go synced 2024-11-25 17:07:57 -07:00

[release-branch.go1.23] runtime: don't frob isSending for tickers

The Ticker Stop and Reset methods don't report a value,
so we don't need to track whether they are interrupting a send.

This includes a test that used to fail about 2% of the time on
my laptop when run under x/tools/cmd/stress.

For #69880
Fixes #69882

Change-Id: Ic6d14b344594149dd3c24b37bbe4e42e83f9a9ad
Reviewed-on: https://go-review.googlesource.com/c/go/+/620136
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Auto-Submit: Michael Knyszek <mknyszek@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
(cherry picked from commit 48849e0866)
Reviewed-on: https://go-review.googlesource.com/c/go/+/620137
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>
This commit is contained in:
Ian Lance Taylor 2024-10-14 11:46:17 -07:00 committed by Gopher Robot
parent 35c010ad6d
commit 8d79bf799b
2 changed files with 36 additions and 6 deletions

View File

@ -33,6 +33,7 @@ type timer struct {
// isSending is used to handle races between running a
// channel timer and stopping or resetting the timer.
// It is used only for channel timers (t.isChan == true).
// It is not used for tickers.
// The lowest zero bit is set when about to send a value on the channel,
// and cleared after sending the value.
// The stop/reset code uses this to detect whether it
@ -467,7 +468,7 @@ func (t *timer) stop() bool {
// send from actually happening. That means
// that we should return true: the timer was
// stopped, even though t.when may be zero.
if t.isSending.Load() > 0 {
if t.period == 0 && t.isSending.Load() > 0 {
pending = true
}
}
@ -529,6 +530,7 @@ func (t *timer) modify(when, period int64, f func(arg any, seq uintptr, delay in
t.maybeRunAsync()
}
t.trace("modify")
oldPeriod := t.period
t.period = period
if f != nil {
t.f = f
@ -570,7 +572,7 @@ func (t *timer) modify(when, period int64, f func(arg any, seq uintptr, delay in
// send from actually happening. That means
// that we should return true: the timer was
// stopped, even though t.when may be zero.
if t.isSending.Load() > 0 {
if oldPeriod == 0 && t.isSending.Load() > 0 {
pending = true
}
}
@ -1064,7 +1066,7 @@ func (t *timer) unlockAndRun(now int64) {
async := debug.asynctimerchan.Load() != 0
var isSendingClear uint8
if !async && t.isChan {
if !async && t.isChan && t.period == 0 {
// Tell Stop/Reset that we are sending a value.
// Set the lowest zero bit.
// We do this awkward step because atomic.Uint8
@ -1115,9 +1117,12 @@ func (t *timer) unlockAndRun(now int64) {
// true meaning that no value was sent.
lock(&t.sendLock)
// We are committed to possibly sending a value based on seq,
// so no need to keep telling stop/modify that we are sending.
if t.period == 0 {
// We are committed to possibly sending a value
// based on seq, so no need to keep telling
// stop/modify that we are sending.
t.isSending.And(^isSendingClear)
}
if t.seq != seq {
f = func(any, uintptr, int64) {}

View File

@ -847,6 +847,31 @@ func testStopResetResultGODEBUG(t *testing.T, testStop bool, godebug string) {
wg.Wait()
}
// Test having a large number of goroutines wake up a timer simultaneously.
// This used to trigger a crash when run under x/tools/cmd/stress.
func TestMultiWakeup(t *testing.T) {
if testing.Short() {
t.Skip("-short")
}
goroutines := runtime.GOMAXPROCS(0)
timer := NewTicker(Microsecond)
var wg sync.WaitGroup
wg.Add(goroutines)
for range goroutines {
go func() {
defer wg.Done()
for range 100000 {
select {
case <-timer.C:
case <-After(Millisecond):
}
}
}()
}
wg.Wait()
}
// Benchmark timer latency when the thread that creates the timer is busy with
// other work and the timers must be serviced by other threads.
// https://golang.org/issue/38860