Skip to content

Commit

Permalink
[release-branch.go1.18] runtime: clear timerModifiedEarliest when las…
Browse files Browse the repository at this point in the history
…t timer is deleted

timerModifiedEarliest contains the lowest possible expiration for a
modified earlier timer, which may be earlier than timer0When because we
haven't yet updated the heap. Note "may", as the modified earlier timer
that set timerModifiedEarliest may have since been modified later or
deleted.

We can clear timerModifiedEarliest when the last timer is deleted
because by definition there must not be any modified earlier timers.

Why does this matter? checkTimersNoP claims that there is work to do if
timerModifiedEarliest has passed, causing findRunnable to loop back
around to checkTimers. But the code to clean up timerModifiedEarliest in
checkTimers (i.e., the call to adjusttimers) is conditional behind a
check that len(pp.timers) > 0.

Without clearing timerModifiedEarliest, a spinning M that would
otherwise go to sleep will busy loop in findRunnable until some other
work is available.

Note that changing the condition on the call to adjusttimers would also
be a valid fix. I took this approach because it feels a bit cleaner to
clean up timerModifiedEarliest as soon as it is known to be irrelevant.

For golang#51654.
Fixes golang#53847.

Change-Id: I3f3787c67781cac7ce87939c5706cef8db927dd5
Reviewed-on: https://go-review.googlesource.com/c/go/+/417434
Auto-Submit: Michael Pratt <mpratt@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Run-TryBot: Michael Pratt <mpratt@google.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Ian Lance Taylor <iant@google.com>
(cherry picked from commit c006b7a)
  • Loading branch information
prattmic authored and bradfitz committed Jul 14, 2022
1 parent 8096402 commit 149f7d8
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions src/runtime/time.go
Original file line number Diff line number Diff line change
Expand Up @@ -391,7 +391,11 @@ func dodeltimer(pp *p, i int) int {
if i == 0 {
updateTimer0When(pp)
}
atomic.Xadd(&pp.numTimers, -1)
n := atomic.Xadd(&pp.numTimers, -1)
if n == 0 {
// If there are no timers, then clearly none are modified.
atomic.Store64(&pp.timerModifiedEarliest, 0)
}
return smallestChanged
}

Expand All @@ -415,7 +419,11 @@ func dodeltimer0(pp *p) {
siftdownTimer(pp.timers, 0)
}
updateTimer0When(pp)
atomic.Xadd(&pp.numTimers, -1)
n := atomic.Xadd(&pp.numTimers, -1)
if n == 0 {
// If there are no timers, then clearly none are modified.
atomic.Store64(&pp.timerModifiedEarliest, 0)
}
}

// modtimer modifies an existing timer.
Expand Down

0 comments on commit 149f7d8

Please sign in to comment.