[release-branch.go1.17] runtime: clear timerModifiedEarliest when last 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 #51654.
Fixes #53846.

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 c006b7ac2765252f397dec40fef610a3c17d956d)
Reviewed-on: https://go-review.googlesource.com/c/go/+/417476

1 files changed, 10 insertions, 2 deletions

diff --git a/src/runtime/time.go b/src/runtime/time.go
index 517a493a70..70e21d4482 100644
--- a/src/runtime/time.go
+++ b/src/runtime/time.go
@@ -390,7 +390,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
 }
 
@@ -414,7 +418,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.