diff options
author | Austin Clements <austin@google.com> | 2017-01-17 21:58:10 -0500 |
---|---|---|
committer | Brad Fitzpatrick <bradfitz@golang.org> | 2017-01-25 17:18:04 +0000 |
commit | c4552c1c61d668886d31391a4694983b2912eaa7 (patch) | |
tree | a513a7a51bda91639c7d94d2dd5392b49b5a6099 | |
parent | faafe0e30c1779bd928e21421e67924108841c73 (diff) | |
download | go-c4552c1c61d668886d31391a4694983b2912eaa7.tar.gz go-c4552c1c61d668886d31391a4694983b2912eaa7.zip |
[release-branch.go1.7] runtime: force workers out before checking mark roots
Fixes #18700 (backport)
Currently we check that all roots are marked as soon as gcMarkDone
decides to transition from mark 1 to mark 2. However, issue #16083
indicates that there may be a race where we try to complete mark 1
while a worker is still scanning a stack, causing the root mark check
to fail.
We don't yet understand this race, but as a simple mitigation, move
the root check to after gcMarkDone performs a ragged barrier, which
will force any remaining workers to finish their current job.
Updates #16083. This may "fix" it, but it would be better to
understand and fix the underlying race.
Change-Id: I1af9ce67bd87ade7bc2a067295d79c28cd11abd2
Reviewed-on: https://go-review.googlesource.com/35678
Run-TryBot: Austin Clements <austin@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
-rw-r--r-- | src/runtime/mgc.go | 12 |
1 files changed, 10 insertions, 2 deletions
diff --git a/src/runtime/mgc.go b/src/runtime/mgc.go index 3b238cba1c..67b4a216cd 100644 --- a/src/runtime/mgc.go +++ b/src/runtime/mgc.go @@ -1076,8 +1076,6 @@ top: // objects reachable from global roots since they don't have write // barriers. Rescan some roots and flush work caches. - gcMarkRootCheck() - // Disallow caching workbufs and indicate that we're in mark 2. gcBlackenPromptly = true @@ -1101,6 +1099,16 @@ top: }) }) + // Check that roots are marked. We should be able to + // do this before the forEachP, but based on issue + // #16083 there may be a (harmless) race where we can + // enter mark 2 while some workers are still scanning + // stacks. The forEachP ensures these scans are done. + // + // TODO(austin): Figure out the race and fix this + // properly. + gcMarkRootCheck() + // Now we can start up mark 2 workers. atomic.Xaddint64(&gcController.dedicatedMarkWorkersNeeded, 0xffffffff) atomic.Xaddint64(&gcController.fractionalMarkWorkersNeeded, 0xffffffff) |