runtime: doubly fix "double wakeup" panic

runtime.gchelper depends on the non-atomic load of work.ndone
happening strictly before the atomic add of work.nwait. Until very
recently (commit 978af9c2db, fixing #20334), the compiler reordered
these operations. This created a race since work.ndone can change as
soon as work.nwait is equal to work.ndone. If that happened, more than
one gchelper could attempt to wake up the work.alldone note, causing a
"double wakeup" panic.

This was fixed in the compiler, but to make this code less subtle,
make the load of work.ndone atomic. This clearly forces the order of
these operations, ensuring the race doesn't happen.

Fixes #19305 (though really 978af9c2db fixed it).

Change-Id: Ieb1a84e1e5044c33ac612c8a5ab6297e7db4c57d
Reviewed-on: https://go-review.googlesource.com/43311
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Keith Randall <khr@golang.org>

diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go
index 5dc417038ab5cd8b76bf70b7af1fb9117a5373b4..22e8c313176eac8fed9b7ab39d45db83eec47a37 100644 (file)
--- a/src/runtime/mgc.go
+++ b/src/runtime/mgc.go
@@ -2105,7 +2105,7 @@ func gchelper() {
                traceGCScanDone()
        }
 
-       nproc := work.nproc // work.nproc can change right after we increment work.ndone
+       nproc := atomic.Load(&work.nproc) // work.nproc can change right after we increment work.ndone
        if atomic.Xadd(&work.ndone, +1) == nproc-1 {
                notewakeup(&work.alldone)
        }