runtime: release worldsema with a direct G handoff

Currently worldsema is not released with direct handoff, so the
semaphore is an unfair synchronization mechanism. If, for example,
ReadMemStats is called in a loop, it can continuously stomp on attempts
by the GC to stop the world.

Note that it's specifically possible for ReadMemStats to delay a STW to
end GC since ReadMemStats is able to STW during a GC since #19112 was
fixed.

While this particular case is unlikely and the right answer in most
applications is to simply not call such an expensive operation in a
loop, this pattern is used often in tests.

Fixes #40459.

Change-Id: Ia4a54f0fd956ea145a319f9f06c4cd37dd52fd8a
Reviewed-on: https://go-review.googlesource.com/c/go/+/243977
Run-TryBot: Michael Knyszek <mknyszek@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Trust: Michael Knyszek <mknyszek@google.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
Reviewed-by: Austin Clements <austin@google.com>
Reviewed-by: David Chase <drchase@google.com>

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 071257b5a505590a3b79b0dbdfc14efdead24992..79529ac7ec78acf76a56ecc3fa22f4f0853b2e73 100644 (file)
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -961,10 +961,26 @@ func stopTheWorld(reason string) {
 // startTheWorld undoes the effects of stopTheWorld.
 func startTheWorld() {
        systemstack(func() { startTheWorldWithSema(false) })
+
        // worldsema must be held over startTheWorldWithSema to ensure
        // gomaxprocs cannot change while worldsema is held.
-       semrelease(&worldsema)
-       getg().m.preemptoff = ""
+       //
+       // Release worldsema with direct handoff to the next waiter, but
+       // acquirem so that semrelease1 doesn't try to yield our time.
+       //
+       // Otherwise if e.g. ReadMemStats is being called in a loop,
+       // it might stomp on other attempts to stop the world, such as
+       // for starting or ending GC. The operation this blocks is
+       // so heavy-weight that we should just try to be as fair as
+       // possible here.
+       //
+       // We don't want to just allow us to get preempted between now
+       // and releasing the semaphore because then we keep everyone
+       // (including, for example, GCs) waiting longer.
+       mp := acquirem()
+       mp.preemptoff = ""
+       semrelease1(&worldsema, true, 0)
+       releasem(mp)
 }
 
 // stopTheWorldGC has the same effect as stopTheWorld, but blocks