From f85c40438fea862be03d2de4b58ed3afe7cfe033 Mon Sep 17 00:00:00 2001
From: Russ Cox <rsc@golang.org>
Date: Fri, 24 May 2024 09:33:45 -0400
Subject: internal/runtime/exithook: make safe for concurrent os.Exit

Real programs can call os.Exit concurrently from multiple goroutines.
Make internal/runtime/exithook not crash in that case.

The throw on panic also now runs in the deferred context,
so that we will see the full stack trace that led to the panic.
That should give us more visibility into the flaky failures on
bugs #55167 and #56197 as well.

Fixes #67631.

Change-Id: Iefdf71b3a3b52a793ca88d89a9c270eb50ece094
Reviewed-on: https://go-review.googlesource.com/c/go/+/588235
Reviewed-by: Than McIntosh <thanm@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Auto-Submit: Russ Cox <rsc@golang.org>
---
 src/runtime/ehooks_test.go                         | 46 +++++++++++-----------
 src/runtime/proc.go                                | 10 +++--
 .../testdata/testexithooks/testexithooks.go        | 14 ++++++-
 3 files changed, 44 insertions(+), 26 deletions(-)

(limited to 'src/runtime')

diff --git a/src/runtime/ehooks_test.go b/src/runtime/ehooks_test.go
index 2265256a0b..4beb20b0be 100644
--- a/src/runtime/ehooks_test.go
+++ b/src/runtime/ehooks_test.go
@@ -28,32 +28,36 @@ func TestExitHooks(t *testing.T) {
 		scenarios := []struct {
 			mode     string
 			expected string
-			musthave string
+			musthave []string
 		}{
 			{
 				mode:     "simple",
 				expected: "bar foo",
-				musthave: "",
 			},
 			{
 				mode:     "goodexit",
 				expected: "orange apple",
-				musthave: "",
 			},
 			{
 				mode:     "badexit",
 				expected: "blub blix",
-				musthave: "",
 			},
 			{
-				mode:     "panics",
-				expected: "",
-				musthave: "fatal error: exit hook invoked panic",
+				mode: "panics",
+				musthave: []string{
+					"fatal error: exit hook invoked panic",
+					"main.testPanics",
+				},
+			},
+			{
+				mode: "callsexit",
+				musthave: []string{
+					"fatal error: exit hook invoked exit",
+				},
 			},
 			{
-				mode:     "callsexit",
+				mode:     "exit2",
 				expected: "",
-				musthave: "fatal error: exit hook invoked exit",
 			},
 		}
 
@@ -71,20 +75,18 @@ func TestExitHooks(t *testing.T) {
 			out, _ := cmd.CombinedOutput()
 			outs := strings.ReplaceAll(string(out), "\n", " ")
 			outs = strings.TrimSpace(outs)
-			if s.expected != "" {
-				if s.expected != outs {
-					t.Logf("raw output: %q", outs)
-					t.Errorf("failed%s mode %s: wanted %q got %q", bt,
-						s.mode, s.expected, outs)
-				}
-			} else if s.musthave != "" {
-				if !strings.Contains(outs, s.musthave) {
-					t.Logf("raw output: %q", outs)
-					t.Errorf("failed mode %s: output does not contain %q",
-						s.mode, s.musthave)
+			if s.expected != "" && s.expected != outs {
+				t.Fatalf("failed%s mode %s: wanted %q\noutput:\n%s", bt,
+					s.mode, s.expected, outs)
+			}
+			for _, need := range s.musthave {
+				if !strings.Contains(outs, need) {
+					t.Fatalf("failed mode %s: output does not contain %q\noutput:\n%s",
+						s.mode, need, outs)
 				}
-			} else {
-				panic("badly written scenario")
+			}
+			if s.expected == "" && s.musthave == nil && outs != "" {
+				t.Errorf("failed mode %s: wanted no output\noutput:\n%s", s.mode, outs)
 			}
 		}
 	}
diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index c5bf537a75..17b2e4d9c2 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -310,10 +310,14 @@ func os_beforeExit(exitCode int) {
 	}
 }
 
+func init() {
+	exithook.Gosched = Gosched
+	exithook.Goid = func() uint64 { return getg().goid }
+	exithook.Throw = throw
+}
+
 func runExitHooks(code int) {
-	if err := exithook.Run(code); err != nil {
-		throw(err.Error())
-	}
+	exithook.Run(code)
 }
 
 // start forcegc helper goroutine
diff --git a/src/runtime/testdata/testexithooks/testexithooks.go b/src/runtime/testdata/testexithooks/testexithooks.go
index 151b5dc62b..d734aacb2d 100644
--- a/src/runtime/testdata/testexithooks/testexithooks.go
+++ b/src/runtime/testdata/testexithooks/testexithooks.go
@@ -6,8 +6,9 @@ package main
 
 import (
 	"flag"
-	"os"
 	"internal/runtime/exithook"
+	"os"
+	"time"
 	_ "unsafe"
 )
 
@@ -26,6 +27,8 @@ func main() {
 		testPanics()
 	case "callsexit":
 		testHookCallsExit()
+	case "exit2":
+		testExit2()
 	default:
 		panic("unknown mode")
 	}
@@ -81,3 +84,12 @@ func testHookCallsExit() {
 	exithook.Add(exithook.Hook{F: f3, RunOnFailure: true})
 	os.Exit(1)
 }
+
+func testExit2() {
+	f1 := func() { time.Sleep(100 * time.Millisecond) }
+	exithook.Add(exithook.Hook{F: f1})
+	for range 10 {
+		go os.Exit(0)
+	}
+	os.Exit(0)
+}
-- 
cgit v1.3