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>

diff --git a/src/net/timeout_test.go b/src/net/timeout_test.go
index e90c16f34b13b31548b23a5d930731dddb5ebe89..f54c9564f9e37b63d16179a31bfb0bebdc797582 100644 (file)
--- 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 6ae5225c68d20edf378baaebb4689ecce295d515..ad5eaf7c4830a1001ca334da75ea9c87a793ebeb 100644 (file)
--- 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()
                }
        }
 }