From 64a9a75ce9a353ef3d488b8e3ca977bf6df204f8 Mon Sep 17 00:00:00 2001 From: Michael Anthony Knyszek Date: Mon, 20 Jul 2020 18:19:56 +0000 Subject: [PATCH] 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 TryBot-Result: Go Bot Trust: Michael Knyszek Reviewed-by: Michael Pratt Reviewed-by: Austin Clements Reviewed-by: David Chase --- src/runtime/proc.go | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/runtime/proc.go b/src/runtime/proc.go index 071257b5a50..79529ac7ec7 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