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/10156Reviewed-by: Russ Cox <rsc@golang.org>

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

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.