Skip to content

Commit

Permalink
runtime: fix sleep/wakeup race for GC assists
Browse files Browse the repository at this point in the history
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 golang#13645.

Change-Id: I7571f059530481dc781d8fd96a1a40aadebecb0d
Reviewed-on: https://go-review.googlesource.com/18682
Run-TryBot: Austin Clements <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Rick Hudson <[email protected]>
  • Loading branch information
aclements committed Jan 16, 2016
1 parent aea4de8 commit b05f18e
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
4 changes: 4 additions & 0 deletions src/runtime/mgc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion src/runtime/mgcmark.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down

0 comments on commit b05f18e

Please sign in to comment.