runtime: fix mark 2 completion in fractional/idle workers

Currently fractional and idle mark workers dispose of their gcWork
cache during mark 2 after incrementing work.nwait and after checking
whether there are any workers or any work available. This creates a
window for two races:

1) If the only remaining work is in this worker's gcWork cache, it
   will see that there are no more workers and no more work on the
   global lists (since it has not yet flushed its own cache) and
   prematurely signal mark 2 completion.

2) After this worker has incremented work.nwait but before it has
   flushed its cache, another worker may observe that there are no
   more workers and no more work and prematurely signal mark 2
   completion.

We can fix both of these by simply moving the cache flush above the
increment of nwait and the test of the completion condition.

This is probably contributing to #11694, though this alone is not
enough to fix it.

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

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index 889abfbbc9c3f9220bc565cb96d92a65b63fb45e..eab13a99fc1e2eb54fdf6fb6dc305456fe04f1d8 100644 (file)
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -1269,8 +1269,22 @@ func gcBgMarkWorker(p *p) {
                        // match the decrement above. It only returns
                        // at a mark completion point.
                        done = true
+                       if !p.gcw.empty() {
+                               throw("gcDrain returned with buffer")
+                       }
                case gcMarkWorkerFractionalMode, gcMarkWorkerIdleMode:
                        gcDrainUntilPreempt(&p.gcw, gcBgCreditSlack)
+
+                       // If we are nearing the end of mark, dispose
+                       // of the cache promptly. We must do this
+                       // before signaling that we're no longer
+                       // working so that other workers can't observe
+                       // no workers and no work while we have this
+                       // cached, and before we compute done.
+                       if gcBlackenPromptly {
+                               p.gcw.dispose()
+                       }
+
                        // Was this the last worker and did we run out
                        // of work?
                        incnwait := xadd(&work.nwait, +1)
@@ -1281,10 +1295,6 @@ func gcBgMarkWorker(p *p) {
                        }
                        done = incnwait == work.nproc && work.full == 0 && work.partial == 0
                }
-               // If we are near the end of the mark phase dispose of p.gcw.
-               if gcBlackenPromptly {
-                       p.gcw.dispose()
-               }
 
                // If this worker reached a background mark completion
                // point, signal the main GC goroutine.