runtime: fix preemption of root marking jobs

The current logic in gcDrain conflates non-blocking with preemptible
draining for root jobs. As a result, if you do a non-blocking (but
*not* preemptible) drain, like dedicated workers do, the root job
drain will stop if preempted and fall through to heap marking jobs,
which won't stop until it fails to get a heap marking job.

This commit fixes the condition on root marking jobs so they only stop
when preempted if the drain is preemptible.

Coincidentally, this also fixes a nil pointer dereference if we call
gcDrain with gcDrainNoBlock and without a user G, since it tries to
get the preempt flag from the nil user G. This combination never
happens right now, but will in the future.

Change-Id: Ia910ec20a9b46237f7926969144a33b1b4a7b2f9
Reviewed-on: https://go-review.googlesource.com/32291
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go
index 286aeb74a9eade6eb932a82a8545e3368279c127..7819698def1cd339d354c28fd3cd034a271bbac1 100644 (file)
--- a/src/runtime/mgcmark.go
+++ b/src/runtime/mgcmark.go
@@ -973,7 +973,7 @@ func gcDrain(gcw *gcWork, flags gcDrainFlags) {
 
        // Drain root marking jobs.
        if work.markrootNext < work.markrootJobs {
-               for blocking || !gp.preempt {
+               for !(preemptible && gp.preempt) {
                        job := atomic.Xadd(&work.markrootNext, +1) - 1
                        if job >= work.markrootJobs {
                                break