From b05f18e3f7c3bb01b2faa7a52054883b0cc3a98a Mon Sep 17 00:00:00 2001 From: Austin Clements Date: Fri, 15 Jan 2016 13:28:41 -0500 Subject: [PATCH] runtime: fix sleep/wakeup race for GC assists GC assists check gcBlackenEnabled under the assist queue lock to avoid going to sleep after gcWakeAllAssists has already woken all assists. However, currently we clear gcBlackenEnabled shortly *after* waking all assists, which opens a window where this exact race can happen. Fix this by clearing gcBlackenEnabled before waking blocked assists. However, it's unlikely this actually matters because the world is stopped between waking assists and clearing gcBlackenEnabled and there aren't any obvious allocations during this window, so I don't think an assist could actually slip in to this race window. Updates #13645. Change-Id: I7571f059530481dc781d8fd96a1a40aadebecb0d Reviewed-on: https://go-review.googlesource.com/18682 Run-TryBot: Austin Clements TryBot-Result: Gobot Gobot Reviewed-by: Rick Hudson --- src/runtime/mgc.go | 4 ++++ src/runtime/mgcmark.go | 3 ++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index c09f70423d0..92b811830c3 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -1121,6 +1121,10 @@ top: // finalizers have been scanned. work.finalizersDone = true + // Disable assists and background workers. We must do + // this before waking blocked assists. + atomic.Store(&gcBlackenEnabled, 0) + // Flush the gcWork caches. This must be done before // endCycle since endCycle depends on statistics kept // in these caches. diff --git a/src/runtime/mgcmark.go b/src/runtime/mgcmark.go index 52545afa299..eac45ec1685 100644 --- a/src/runtime/mgcmark.go +++ b/src/runtime/mgcmark.go @@ -491,7 +491,8 @@ retry: } // gcWakeAllAssists wakes all currently blocked assists. This is used -// at the end of a GC cycle. +// at the end of a GC cycle. gcBlackenEnabled must be false to prevent +// new assists from going to sleep after this point. func gcWakeAllAssists() { lock(&work.assistQueue.lock) injectglist(work.assistQueue.head.ptr())