Skip to content

Commit 64a9a75

Browse files
committed
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 <[email protected]> TryBot-Result: Go Bot <[email protected]> Trust: Michael Knyszek <[email protected]> Reviewed-by: Michael Pratt <[email protected]> Reviewed-by: Austin Clements <[email protected]> Reviewed-by: David Chase <[email protected]>
1 parent f96b62b commit 64a9a75

File tree

1 file changed

+18
-2
lines changed

1 file changed

+18
-2
lines changed

src/runtime/proc.go

+18-2
Original file line numberDiff line numberDiff line change
@@ -961,10 +961,26 @@ func stopTheWorld(reason string) {
961961
// startTheWorld undoes the effects of stopTheWorld.
962962
func startTheWorld() {
963963
systemstack(func() { startTheWorldWithSema(false) })
964+
964965
// worldsema must be held over startTheWorldWithSema to ensure
965966
// gomaxprocs cannot change while worldsema is held.
966-
semrelease(&worldsema)
967-
getg().m.preemptoff = ""
967+
//
968+
// Release worldsema with direct handoff to the next waiter, but
969+
// acquirem so that semrelease1 doesn't try to yield our time.
970+
//
971+
// Otherwise if e.g. ReadMemStats is being called in a loop,
972+
// it might stomp on other attempts to stop the world, such as
973+
// for starting or ending GC. The operation this blocks is
974+
// so heavy-weight that we should just try to be as fair as
975+
// possible here.
976+
//
977+
// We don't want to just allow us to get preempted between now
978+
// and releasing the semaphore because then we keep everyone
979+
// (including, for example, GCs) waiting longer.
980+
mp := acquirem()
981+
mp.preemptoff = ""
982+
semrelease1(&worldsema, true, 0)
983+
releasem(mp)
968984
}
969985

970986
// stopTheWorldGC has the same effect as stopTheWorld, but blocks

0 commit comments

Comments
 (0)