From: Austin Clements <austin@google.com>
Date: Fri, 15 May 2015 20:13:14 +0000 (-0400)
Subject: runtime: hold worldsema while starting the world
X-Git-Tag: go1.5beta1~546
X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=277acca286e47fd704aae10d030c74927ba2a8d2;p=gostls13.git

runtime: hold worldsema while starting the world

Currently, startTheWorld releases worldsema before starting the
world. Since startTheWorld can change gomaxprocs after allowing Ps to
run, this means that gomaxprocs can change while another P holds
worldsema.

Unfortunately, the garbage collector and forEachP assume that holding
worldsema protects against changes in gomaxprocs (which it *almost*
does). In particular, this is causing somewhat frequent "P did not run
fn" crashes in forEachP in the runtime tests because gomaxprocs is
changing between the several loops that forEachP does over all the Ps.

Fix this by only releasing worldsema after the world is started.

This relates to issue #10618. forEachP still fails under stress
testing, but much less frequently.

Change-Id: I085d627b70cca9ebe9af28fe73b9872f1bb224ff
Reviewed-on: https://go-review.googlesource.com/10156
Reviewed-by: Russ Cox <rsc@golang.org>
---

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index 68636740a6..a16d7603a6 100644
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -952,13 +952,12 @@ func gc(mode int) {
 	// all done
 	mp.preemptoff = ""
 
-	semrelease(&worldsema)
-
 	if gcphase != _GCoff {
 		throw("gc done but gcphase != _GCoff")
 	}
 
 	systemstack(startTheWorldWithSema)
+	semrelease(&worldsema)
 
 	releasem(mp)
 	mp = nil
diff --git a/src/runtime/proc1.go b/src/runtime/proc1.go
index ab0566b470..31247db02a 100644
--- a/src/runtime/proc1.go
+++ b/src/runtime/proc1.go
@@ -550,12 +550,15 @@ func stopTheWorld(reason string) {
 
 // startTheWorld undoes the effects of stopTheWorld.
 func startTheWorld() {
-	semrelease(&worldsema)
 	systemstack(startTheWorldWithSema)
+	// worldsema must be held over startTheWorldWithSema to ensure
+	// gomaxprocs cannot change while worldsema is held.
+	semrelease(&worldsema)
 	getg().m.preemptoff = ""
 }
 
-// Holding worldsema grants an M the right to try to stop the world.
+// Holding worldsema grants an M the right to try to stop the world
+// and prevents gomaxprocs from changing concurrently.
 var worldsema uint32 = 1
 
 // stopTheWorldWithSema is the core implementation of stopTheWorld.
@@ -571,8 +574,8 @@ var worldsema uint32 = 1
 // these three operations separately:
 //
 //	m.preemptoff = ""
-//	semrelease(&worldsema)
 //	systemstack(startTheWorldWithSema)
+//	semrelease(&worldsema)
 //
 // It is allowed to acquire worldsema once and then execute multiple
 // startTheWorldWithSema/stopTheWorldWithSema pairs.