runtime: mark extra M's G as dead when not in use

Currently the extra Ms created for cgo callbacks have a corresponding
G that's kept in syscall state with only a call to goexit on its
stack. This leads to confusing output from runtime.NumGoroutines and
in tracebacks:

goroutine 17 [syscall, locked to thread]:
runtime.goexit()
	.../src/runtime/asm_amd64.s:2197 +0x1

Fix this by putting this goroutine into state _Gdead when it's not in
use instead of _Gsyscall. To keep the goroutine counts correct, we
also add one to sched.ngsys while the goroutine is in _Gdead. The
effect of this is as if the goroutine simply doesn't exist when it's
not in use.

Fixes #16631.
Fixes #16714.

Change-Id: Ieae08a2febd4b3d00bef5c23fd6ca88fb2bb0087
Reviewed-on: https://go-review.googlesource.com/45030
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>

1 files changed, 19 insertions, 3 deletions

diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 24a62492e1..099605fe52 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -1435,6 +1435,10 @@ func needm(x byte) {
 	// Initialize this thread to use the m.
 	asminit()
 	minit()
+
+	// mp.curg is now a real goroutine.
+	casgstatus(mp.curg, _Gdead, _Gsyscall)
+	atomic.Xadd(&sched.ngsys, -1)
 }
 
 var earlycgocallback = []byte("fatal error: cgo callback before cgo call\n")
@@ -1477,9 +1481,11 @@ func oneNewExtraM() {
 	gp.stktopsp = gp.sched.sp
 	gp.gcscanvalid = true
 	gp.gcscandone = true
-	// malg returns status as Gidle, change to Gsyscall before adding to allg
-	// where GC will see it.
-	casgstatus(gp, _Gidle, _Gsyscall)
+	// malg returns status as _Gidle. Change to _Gdead before
+	// adding to allg where GC can see it. We use _Gdead to hide
+	// this from tracebacks and stack scans since it isn't a
+	// "real" goroutine until needm grabs it.
+	casgstatus(gp, _Gidle, _Gdead)
 	gp.m = mp
 	mp.curg = gp
 	mp.locked = _LockInternal
@@ -1492,6 +1498,12 @@ func oneNewExtraM() {
 	// put on allg for garbage collector
 	allgadd(gp)
 
+	// gp is now on the allg list, but we don't want it to be
+	// counted by gcount. It would be more "proper" to increment
+	// sched.ngfree, but that requires locking. Incrementing ngsys
+	// has the same effect.
+	atomic.Xadd(&sched.ngsys, +1)
+
 	// Add m to the extra list.
 	mnext := lockextra(true)
 	mp.schedlink.set(mnext)
@@ -1528,6 +1540,10 @@ func dropm() {
 	// with no pointer manipulation.
 	mp := getg().m
 
+	// Return mp.curg to dead state.
+	casgstatus(mp.curg, _Gsyscall, _Gdead)
+	atomic.Xadd(&sched.ngsys, +1)
+
 	// Block signals before unminit.
 	// Unminit unregisters the signal handling stack (but needs g on some systems).
 	// Setg(nil) clears g, which is the signal handler's cue not to run Go handlers.