]> Cypherpunks repositories - gostls13.git/commit
[release-branch.go1.23] runtime: prevent weak->strong conversions during mark termination
authorMichael Anthony Knyszek <mknyszek@google.com>
Fri, 1 Nov 2024 21:54:07 +0000 (21:54 +0000)
committerGopher Robot <gobot@golang.org>
Wed, 20 Nov 2024 19:08:32 +0000 (19:08 +0000)
commitd8adc6c4c7cc21d607a97aeaa6b7ffc4c2d76e65
tree3d84eb1a92977dcd27fe37431bc9713eff5e0465
parent847cb6f9ca43da48cb10e98808a74a40b41242fa
[release-branch.go1.23] runtime: prevent weak->strong conversions during mark termination

Currently it's possible for weak->strong conversions to create more GC
work during mark termination. When a weak->strong conversion happens
during the mark phase, we need to mark the newly-strong pointer, since
it may now be the only pointer to that object. In other words, the
object could be white.

But queueing new white objects creates GC work, and if this happens
during mark termination, we could end up violating mark termination
invariants. In the parlance of the mark termination algorithm, the
weak->strong conversion is a non-monotonic source of GC work, unlike the
write barriers (which will eventually only see black objects).

This change fixes the problem by forcing weak->strong conversions to
block during mark termination. We can do this efficiently by setting a
global flag before the ragged barrier that is checked at each
weak->strong conversion. If the flag is set, then the conversions block.
The ragged barrier ensures that all Ps have observed the flag and that
any weak->strong conversions which completed before the ragged barrier
have their newly-minted strong pointers visible in GC work queues if
necessary. We later unset the flag and wake all the blocked goroutines
during the mark termination STW.

There are a few subtleties that we need to account for. For one, it's
possible that a goroutine which blocked in a weak->strong conversion
wakes up only to find it's mark termination time again, so we need to
recheck the global flag on wake. We should also stay non-preemptible
while performing the check, so that if the check *does* appear as true,
it cannot switch back to false while we're actively trying to block. If
it switches to false while we try to block, then we'll be stuck in the
queue until the following GC.

All-in-all, this CL is more complicated than I would have liked, but
it's the only idea so far that is clearly correct to me at a high level.

This change adds a test which is somewhat invasive as it manipulates
mark termination, but hopefully that infrastructure will be useful for
debugging, fixing, and regression testing mark termination whenever we
do fix it.

For #69803.
Fixes #70323.

Change-Id: Ie314e6fd357c9e2a07a9be21f217f75f7aba8c4a
Reviewed-on: https://go-review.googlesource.com/c/go/+/623615
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Cherry Mui <cherryyz@google.com>
(cherry picked from commit 80d306da50aef6334bcb65fb02f5728cb9513691)
Reviewed-on: https://go-review.googlesource.com/c/go/+/627615
TryBot-Bypass: Dmitri Shuralyov <dmitshur@google.com>
Auto-Submit: Dmitri Shuralyov <dmitshur@google.com>
src/runtime/export_test.go
src/runtime/gc_test.go
src/runtime/lockrank.go
src/runtime/mgc.go
src/runtime/mheap.go
src/runtime/mklockrank.go
src/runtime/runtime2.go
src/runtime/traceruntime.go