[go] runtime: add an exit hook facility
407 views
Skip to first unread message
Than McIntosh (Gerrit)
Oct 8, 2021, 12:06:06 PM10/8/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Than McIntosh has uploaded this change for review.
runtime: add an exit hook facility
Add a new API (not public/exported) for registering a function with
the runtime that should be called when program execution terminates,
to be used in the new code coverage re-implementation. The API looks
like
func addExitHook(f func(), runOnNonZeroExit bool)
The first argument is the function to be run, second argument controls
whether the function is invoked even if there is a call to os.Exit
with a non-zero status. Exit hooks are run in reverse order of
registration, e.g. the first hook to be registered will be the last to
run. Exit hook functions are not allowed to panic or to make calls to
os.Exit.
Change-Id: I906f8c5184b7c1666f05a62cfc7833bf1a4300c4
---
A src/runtime/exithook.go
M src/os/proc.go
A src/runtime/testdata/testexithooks/testexithooks.go
M src/runtime/proc.go
A src/runtime/ehooks_test.go
5 files changed, 246 insertions(+), 14 deletions(-)
diff --git a/src/os/proc.go b/src/os/proc.go
index cbd5a6a..3aae568 100644
--- a/src/os/proc.go
+++ b/src/os/proc.go
@@ -60,19 +60,21 @@
//
// For portability, the status code should be in the range [0, 125].
func Exit(code int) {
- if code == 0 {
- if testlog.PanicOnExit0() {
- // We were told to panic on calls to os.Exit(0).
- // This is used to fail tests that make an early
- // unexpected call to os.Exit(0).
- panic("unexpected call to os.Exit(0) during test")
- }
-
- // Give race detector a chance to fail the program.
- // Racy programs do not have the right to finish successfully.
- runtime_beforeExit()
+ if code == 0 && testlog.PanicOnExit0() {
+ // We were told to panic on calls to os.Exit(0).
+ // This is used to fail tests that make an early
+ // unexpected call to os.Exit(0).
+ panic("unexpected call to os.Exit(0) during test")
}
+
+ // Inform the runtime that os.Exit is being called. If -race is
+ // enabled, this will give race detector a chance to fail the
+ // program (racy programs do not have the right to finish
+ // successfully). If coverage is enabled, then this call will
+ // enable us to write out a coverage data file.
+ runtime_beforeExit(code)
+
syscall.Exit(code)
}
-func runtime_beforeExit() // implemented in runtime
+func runtime_beforeExit(exitCode int) // implemented in runtime
diff --git a/src/runtime/ehooks_test.go b/src/runtime/ehooks_test.go
new file mode 100644
index 0000000..3566ee1
--- /dev/null
+++ b/src/runtime/ehooks_test.go
@@ -0,0 +1,72 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package runtime_test
+
+import (
+ "os/exec"
+ "strings"
+ "testing"
+)
+
+func TestExitHooks(t *testing.T) {
+ scenarios := []struct {
+ mode string
+ expected 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: internal error: exit hook invoked panic",
+ },
+ {mode: "callsexit",
+ expected: "",
+ musthave: "fatal error: internal error: exit hook invoked exit",
+ },
+ }
+
+ exe, err := buildTestProg(t, "testexithooks", "")
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ for _, s := range scenarios {
+ cmd := exec.Command(exe, []string{"-mode", s.mode}...)
+ out, _ := cmd.CombinedOutput()
+ outs := string(out)
+ if s.expected != "" {
+ if s.expected != outs {
+ t.Logf("raw output: %q", outs)
+ t.Errorf("failed mode %s: wanted %q got %q",
+ 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)
+ }
+ } else {
+ panic("badly written scenario")
+ }
+ }
+}
diff --git a/src/runtime/exithook.go b/src/runtime/exithook.go
new file mode 100644
index 0000000..65c59c3
--- /dev/null
+++ b/src/runtime/exithook.go
@@ -0,0 +1,46 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package runtime
+
+type hook struct {
+ f func()
+ runOnNonZeroExit bool
+}
+
+var hooks []hook
+var runningHooks bool
+
+func addExitHook(f func(), runOnNonZeroExit bool) {
+ hooks = append(hooks, hook{f: f, runOnNonZeroExit: runOnNonZeroExit})
+}
+
+func runHook(f func()) (caughtPanic bool) {
+ defer func() {
+ if x := recover(); x != nil {
+ caughtPanic = true
+ }
+ }()
+ f()
+ return
+}
+
+func runExitHooks(exitCode int) {
+ if runningHooks {
+ throw("internal error: exit hook invoked exit")
+ }
+ runningHooks = true
+ // Hooks are run in reverse order of registration: first hook added
+ // is the last one run.
+ for i := range hooks {
+ h := hooks[len(hooks)-i-1]
+ if exitCode != 0 && !h.runOnNonZeroExit {
+ continue
+ }
+ if caughtPanic := runHook(h.f); caughtPanic {
+ throw("internal error: exit hook invoked panic")
+ }
+ }
+ hooks = nil
+}
diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index aa2ba96..b9ceed4 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -273,6 +273,7 @@
if atomic.Load(&panicking) != 0 {
gopark(nil, nil, waitReasonPanicWait, traceEvGoStop, 1)
}
+ runExitHooks(0)
exit(0)
for {
@@ -283,10 +284,11 @@
// os_beforeExit is called from os.Exit(0).
//go:linkname os_beforeExit os.runtime_beforeExit
-func os_beforeExit() {
- if raceenabled {
+func os_beforeExit(exitCode int) {
+ if exitCode == 0 && raceenabled {
racefini()
}
+ runExitHooks(exitCode)
}
// start forcegc helper goroutine
diff --git a/src/runtime/testdata/testexithooks/testexithooks.go b/src/runtime/testdata/testexithooks/testexithooks.go
new file mode 100644
index 0000000..0a267ff
--- /dev/null
+++ b/src/runtime/testdata/testexithooks/testexithooks.go
@@ -0,0 +1,87 @@
+// Copyright 2021 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+ "flag"
+ "os"
+ _ "unsafe"
+)
+
+import "C"
+
+var modeflag = flag.String("mode", "", "mode to run in")
+
+func main() {
+ flag.Parse()
+ switch *modeflag {
+ case "simple":
+ testSimple()
+ case "goodexit":
+ testGoodExit()
+ case "badexit":
+ testBadExit()
+ case "panics":
+ testPanics()
+ case "callsexit":
+ testHookCallsExit()
+ default:
+ panic("unknown mode")
+ }
+}
+
+//go:linkname runtime_addExitHook runtime.addExitHook
+func runtime_addExitHook(f func(), runOnNonZeroExit bool)
+
+func testSimple() {
+ f1 := func() { println("foo") }
+ f2 := func() { println("bar") }
+ runtime_addExitHook(f1, false)
+ runtime_addExitHook(f2, false)
+ // no explicit call to os.Exit
+}
+
+func testGoodExit() {
+ f1 := func() { println("apple") }
+ f2 := func() { println("orange") }
+ runtime_addExitHook(f1, false)
+ runtime_addExitHook(f2, false)
+ // explicit call to os.Exit
+ os.Exit(0)
+}
+
+func testBadExit() {
+ f1 := func() { println("blog") }
+ f2 := func() { println("blix") }
+ f3 := func() { println("blek") }
+ f4 := func() { println("blub") }
+ f5 := func() { println("blat") }
+ runtime_addExitHook(f1, false)
+ runtime_addExitHook(f2, true)
+ runtime_addExitHook(f3, false)
+ runtime_addExitHook(f4, true)
+ runtime_addExitHook(f5, false)
+ os.Exit(1)
+}
+
+func testPanics() {
+ f1 := func() { println("ok") }
+ f2 := func() { panic("BADBADBAD") }
+ f3 := func() { println("good") }
+ runtime_addExitHook(f1, true)
+ runtime_addExitHook(f2, true)
+ runtime_addExitHook(f3, true)
+ os.Exit(0)
+}
+
+func testHookCallsExit() {
+ f1 := func() { println("ok") }
+ f2 := func() { os.Exit(1) }
+ f3 := func() { println("good") }
+ runtime_addExitHook(f1, true)
+ runtime_addExitHook(f2, true)
+ runtime_addExitHook(f3, true)
+ os.Exit(1)
+}
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
Oct 8, 2021, 12:21:39 PM10/8/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Than McIntosh uploaded patch set #2 to this change.
runtime: add an exit hook facility
DO NOT SUBMIT
Add a new API (not public/exported) for registering a function with
the runtime that should be called when program execution terminates,
to be used in the new code coverage re-implementation. The API looks
like
func addExitHook(f func(), runOnNonZeroExit bool)
The first argument is the function to be run, second argument controls
whether the function is invoked even if there is a call to os.Exit
with a non-zero status. Exit hooks are run in reverse order of
registration, e.g. the first hook to be registered will be the last to
run. Exit hook functions are not allowed to panic or to make calls to
os.Exit.
Change-Id: I906f8c5184b7c1666f05a62cfc7833bf1a4300c4
---
A src/runtime/exithook.go
M src/os/proc.go
A src/runtime/testdata/testexithooks/testexithooks.go
M src/runtime/proc.go
A src/runtime/ehooks_test.go
5 files changed, 248 insertions(+), 14 deletions(-)
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
Oct 21, 2021, 11:36:50 AM10/21/21
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Patch set 4:Run-TryBot +1Trust +1
Than McIntosh (Gerrit)
Oct 29, 2021, 10:14:11 AM10/29/21
to goph...@pubsubhelper.golang.org, Go Bot, golang-co...@googlegroups.com
Patch set 8:Run-TryBot +1Trust +1
Than McIntosh (Gerrit)
Apr 20, 2022, 7:06:39 AM4/20/22
to goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Patch set 16:Run-TryBot +1
Than McIntosh (Gerrit)
Apr 20, 2022, 8:03:09 AM4/20/22
to goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Patch set 17:Run-TryBot +1
Than McIntosh (Gerrit)
Apr 20, 2022, 9:55:17 AM4/20/22
to goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Patch set 18:Run-TryBot +1
Than McIntosh (Gerrit)
Apr 20, 2022, 2:01:10 PM4/20/22
to goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Patch set 19:Run-TryBot +1
Than McIntosh (Gerrit)
Apr 20, 2022, 8:35:42 PM4/20/22
to goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Patch set 20:Run-TryBot +1
Than McIntosh (Gerrit)
Apr 21, 2022, 11:09:13 AM4/21/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Than McIntosh uploaded patch set #22 to this change.
runtime: add an exit hook facility
Add a new API (not public/exported) for registering a function with
the runtime that should be called when program execution terminates,
to be used in the new code coverage re-implementation. The API looks
like
func addExitHook(f func(), runOnNonZeroExit bool)
The first argument is the function to be run, second argument controls
whether the function is invoked even if there is a call to os.Exit
with a non-zero status. Exit hooks are run in reverse order of
registration, e.g. the first hook to be registered will be the last to
run. Exit hook functions are not allowed to panic or to make calls to
os.Exit.
Change-Id: I906f8c5184b7c1666f05a62cfc7833bf1a4300c4
---
M src/os/proc.go
A src/runtime/ehooks_test.go
A src/runtime/exithook.go
M src/runtime/proc.go
A src/runtime/testdata/testexithooks/testexithooks.go
5 files changed, 246 insertions(+), 14 deletions(-)
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
Apr 22, 2022, 2:45:27 PM4/22/22
to goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Than McIntosh (Gerrit)
Apr 22, 2022, 2:45:30 PM4/22/22
to goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Patch set 25:Run-TryBot +1
Than McIntosh (Gerrit)
Apr 22, 2022, 2:50:53 PM4/22/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Than McIntosh uploaded patch set #26 to this change.
runtime: add an exit hook facility
DO NOT SUBMIT
Add a new API (not public/exported) for registering a function with
the runtime that should be called when program execution terminates,
to be used in the new code coverage re-implementation. The API looks
like
func addExitHook(f func(), runOnNonZeroExit bool)
The first argument is the function to be run, second argument controls
whether the function is invoked even if there is a call to os.Exit
with a non-zero status. Exit hooks are run in reverse order of
registration, e.g. the first hook to be registered will be the last to
run. Exit hook functions are not allowed to panic or to make calls to
os.Exit.
Change-Id: I906f8c5184b7c1666f05a62cfc7833bf1a4300c4
---
M src/os/proc.go
A src/runtime/ehooks_test.go
A src/runtime/exithook.go
M src/runtime/proc.go
A src/runtime/testdata/testexithooks/testexithooks.go
5 files changed, 259 insertions(+), 14 deletions(-)
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
Apr 22, 2022, 3:25:41 PM4/22/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Than McIntosh.
Than McIntosh uploaded patch set #27 to this change.
runtime: add an exit hook facility
DO NOT SUBMIT
Add a new API (not public/exported) for registering a function with
the runtime that should be called when program execution terminates,
to be used in the new code coverage re-implementation. The API looks
like
func addExitHook(f func(), runOnNonZeroExit bool)
The first argument is the function to be run, second argument controls
whether the function is invoked even if there is a call to os.Exit
with a non-zero status. Exit hooks are run in reverse order of
registration, e.g. the first hook to be registered will be the last to
run. Exit hook functions are not allowed to panic or to make calls to
os.Exit.
Change-Id: I906f8c5184b7c1666f05a62cfc7833bf1a4300c4
---
M src/os/proc.go
A src/runtime/ehooks_test.go
A src/runtime/exithook.go
M src/runtime/proc.go
A src/runtime/testdata/testexithooks/testexithooks.go
5 files changed, 265 insertions(+), 14 deletions(-)
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
Apr 22, 2022, 3:51:38 PM4/22/22
to goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Patch set 27:Run-TryBot +1
Than McIntosh (Gerrit)
Apr 27, 2022, 10:52:34 AM4/27/22
to goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Patch set 28:Run-TryBot +1
Michael Knyszek (Gerrit)
Apr 27, 2022, 9:10:15 PM4/27/22
to Than McIntosh, goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Than McIntosh.
9 comments:
Patchset:
looks good overall. just some naming nits and requests for additional documentation.
File src/runtime/exithook.go:
nit: exitHook (just for namespacing and clarity)
please add documentation for the fields and type
maybe group these globals into a "var" block?
Patch Set #28, Line 12:
hooksnit: exitHooks
Patch Set #28, Line 13:
runningHooksnit: runningExitHooks
Patch Set #28, Line 19:
runHookwhat do you think of defining this as a helper in runExitHooks (without closing over anything, I mean)?
// Hooks are run in reverse order of registration: first hook added
// is the last one run.
since this function is short, I think you could lift this up into documentation for runExitHooks (and also add a sentence about what it does and why)
File src/runtime/proc.go:
Patch Set #23, Line 252:
runExitHooks(0)is this necessary given that it's called again below?
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
Apr 28, 2022, 10:30:16 AM4/28/22
to goph...@pubsubhelper.golang.org, Michael Knyszek, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Michael Knyszek.
8 comments:
File src/runtime/exithook.go:
nit: exitHook (just for namespacing and clarity)
Done
please add documentation for the fields and type
Done
Patch Set #28, Line 12:
hooksnit: exitHooks
Done
maybe group these globals into a "var" block?
Done
Patch Set #28, Line 13:
runningHooksnit: runningExitHooks
Done
Patch Set #28, Line 19:
runHook
what do you think of defining this as a helper in runExitHooks (without closing over anything, I mea […]
Done
// Hooks are run in reverse order of registration: first hook added
// is the last one run.
since this function is short, I think you could lift this up into documentation for runExitHooks (an […]
Done
File src/runtime/proc.go:
Patch Set #23, Line 252:
runExitHooks(0)is this necessary given that it's called again below?
It is needed. racefini() does not return; it calls __tsan_fini which calls C.exit. I'll add a comment to make this more clear.
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
Apr 28, 2022, 10:34:15 AM4/28/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Michael Knyszek, Than McIntosh.
Than McIntosh uploaded patch set #29 to this change.
runtime: add an exit hook facility
Add a new API (not public/exported) for registering a function with
the runtime that should be called when program execution terminates,
to be used in the new code coverage re-implementation. The API looks
like
func addExitHook(f func(), runOnNonZeroExit bool)
The first argument is the function to be run, second argument controls
whether the function is invoked even if there is a call to os.Exit
with a non-zero status. Exit hooks are run in reverse order of
registration, e.g. the first hook to be registered will be the last to
run. Exit hook functions are not allowed to panic or to make calls to
os.Exit.
Change-Id: I906f8c5184b7c1666f05a62cfc7833bf1a4300c4
---
M src/os/proc.go
A src/runtime/ehooks_test.go
A src/runtime/exithook.go
M src/runtime/proc.go
A src/runtime/testdata/testexithooks/testexithooks.go
5 files changed, 277 insertions(+), 14 deletions(-)
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
Apr 28, 2022, 10:35:12 AM4/28/22
to goph...@pubsubhelper.golang.org, Michael Knyszek, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Michael Knyszek.
Patch set 29:Run-TryBot +1
Michael Knyszek (Gerrit)
Apr 29, 2022, 2:29:46 PM4/29/22
to Than McIntosh, goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Attention is currently required from: Than McIntosh.
Patch set 29:Code-Review +2
2 comments:
Patchset:
LGTM. just one comment about additional documentation, but after resolving that, no need to wait on me.
File src/runtime/exithook.go:
Patch Set #29, Line 11:
addExitHookgiven your next CL, and how exit hooks might be running higher-level Go code, I think it's worth being explicit that f will always run on a regular goroutine. that, and addExitHook should only be called from a regular goroutine (preemption enabled, write barriers allowed, has a P, etc.). I don't really know how to succinctly and precisely specify that, but I think "regular goroutine" or "user goroutine" gets close.
because this is a general mechanism, I just want to be clear that if someone wants to be able to, say, run exist hooks when some kind of user crash happens or something, that we need to be careful to always run it from a safe context (sometimes we discover the crash in an unsafe context, like in a signal handler).
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
May 3, 2022, 9:54:03 AM5/3/22
to goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
1 comment:
File src/runtime/exithook.go:
Patch Set #29, Line 11:
addExitHook
given your next CL, and how exit hooks might be running higher-level Go code, I think it's worth bei […]
I've updated the comment with language to this effect.
Might also add that I am hoping/anticipating that there won't be a huge number of new uses of addExitHook in the future (could be wrong, but that would be my guess).
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
May 3, 2022, 9:54:52 AM5/3/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Than McIntosh uploaded patch set #32 to this change.
runtime: add an exit hook facility
Add a new API (not public/exported) for registering a function with
the runtime that should be called when program execution terminates,
to be used in the new code coverage re-implementation. The API looks
like
func addExitHook(f func(), runOnNonZeroExit bool)
The first argument is the function to be run, second argument controls
whether the function is invoked even if there is a call to os.Exit
with a non-zero status. Exit hooks are run in reverse order of
registration, e.g. the first hook to be registered will be the last to
run. Exit hook functions are not allowed to panic or to make calls to
os.Exit.
Change-Id: I906f8c5184b7c1666f05a62cfc7833bf1a4300c4
---
M src/os/proc.go
A src/runtime/ehooks_test.go
A src/runtime/exithook.go
M src/runtime/proc.go
A src/runtime/testdata/testexithooks/testexithooks.go
5 files changed, 286 insertions(+), 14 deletions(-)
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
May 3, 2022, 9:55:06 AM5/3/22
to goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Patch set 32:Run-TryBot +1
Than McIntosh (Gerrit)
May 5, 2022, 1:21:07 PM5/5/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Than McIntosh.
Than McIntosh uploaded patch set #35 to this change.
runtime: add an exit hook facility
Add a new API (not public/exported) for registering a function with
the runtime that should be called when program execution terminates,
to be used in the new code coverage re-implementation. The API looks
like
func addExitHook(f func(), runOnNonZeroExit bool)
The first argument is the function to be run, second argument controls
whether the function is invoked even if there is a call to os.Exit
with a non-zero status. Exit hooks are run in reverse order of
registration, e.g. the first hook to be registered will be the last to
run. Exit hook functions are not allowed to panic or to make calls to
os.Exit.
Updates #51430.
Change-Id: I906f8c5184b7c1666f05a62cfc7833bf1a4300c4
---
M src/os/proc.go
A src/runtime/ehooks_test.go
A src/runtime/exithook.go
M src/runtime/proc.go
A src/runtime/testdata/testexithooks/testexithooks.go
5 files changed, 288 insertions(+), 14 deletions(-)
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
May 6, 2022, 11:16:51 AM5/6/22
to goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Patch set 37:Run-TryBot +1
Than McIntosh (Gerrit)
May 6, 2022, 1:37:51 PM5/6/22
to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com
Attention is currently required from: Than McIntosh.
Than McIntosh uploaded patch set #38 to this change.
runtime: add an exit hook facility
Add a new API (not public/exported) for registering a function with
the runtime that should be called when program execution terminates,
to be used in the new code coverage re-implementation. The API looks
like
func addExitHook(f func(), runOnNonZeroExit bool)
The first argument is the function to be run, second argument controls
whether the function is invoked even if there is a call to os.Exit
with a non-zero status. Exit hooks are run in reverse order of
registration, e.g. the first hook to be registered will be the last to
run. Exit hook functions are not allowed to panic or to make calls to
os.Exit.
Updates #51430.
Change-Id: I906f8c5184b7c1666f05a62cfc7833bf1a4300c4
---
M src/os/proc.go
A src/runtime/ehooks_test.go
A src/runtime/exithook.go
M src/runtime/proc.go
A src/runtime/testdata/testexithooks/testexithooks.go
5 files changed, 287 insertions(+), 14 deletions(-)
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
Sep 26, 2022, 4:28:46 PM9/26/22
to goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
Patch set 55:Run-TryBot +1
Than McIntosh (Gerrit)
Sep 26, 2022, 5:49:19 PM9/26/22
to goph...@pubsubhelper.golang.org, golang-...@googlegroups.com, Gopher Robot, Michael Knyszek, golang-co...@googlegroups.com
Than McIntosh submitted this change.
29 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/runtime/exithook.go
Insertions: 8, Deletions: 0.
The diff is too large to show. Please review the diff.
```
Approvals:
Gopher Robot: TryBots succeeded
Than McIntosh: Run TryBots
Michael Knyszek: Looks good to me, approved
runtime: add an exit hook facility
Add a new API (not public/exported) for registering a function with
the runtime that should be called when program execution terminates,
to be used in the new code coverage re-implementation. The API looks
like
func addExitHook(f func(), runOnNonZeroExit bool)
The first argument is the function to be run, second argument controls
whether the function is invoked even if there is a call to os.Exit
with a non-zero status. Exit hooks are run in reverse order of
registration, e.g. the first hook to be registered will be the last to
run. Exit hook functions are not allowed to panic or to make calls to
os.Exit.
Updates #51430.
Change-Id: I906f8c5184b7c1666f05a62cfc7833bf1a4300c4
Reviewed-on: https://go-review.googlesource.com/c/go/+/354790
TryBot-Result: Gopher Robot <go...@golang.org>
Run-TryBot: Than McIntosh <th...@google.com>
Reviewed-by: Michael Knyszek <mkny...@google.com>
---
M src/os/proc.go
A src/runtime/ehooks_test.go
A src/runtime/exithook.go
M src/runtime/proc.go
A src/runtime/testdata/testexithooks/testexithooks.go
5 files changed, 291 insertions(+), 14 deletions(-)
diff --git a/src/os/proc.go b/src/os/proc.go
index cbd5a6a..3aae568 100644
--- a/src/os/proc.go
+++ b/src/os/proc.go
@@ -60,19 +60,21 @@
//
// For portability, the status code should be in the range [0, 125].
func Exit(code int) {
- if code == 0 {
- if testlog.PanicOnExit0() {
- // We were told to panic on calls to os.Exit(0).
- // This is used to fail tests that make an early
- // unexpected call to os.Exit(0).
- panic("unexpected call to os.Exit(0) during test")
- }
-
- // Give race detector a chance to fail the program.
- // Racy programs do not have the right to finish successfully.
- runtime_beforeExit()
+ if code == 0 && testlog.PanicOnExit0() {
+ // We were told to panic on calls to os.Exit(0).
+ // This is used to fail tests that make an early
+ // unexpected call to os.Exit(0).
+ panic("unexpected call to os.Exit(0) during test")
}
+
+ // Inform the runtime that os.Exit is being called. If -race is
+ // enabled, this will give race detector a chance to fail the
+ // program (racy programs do not have the right to finish
+ // successfully). If coverage is enabled, then this call will
+ // enable us to write out a coverage data file.
+ runtime_beforeExit(code)
+
syscall.Exit(code)
}
-func runtime_beforeExit() // implemented in runtime
+func runtime_beforeExit(exitCode int) // implemented in runtime
diff --git a/src/runtime/ehooks_test.go b/src/runtime/ehooks_test.go
new file mode 100644
index 0000000..021495f
--- /dev/null
+++ b/src/runtime/ehooks_test.go
@@ -0,0 +1,88 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package runtime_test
+
+import (
+ "os/exec"
+ "runtime"
+ "strings"
+ "testing"
+)
+
+func TestExitHooks(t *testing.T) {
+ bmodes := []string{"", "-race"}
+ if !testing.Short() {
+ bmodes = append(bmodes, "-race")
+ }
+ for _, bmode := range bmodes {
+ // Race detector is not supported everywhere -- limit to just
+ // amd64 to keep things simple.
+ if bmode == "-race" && runtime.GOARCH != "amd64" {
+ t.Skipf("Skipping on %s/%s", runtime.GOOS, runtime.GOARCH)
+ }
+ scenarios := []struct {
+ mode string
+ expected 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: internal error: exit hook invoked panic",
+ },
+ {
+ mode: "callsexit",
+ expected: "",
+ musthave: "fatal error: internal error: exit hook invoked exit",
+ },
+ }
+
+ exe, err := buildTestProg(t, "testexithooks", bmode)
+ if err != nil {
+ t.Fatal(err)
+ }
+
+ bt := ""
+ if bmode != "" {
+ bt = " bmode: " + bmode
+ }
+ for _, s := range scenarios {
+ cmd := exec.Command(exe, []string{"-mode", s.mode}...)
+ 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)
+ }
+ } else {
+ panic("badly written scenario")
+ }
+ }
+ }
+}
diff --git a/src/runtime/exithook.go b/src/runtime/exithook.go
new file mode 100644
index 0000000..a94d4ed
--- /dev/null
+++ b/src/runtime/exithook.go
@@ -0,0 +1,68 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package runtime
+
+// addExitHook registers the specified function 'f' to be run at
+// program termination (e.g. when someone invokes os.Exit(), or when
+// main.main returns). Hooks are run in reverse order of registration:
+// first hook added is the last one run.
+//
+// CAREFUL: the expectation is that addExitHook should only be called
+// from a safe context (e.g. not an error/panic path or signal
+// handler, preemption enabled, allocation allowed, write barriers
+// allowed, etc), and that the exit function 'f' will be invoked under
+// similar circumstances. That is the say, we are expecting that 'f'
+// uses normal / high-level Go code as opposed to one of the more
+// restricted dialects used for the trickier parts of the runtime.
+func addExitHook(f func(), runOnNonZeroExit bool) {
+ exitHooks.hooks = append(exitHooks.hooks, exitHook{f: f, runOnNonZeroExit: runOnNonZeroExit})
+}
+
+// exitHook stores a function to be run on program exit, registered
+// by the utility runtime.addExitHook.
+type exitHook struct {
+ f func() // func to run
+ runOnNonZeroExit bool // whether to run on non-zero exit code
+}
+
+// exitHooks stores state related to hook functions registered to
+// run when program execution terminates.
+var exitHooks struct {
+ hooks []exitHook
+ runningExitHooks bool
+}
+
+// runExitHooks runs any registered exit hook functions (funcs
+// previously registered using runtime.addExitHook). Here 'exitCode'
+// is the status code being passed to os.Exit, or zero if the program
+// is terminating normally without calling os.Exit).
+func runExitHooks(exitCode int) {
+ if exitHooks.runningExitHooks {
+ throw("internal error: exit hook invoked exit")
+ }
+ exitHooks.runningExitHooks = true
+
+ runExitHook := func(f func()) (caughtPanic bool) {
+ defer func() {
+ if x := recover(); x != nil {
+ caughtPanic = true
+ }
+ }()
+ f()
+ return
+ }
+
+ for i := range exitHooks.hooks {
+ h := exitHooks.hooks[len(exitHooks.hooks)-i-1]
+ if exitCode != 0 && !h.runOnNonZeroExit {
+ continue
+ }
+ if caughtPanic := runExitHook(h.f); caughtPanic {
+ throw("internal error: exit hook invoked panic")
+ }
+ }
+ exitHooks.hooks = nil
+ exitHooks.runningExitHooks = false
+}
diff --git a/src/runtime/proc.go b/src/runtime/proc.go
index 2986a30..62e96e3 100644
--- a/src/runtime/proc.go
+++ b/src/runtime/proc.go
@@ -249,6 +249,7 @@
fn := main_main // make an indirect call, as the linker doesn't know the address of the main package when laying down the runtime
fn()
if raceenabled {
+ runExitHooks(0) // run hooks now, since racefini does not return
racefini()
}
@@ -268,6 +269,7 @@
if panicking.Load() != 0 {
gopark(nil, nil, waitReasonPanicWait, traceEvGoStop, 1)
}
+ runExitHooks(0)
exit(0)
for {
@@ -279,8 +281,9 @@
// os_beforeExit is called from os.Exit(0).
//
//go:linkname os_beforeExit os.runtime_beforeExit
-func os_beforeExit() {
- if raceenabled {
+func os_beforeExit(exitCode int) {
+ runExitHooks(exitCode)
+ if exitCode == 0 && raceenabled {
racefini()
}
}
diff --git a/src/runtime/testdata/testexithooks/testexithooks.go b/src/runtime/testdata/testexithooks/testexithooks.go
new file mode 100644
index 0000000..57561fa
--- /dev/null
+++ b/src/runtime/testdata/testexithooks/testexithooks.go
@@ -0,0 +1,87 @@
+// Copyright 2022 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+ "flag"
+ "os"
+ _ "unsafe"
+)
+
+import "C"
+
+var modeflag = flag.String("mode", "", "mode to run in")
+
+func main() {
+ flag.Parse()
+ switch *modeflag {
+ case "simple":
+ testSimple()
+ case "goodexit":
+ testGoodExit()
+ case "badexit":
+ testBadExit()
+ case "panics":
+ testPanics()
+ case "callsexit":
+ testHookCallsExit()
+ default:
+ panic("unknown mode")
+ }
+}
+
+//go:linkname runtime_addExitHook runtime.addExitHook
+func runtime_addExitHook(f func(), runOnNonZeroExit bool)
+
+func testSimple() {
+ f1 := func() { println("foo") }
+ f2 := func() { println("bar") }
+ runtime_addExitHook(f1, false)
+ runtime_addExitHook(f2, false)
+ // no explicit call to os.Exit
+}
+
+func testGoodExit() {
+ f1 := func() { println("apple") }
+ f2 := func() { println("orange") }
+ runtime_addExitHook(f1, false)
+ runtime_addExitHook(f2, false)
+ // explicit call to os.Exit
+ os.Exit(0)
+}
+
+func testBadExit() {
+ f1 := func() { println("blog") }
+ f2 := func() { println("blix") }
+ f3 := func() { println("blek") }
+ f4 := func() { println("blub") }
+ f5 := func() { println("blat") }
+ runtime_addExitHook(f1, false)
+ runtime_addExitHook(f2, true)
+ runtime_addExitHook(f3, false)
+ runtime_addExitHook(f4, true)
+ runtime_addExitHook(f5, false)
+ os.Exit(1)
+}
+
+func testPanics() {
+ f1 := func() { println("ok") }
+ f2 := func() { panic("BADBADBAD") }
+ f3 := func() { println("good") }
+ runtime_addExitHook(f1, true)
+ runtime_addExitHook(f2, true)
+ runtime_addExitHook(f3, true)
+ os.Exit(0)
+}
+
+func testHookCallsExit() {
+ f1 := func() { println("ok") }
+ f2 := func() { os.Exit(1) }
+ f3 := func() { println("good") }
+ runtime_addExitHook(f1, true)
+ runtime_addExitHook(f2, true)
+ runtime_addExitHook(f3, true)
+ os.Exit(1)
+}
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
Sep 26, 2022, 8:57:27 PM9/26/22
to goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
1 comment:
Patchset:
Sent http://go.dev/cl/c/go/+/434935?forceReload=true to fix nocgo builder failures caused by this CL.
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Alan Donovan (Gerrit)
Sep 27, 2022, 10:11:10 AM9/27/22
to Than McIntosh, goph...@pubsubhelper.golang.org, Gopher Robot, golang-co...@googlegroups.com
1 comment:
Patchset:
Late to the party, but:
If I recall correctly, Go never had an equivalent of C's atexit(3) function because it was felt that it was a hack, and it tended to lead to programs that relied upon graceful shutdown rather than being robust against unexpected failures such as power cut, container termination, etc. I understand that this feature is intended for use only in coverage, but I suspect it will be a source of temptation for users who know about the linkname hack.
I wonder: were alternative implementations considered and rejected? For example, perhaps the counters could be located in a memory mapping shared with a child process that detects termination of the parent via socket disconnect. Such an implementation would not require any new features in the runtime.
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Than McIntosh (Gerrit)
Sep 27, 2022, 10:24:35 AM9/27/22
to goph...@pubsubhelper.golang.org, Alan Donovan, Gopher Robot, golang-co...@googlegroups.com
1 comment:
Patchset:
Late to the party, but: […]
We thought about various schemes but never settled on anything that seemed as though it would be robust across all the various supported architectures (windows, android, wasm, etc).
The idea that seemed to get the closest was to open the counter data file and mmap() it into the instrumented program's counter segment, so that as the running program wrote to its counters it would be emitting its counter data. That idea never quite got off the ground however, since we do support GOOS's that do not provide mmap.
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Austin Clements (Gerrit)
Sep 27, 2022, 11:35:30 AM9/27/22
to Than McIntosh, goph...@pubsubhelper.golang.org, Austin Clements, Alan Donovan, Gopher Robot, golang-co...@googlegroups.com
1 comment:
Patchset:
We thought about various schemes but never settled on anything that seemed as though it would be rob […]
I'm not so concerned about the hook, but I am concerned that this means if a program exits by panicking (just a regular, unrecovered user panic), that it won't get any coverage data.
To view, visit change 354790. To unsubscribe, or for help writing mail filters, visit settings.
Reply all
Reply to author
Forward
0 new messages