Skip to content

Commit 89dee70

Browse files
mknyszekgopherbot
authored andcommitted
runtime: prioritize panic output over racefini
For some reason CL 646198 uncovered #3934 and #20018 again, but only in race mode. It turns out that because racefini does not return, and racefini is called early after main returns, we would not properly wait for a concurrent panic to complete. This would result in fairly consistent failures of TestPanicRace, which specifically looks for the panic output to appear if main concurrently exits. The important part of this change is that race mode will no longer have the bug described in #3934 and #20018. A byproduct, however, is that racefini is that we're essentially prioritizing the panic output over racefini in this scenario. If racefini were to reveal a latent race condition and fail, we'll prefer to surface the panic. Such a case is probably fine, because the panic is always an crashing, unrecoverable panic. For #3934. For #20018. Change-Id: I0674a75c918563c5ec4ee1eec057dfd096fcfbc8 Reviewed-on: https://go-review.googlesource.com/c/go/+/691795 LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Auto-Submit: Michael Knyszek <mknyszek@google.com> Reviewed-by: Michael Pratt <mpratt@google.com>
1 parent 8683bb8 commit 89dee70

File tree

2 files changed

+4
-10
lines changed

2 files changed

+4
-10
lines changed

src/runtime/crash_test.go

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -788,9 +788,6 @@ func TestPanicInlined(t *testing.T) {
788788
// Test for issues #3934 and #20018.
789789
// We want to delay exiting until a panic print is complete.
790790
func TestPanicRace(t *testing.T) {
791-
if race.Enabled {
792-
t.Skip("racefini exits program before main can delay exiting")
793-
}
794791
testenv.MustHaveGoRun(t)
795792

796793
exe, err := buildTestProg(t, "testprog")

src/runtime/proc.go

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -288,20 +288,14 @@ func main() {
288288
fn := main_main // make an indirect call, as the linker doesn't know the address of the main package when laying down the runtime
289289
fn()
290290

291-
exitHooksRun := false
292-
if raceenabled {
293-
runExitHooks(0) // run hooks now, since racefini does not return
294-
exitHooksRun = true
295-
racefini()
296-
}
297-
298291
// Check for C memory leaks if using ASAN and we've made cgo calls,
299292
// or if we are running as a library in a C program.
300293
// We always make one cgo call, above, to notify_runtime_init_done,
301294
// so we ignore that one.
302295
// No point in leak checking if no cgo calls, since leak checking
303296
// just looks for objects allocated using malloc and friends.
304297
// Just checking iscgo doesn't help because asan implies iscgo.
298+
exitHooksRun := false
305299
if asanenabled && (isarchive || islibrary || NumCgoCall() > 1) {
306300
runExitHooks(0) // lsandoleakcheck may not return
307301
exitHooksRun = true
@@ -327,6 +321,9 @@ func main() {
327321
if !exitHooksRun {
328322
runExitHooks(0)
329323
}
324+
if raceenabled {
325+
racefini() // does not return
326+
}
330327

331328
exit(0)
332329
for {

0 commit comments

Comments
 (0)