[dev.garbage] runtime: Stop running gs during the GCscan phase.

Ensure that all gs are in a scan state when their stacks are being scanned.

LGTM=rsc
R=rsc
CC=golang-codereviews
https://golang.org/cl/179160044

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index 5b0c9b9905e80cdb3246232672777069c88f0f77..a13de0488c9d9865ab81be35da7a8b654d47172d 100644 (file)
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -923,14 +923,11 @@ func scanframe(frame *stkframe, unused unsafe.Pointer) bool {
 }
 
 func scanstack(gp *g) {
-       // TODO(rsc): Due to a precedence error, this was never checked in the original C version.
-       // If you enable the check, the gothrow happens.
-       /*
-               if readgstatus(gp)&_Gscan == 0 {
-                       print("runtime: gp=", gp, ", goid=", gp.goid, ", gp->atomicstatus=", readgstatus(gp), "\n")
-                       gothrow("mark - bad status")
-               }
-       */
+
+       if readgstatus(gp)&_Gscan == 0 {
+               print("runtime:scanstack: gp=", gp, ", goid=", gp.goid, ", gp->atomicstatus=", hex(readgstatus(gp)), "\n")
+               gothrow("scanstack - bad status")
+       }
 
        switch readgstatus(gp) &^ _Gscan {
        default:

diff --git a/src/runtime/proc1.go b/src/runtime/proc1.go
index 8c941dd35dc47d1b1d34e3dfe17ad9fbcedb7975..be01f26718fa2ed1b3ddd5bfd742fd066db1e63e 100644 (file)
--- a/src/runtime/proc1.go
+++ b/src/runtime/proc1.go
@@ -316,6 +316,10 @@ func casfrom_Gscanstatus(gp *g, oldval, newval uint32) {
 
        // Check that transition is valid.
        switch oldval {
+       default:
+               print("runtime: casfrom_Gscanstatus bad oldval gp=", gp, ", oldval=", hex(oldval), ", newval=", hex(newval), "\n")
+               dumpgstatus(gp)
+               gothrow("casfrom_Gscanstatus:top gp->status is not in scan state")
        case _Gscanrunnable,
                _Gscanwaiting,
                _Gscanrunning,
@@ -417,13 +421,6 @@ func stopg(gp *g) bool {
                        return false
 
                case _Grunning:
-                       if gcphase == _GCscan {
-                               // Running routines not scanned during
-                               // GCscan phase, we only scan non-running routines.
-                               gp.gcworkdone = true
-                               return false
-                       }
-
                        // Claim goroutine, so we aren't racing with a status
                        // transition away from Grunning.
                        if !castogscanstatus(gp, _Grunning, _Gscanrunning) {

diff --git a/src/runtime/stack1.go b/src/runtime/stack1.go
index 78bcccc41baa5f0c15faaa5d8a9f88961bee9490..57d0f8c65ef271520b9480f576678a2f5ddcdd29 100644 (file)
--- a/src/runtime/stack1.go
+++ b/src/runtime/stack1.go
@@ -682,7 +682,12 @@ func newstack() {
                        gothrow("runtime: g is running but p is not")
                }
                if gp.preemptscan {
+                       for !castogscanstatus(gp, _Gwaiting, _Gscanwaiting) {
+                               // Likely to be racing with the GC as it sees a _Gwaiting and does the stack scan.
+                               // If so this stack will be scanned twice which does not change correctness.
+                       }
                        gcphasework(gp)
+                       casfrom_Gscanstatus(gp, _Gscanwaiting, _Gwaiting)
                        casgstatus(gp, _Gwaiting, _Grunning)
                        gp.stackguard0 = gp.stack.lo + _StackGuard
                        gp.preempt = false