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>

1 files changed, 18 insertions, 2 deletions

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 071257b5a5..79529ac7ec 100644
--- 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