[go] runtime: call runtime.GC in several tests that disable GC

[Michael Knyszek (Gerrit)]

Michael Knyszek submitted this change.

View Change

Approvals: David Chase: Looks good to me, approved Michael Pratt: Looks good to me, approved Michael Knyszek: Trusted; Run TryBots Gopher Robot: TryBots succeeded

runtime: call runtime.GC in several tests that disable GC

These tests disable GC because of the potential for a deadlock, but
don't consider that a GC could be in progress due to other tests. The
likelihood of this case was increased when the minimum heap size was
lowered during the Go 1.18 cycle. The issue was then mitigated by
CL 368137 but in theory is always a problem.

This change is intended specifically for #45867, but I just walked over
a whole bunch of other tests that don't take this precaution where it
seems like it could be relevant (some tests it's not, like the
UserForcedGC test, or testprogs where no other code has run before it).

Fixes #45867.

Change-Id: I6a1b4ae73e05cab5a0b2d2cce14126bd13be0ba5
Reviewed-on: https://go-review.googlesource.com/c/go/+/369747
Reviewed-by: Michael Pratt <mpr...@google.com>
Reviewed-by: David Chase <drc...@google.com>
Trust: Michael Knyszek <mkny...@google.com>
Run-TryBot: Michael Knyszek <mkny...@google.com>
TryBot-Result: Gopher Robot <go...@golang.org>
---
M src/runtime/testdata/testprog/badtraceback.go
M src/runtime/testdata/testprog/preempt.go
M src/runtime/proc_test.go
3 files changed, 43 insertions(+), 0 deletions(-)

diff --git a/src/runtime/proc_test.go b/src/runtime/proc_test.go
index 53cafe8..9198022 100644
--- a/src/runtime/proc_test.go
+++ b/src/runtime/proc_test.go
@@ -119,6 +119,9 @@
 	// 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.
+	runtime.GC()
 	for try := 0; try < N; try++ {
 		done := make(chan bool)
 		x := uint32(0)
@@ -163,6 +166,9 @@
 	// 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.
+	runtime.GC()
 	for try := 0; try < N; try++ {
 		if load {
 			// Create P goroutines and wait until they all run.
@@ -623,6 +629,9 @@
 	// 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.
+	runtime.GC()
 
 	iters := int(1e5)
 	if testing.Short() {
diff --git a/src/runtime/testdata/testprog/badtraceback.go b/src/runtime/testdata/testprog/badtraceback.go
index d558adc..09aa2b8 100644
--- a/src/runtime/testdata/testprog/badtraceback.go
+++ b/src/runtime/testdata/testprog/badtraceback.go
@@ -17,6 +17,9 @@
 func BadTraceback() {
 	// Disable GC to prevent traceback at unexpected time.
 	debug.SetGCPercent(-1)
+	// Out of an abundance of caution, also make sure that there are
+	// no GCs actively in progress.
+	runtime.GC()
 
 	// Run badLR1 on its own stack to minimize the stack size and
 	// exercise the stack bounds logic in the hex dump.
diff --git a/src/runtime/testdata/testprog/preempt.go b/src/runtime/testdata/testprog/preempt.go
index 1c74d0e..eb9f590 100644
--- a/src/runtime/testdata/testprog/preempt.go
+++ b/src/runtime/testdata/testprog/preempt.go
@@ -20,6 +20,9 @@
 	runtime.GOMAXPROCS(1)
 	// 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.
+	runtime.GC()
 
 	// Start a goroutine with no sync safe-points.
 	var ready, ready2 uint32

To view, visit change 369747. To unsubscribe, or for help writing mail filters, visit settings.

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I6a1b4ae73e05cab5a0b2d2cce14126bd13be0ba5

Gerrit-Change-Number: 369747

Gerrit-PatchSet: 3

Gerrit-Owner: Michael Knyszek <mkny...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Gopher Robot <go...@golang.org>

Gerrit-Reviewer: Michael Knyszek <mkny...@google.com>

Gerrit-Reviewer: Michael Pratt <mpr...@google.com>

Gerrit-MessageType: merged