mirror of
https://github.com/golang/go
synced 2024-11-12 03:00:22 -07:00
runtime: fix comments on the behavior of SetGCPercent
Fixes for #49680, #49695, #45867, and #49370 all assumed that SetGCPercent(-1) doesn't block until the GC's mark phase is done, but it actually does. The cause of 3 of those 4 failures comes from the fact that at the beginning of the sweep phase, the GC does try to preempt every P once, and this may run concurrently with test code. In the fourth case, the issue was likely that only *one* of the debug_test.go tests was missing a call to SetGCPercent(-1). Just to be safe, leave a TODO there for now to remove the extraneous runtime.GC calls, but leave the calls in. Updates #49680, #49695, #45867, and #49370. Change-Id: Ibf4e64addfba18312526968bcf40f1f5d54eb3f1 Reviewed-on: https://go-review.googlesource.com/c/go/+/369815 Reviewed-by: Austin Clements <austin@google.com> Trust: Michael Knyszek <mknyszek@google.com> Run-TryBot: Michael Knyszek <mknyszek@google.com> TryBot-Result: Gopher Robot <gobot@golang.org>
This commit is contained in:
parent
dc65c489cc
commit
4c943abb95
4
misc/cgo/test/testdata/issue9400_linux.go
vendored
4
misc/cgo/test/testdata/issue9400_linux.go
vendored
@ -50,7 +50,9 @@ func test9400(t *testing.T) {
|
||||
// Disable GC for the duration of the test.
|
||||
// This avoids a potential GC deadlock when spinning in uninterruptable ASM below #49695.
|
||||
defer debug.SetGCPercent(debug.SetGCPercent(-1))
|
||||
// And finish any pending GC after we pause, if any.
|
||||
// SetGCPercent waits until the mark phase is over, but the runtime
|
||||
// also preempts at the start of the sweep phase, so make sure that's
|
||||
// done too. See #49695.
|
||||
runtime.GC()
|
||||
|
||||
// Temporarily rewind the stack and trigger SIGSETXID
|
||||
|
@ -34,10 +34,17 @@ func startDebugCallWorker(t *testing.T) (g *runtime.G, after func()) {
|
||||
skipUnderDebugger(t)
|
||||
|
||||
// This can deadlock if there aren't enough threads or if a GC
|
||||
// tries to interrupt an atomic loop (see issue #10958). A GC
|
||||
// could also actively be in progress (see issue #49370), so we
|
||||
// need to call runtime.GC to block until it has complete. We
|
||||
// use 8 Ps so there's room for the debug call worker,
|
||||
// tries to interrupt an atomic loop (see issue #10958). Execute
|
||||
// an extra GC to ensure even the sweep phase is done (out of
|
||||
// caution to prevent #49370 from happening).
|
||||
// TODO(mknyszek): This extra GC cycle is likely unnecessary
|
||||
// because preemption (which may happen during the sweep phase)
|
||||
// isn't much of an issue anymore thanks to asynchronous preemption.
|
||||
// The biggest risk is having a write barrier in the debug call
|
||||
// injection test code fire, because it runs in a signal handler
|
||||
// and may not have a P.
|
||||
//
|
||||
// We use 8 Ps so there's room for the debug call worker,
|
||||
// something that's trying to preempt the call worker, and the
|
||||
// goroutine that's trying to stop the call worker.
|
||||
ogomaxprocs := runtime.GOMAXPROCS(8)
|
||||
@ -270,8 +277,14 @@ func TestDebugCallPanic(t *testing.T) {
|
||||
// progress. Wait until the current GC is done, and turn it off.
|
||||
//
|
||||
// See #10958 and #49370.
|
||||
runtime.GC()
|
||||
defer debug.SetGCPercent(debug.SetGCPercent(-1))
|
||||
// TODO(mknyszek): This extra GC cycle is likely unnecessary
|
||||
// because preemption (which may happen during the sweep phase)
|
||||
// isn't much of an issue anymore thanks to asynchronous preemption.
|
||||
// The biggest risk is having a write barrier in the debug call
|
||||
// injection test code fire, because it runs in a signal handler
|
||||
// and may not have a P.
|
||||
runtime.GC()
|
||||
|
||||
ready := make(chan *runtime.G)
|
||||
var stop uint32
|
||||
|
@ -119,8 +119,9 @@ func TestGoroutineParallelism(t *testing.T) {
|
||||
// since the goroutines can't be stopped/preempted.
|
||||
// Disable GC for this test (see issue #10958).
|
||||
defer debug.SetGCPercent(debug.SetGCPercent(-1))
|
||||
// Now that GCs are disabled, block until any outstanding GCs
|
||||
// are also done.
|
||||
// SetGCPercent waits until the mark phase is over, but the runtime
|
||||
// also preempts at the start of the sweep phase, so make sure that's
|
||||
// done too. See #45867.
|
||||
runtime.GC()
|
||||
for try := 0; try < N; try++ {
|
||||
done := make(chan bool)
|
||||
@ -166,8 +167,9 @@ func testGoroutineParallelism2(t *testing.T, load, netpoll bool) {
|
||||
// since the goroutines can't be stopped/preempted.
|
||||
// Disable GC for this test (see issue #10958).
|
||||
defer debug.SetGCPercent(debug.SetGCPercent(-1))
|
||||
// Now that GCs are disabled, block until any outstanding GCs
|
||||
// are also done.
|
||||
// SetGCPercent waits until the mark phase is over, but the runtime
|
||||
// also preempts at the start of the sweep phase, so make sure that's
|
||||
// done too. See #45867.
|
||||
runtime.GC()
|
||||
for try := 0; try < N; try++ {
|
||||
if load {
|
||||
@ -629,8 +631,9 @@ func TestSchedLocalQueueEmpty(t *testing.T) {
|
||||
// If runtime triggers a forced GC during this test then it will deadlock,
|
||||
// since the goroutines can't be stopped/preempted during spin wait.
|
||||
defer debug.SetGCPercent(debug.SetGCPercent(-1))
|
||||
// Now that GCs are disabled, block until any outstanding GCs
|
||||
// are also done.
|
||||
// SetGCPercent waits until the mark phase is over, but the runtime
|
||||
// also preempts at the start of the sweep phase, so make sure that's
|
||||
// done too. See #45867.
|
||||
runtime.GC()
|
||||
|
||||
iters := int(1e5)
|
||||
|
@ -55,7 +55,9 @@ func TestParallelRWMutexReaders(t *testing.T) {
|
||||
// since the goroutines can't be stopped/preempted.
|
||||
// Disable GC for this test (see issue #10958).
|
||||
defer debug.SetGCPercent(debug.SetGCPercent(-1))
|
||||
// Finish any in-progress GCs and get ourselves to a clean slate.
|
||||
// SetGCPercent waits until the mark phase is over, but the runtime
|
||||
// also preempts at the start of the sweep phase, so make sure that's
|
||||
// done too.
|
||||
GC()
|
||||
|
||||
doTestParallelReaders(1)
|
||||
|
3
src/runtime/testdata/testprog/preempt.go
vendored
3
src/runtime/testdata/testprog/preempt.go
vendored
@ -21,7 +21,8 @@ func AsyncPreempt() {
|
||||
// Disable GC so we have complete control of what we're testing.
|
||||
debug.SetGCPercent(-1)
|
||||
// Out of an abundance of caution, also make sure that there are
|
||||
// no GCs actively in progress.
|
||||
// no GCs actively in progress. The sweep phase of a GC cycle
|
||||
// for instance tries to preempt Ps at the very beginning.
|
||||
runtime.GC()
|
||||
|
||||
// Start a goroutine with no sync safe-points.
|
||||
|
Loading…
Reference in New Issue
Block a user