runtime: keep adjusted timers in timerMoving status until moved

Before this CL adjustTimers left timers being moved in an inconsistent
state: status timerWaiting but not on a P. Simplify the code by
leaving the timers in timerMoving status until they are actually moved.
Other functions (deltimer, modtimer) will wait until the move is complete
before changing anything on the timer. This does leave timers in timerMoving
state for longer, but still not all that long.

Fixes #35367

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

2 files changed, 45 insertions, 44 deletions

diff --git a/src/net/timeout_test.go b/src/net/timeout_test.go
index e90c16f34b..f54c9564f9 100644
--- a/src/net/timeout_test.go
+++ b/src/net/timeout_test.go
@@ -1033,3 +1033,43 @@ func TestReadWriteDeadlineRace(t *testing.T) {
 	}()
 	wg.Wait() // wait for tester goroutine to stop
 }
+
+// Issue 35367.
+func TestConcurrentSetDeadline(t *testing.T) {
+	ln, err := newLocalListener("tcp")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer ln.Close()
+
+	const goroutines = 8
+	const conns = 10
+	const tries = 100
+
+	var c [conns]Conn
+	for i := 0; i < conns; i++ {
+		c[i], err = Dial(ln.Addr().Network(), ln.Addr().String())
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer c[i].Close()
+	}
+
+	var wg sync.WaitGroup
+	wg.Add(goroutines)
+	now := time.Now()
+	for i := 0; i < goroutines; i++ {
+		go func(i int) {
+			defer wg.Done()
+			// Make the deadlines steadily earlier,
+			// to trigger runtime adjusttimers calls.
+			for j := tries; j > 0; j-- {
+				for k := 0; k < conns; k++ {
+					c[k].SetReadDeadline(now.Add(2*time.Hour + time.Duration(i*j*k)*time.Second))
+					c[k].SetWriteDeadline(now.Add(1*time.Hour + time.Duration(i*j*k)*time.Second))
+				}
+			}
+		}(i)
+	}
+	wg.Wait()
+}
diff --git a/src/runtime/time.go b/src/runtime/time.go
index 6ae5225c68..ad5eaf7c48 100644
--- a/src/runtime/time.go
+++ b/src/runtime/time.go
@@ -947,9 +947,6 @@ func adjusttimers(pp *p) {
 					badTimer()
 				}
 				moved = append(moved, t)
-				if !atomic.Cas(&t.status, timerMoving, timerWaiting) {
-					badTimer()
-				}
 				if s == timerModifiedEarlier {
 					if n := atomic.Xadd(&pp.adjustTimers, -1); int32(n) <= 0 {
 						addAdjustedTimers(pp, moved)
@@ -979,47 +976,11 @@ func adjusttimers(pp *p) {
 // back to the timer heap.
 func addAdjustedTimers(pp *p, moved []*timer) {
 	for _, t := range moved {
-	loop:
-		for {
-			switch s := atomic.Load(&t.status); s {
-			case timerWaiting:
-				// This is the normal case.
-				if !doaddtimer(pp, t) {
-					badTimer()
-				}
-				break loop
-			case timerDeleted:
-				// Timer has been deleted since we adjusted it.
-				// This timer is already out of the heap.
-				if atomic.Cas(&t.status, s, timerRemoving) {
-					if !atomic.Cas(&t.status, timerRemoving, timerRemoved) {
-						badTimer()
-					}
-					break loop
-				}
-			case timerModifiedEarlier, timerModifiedLater:
-				// Timer has been modified again since
-				// we adjusted it.
-				if atomic.Cas(&t.status, s, timerMoving) {
-					t.when = t.nextwhen
-					if !doaddtimer(pp, t) {
-						badTimer()
-					}
-					if !atomic.Cas(&t.status, timerMoving, timerWaiting) {
-						badTimer()
-					}
-					if s == timerModifiedEarlier {
-						atomic.Xadd(&pp.adjustTimers, -1)
-					}
-					break loop
-				}
-			case timerNoStatus, timerRunning, timerRemoving, timerRemoved, timerMoving:
-				badTimer()
-			case timerModifying:
-				// Wait and try again.
-				osyield()
-				continue
-			}
+		if !doaddtimer(pp, t) {
+			badTimer()
+		}
+		if !atomic.Cas(&t.status, timerMoving, timerWaiting) {
+			badTimer()
 		}
 	}
 }