runtime: use atomic.Cas to change timerRemoved to timerWaiting

If multiple goroutines call time.(*Timer).Reset then the timer will go
from timerWaiting to timerDeleted to timerModifying to timerModifiedLater.
The timer can be on a different P, meaning that simultaneously cleantimers
could change it from timerDeleted to timerRemoving to timerRemoved.
If Reset sees timerRemoved, it was doing an atomic.Store of timerWaiting,
meaning that it did not necessarily see the other values set in the timer,
so the timer could appear to be in an inconsistent state. Use atomic.Cas
to avoid that possibility.

Updates #6239
Updates #27707
Fixes #35272

Change-Id: I1d59a13dc4f2ff4af110fc6e032c8c9d59cfc270
Reviewed-on: https://go-review.googlesource.com/c/go/+/204717
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Michael Knyszek <mknyszek@google.com>

1 files changed, 9 insertions, 7 deletions

diff --git a/src/runtime/time.go b/src/runtime/time.go
index 6c1170bbc0..e6a24c5561 100644
--- a/src/runtime/time.go
+++ b/src/runtime/time.go
@@ -585,9 +585,10 @@ loop:
 		case timerNoStatus, timerRemoved:
 			// Timer was already run and t is no longer in a heap.
 			// Act like addtimer.
-			wasRemoved = true
-			atomic.Store(&t.status, timerWaiting)
-			break loop
+			if atomic.Cas(&t.status, status, timerWaiting) {
+				wasRemoved = true
+				break loop
+			}
 		case timerRunning, timerRemoving, timerMoving:
 			// The timer is being run or moved, by a different P.
 			// Wait for it to complete.
@@ -687,10 +688,11 @@ func resettimer(t *timer, when int64) {
 	for {
 		switch s := atomic.Load(&t.status); s {
 		case timerNoStatus, timerRemoved:
-			atomic.Store(&t.status, timerWaiting)
-			t.when = when
-			addInitializedTimer(t)
-			return
+			if atomic.Cas(&t.status, s, timerWaiting) {
+				t.when = when
+				addInitializedTimer(t)
+				return
+			}
 		case timerDeleted:
 			if atomic.Cas(&t.status, s, timerModifying) {
 				t.nextwhen = when