diff --git a/src/os/signal/signal.go b/src/os/signal/signal.go index b9fe16baa50b1f..81b4dbbcce3f91 100644 --- a/src/os/signal/signal.go +++ b/src/os/signal/signal.go @@ -168,8 +168,8 @@ func Notify(c chan<- os.Signal, sig ...os.Signal) { } } -// Reset undoes the effect of any prior calls to [Notify] for the provided -// signals. +// Reset undoes the effect of any prior calls to [Notify] or [Ignore] for the +// provided signals. // If no signals are provided, all signal handlers will be reset. func Reset(sig ...os.Signal) { cancel(sig, disableSignal) diff --git a/src/os/signal/signal_test.go b/src/os/signal/signal_test.go index d54787bc199a3b..d56866723489d8 100644 --- a/src/os/signal/signal_test.go +++ b/src/os/signal/signal_test.go @@ -217,9 +217,7 @@ func testCancel(t *testing.T, ignore bool) { // Either way, this should undo both calls to Notify above. if ignore { Ignore(syscall.SIGWINCH, syscall.SIGHUP) - // Don't bother deferring a call to Reset: it is documented to undo Notify, - // but its documentation says nothing about Ignore, and (as of the time of - // writing) it empirically does not undo an Ignore. + defer Reset(syscall.SIGWINCH, syscall.SIGHUP) } else { Reset(syscall.SIGWINCH, syscall.SIGHUP) } @@ -349,6 +347,8 @@ func TestStop(t *testing.T) { for _, sig := range sigs { sig := sig t.Run(fmt.Sprint(sig), func(t *testing.T) { + defer Reset(sig) + // When calling Notify with a specific signal, // independent signals should not interfere with each other, // and we end up needing to wait for signals to quiesce a lot. @@ -913,3 +913,106 @@ func TestSignalTrace(t *testing.T) { close(quit) <-done } + +// #46321 test Reset actually undoes the effect of Ignore. +func TestResetIgnore(t *testing.T) { + if os.Getenv("GO_TEST_RESET_IGNORE") != "" { + s, err := strconv.Atoi(os.Getenv("GO_TEST_RESET_IGNORE")) + if err != nil { + t.Fatalf("failed to parse signal: %v", err) + } + if Ignored(syscall.Signal(s)) { + os.Exit(1) + } + os.Exit(0) + } + + sigs := []syscall.Signal{ + syscall.SIGHUP, + syscall.SIGINT, + syscall.SIGUSR1, + syscall.SIGTERM, + syscall.SIGCHLD, + syscall.SIGWINCH, + } + + for _, notify := range []bool{false, true} { + for _, sig := range sigs { + t.Run(fmt.Sprintf("%s[notify=%t]", sig, notify), func(t *testing.T) { + if Ignored(sig) { + t.Skipf("expected %q to not be ignored initially", sig) + } + + Ignore(sig) + if notify { + c := make(chan os.Signal, 1) + Notify(c, sig) + defer Stop(c) + } + Reset(sig) + + if Ignored(sig) { + t.Fatalf("expected %q to not be ignored", sig) + } + + // Child processes inherit the ignored status of signals, so verify that it + // is indeed not ignored. + cmd := testenv.Command(t, testenv.Executable(t), "-test.run=^TestResetIgnore$") + cmd.Env = append(os.Environ(), "GO_TEST_RESET_IGNORE="+strconv.Itoa(int(sig))) + err := cmd.Run() + if err != nil { + t.Fatalf("expected %q to not be ignored in child process: %v", sig, err) + } + }) + } + } +} + +// #46321 test Reset correctly undoes the effect of Ignore when the child +// process is started with a signal ignored. +func TestInitiallyIgnoredResetIgnore(t *testing.T) { + testenv.MustHaveExec(t) + + if os.Getenv("GO_TEST_INITIALLY_IGNORED_RESET_IGNORE") != "" { + s, err := strconv.Atoi(os.Getenv("GO_TEST_INITIALLY_IGNORED_RESET_IGNORE")) + if err != nil { + t.Fatalf("failed to parse signal: %v", err) + } + initiallyIgnoredResetIgnoreTestProgram(syscall.Signal(s)) + } + + sigs := []syscall.Signal{ + syscall.SIGINT, + syscall.SIGHUP, + } + + for _, sig := range sigs { + t.Run(fmt.Sprint(sig), func(t *testing.T) { + Ignore(sig) + defer Reset(sig) + + cmd := testenv.Command(t, testenv.Executable(t), "-test.run=^TestInitiallyIgnoredResetIgnore$") + cmd.Env = append(os.Environ(), "GO_TEST_INITIALLY_IGNORED_RESET_IGNORE="+strconv.Itoa(int(sig))) + err := cmd.Run() + if err != nil { + t.Fatalf("expected %q to be ignored in child process: %v", sig, err) + } + }) + } +} + +func initiallyIgnoredResetIgnoreTestProgram(sig os.Signal) { + if !Ignored(sig) { + os.Exit(1) + } + Reset(sig) + if !Ignored(sig) { + os.Exit(1) + } + Ignore(sig) + Reset(sig) + if !Ignored(sig) { + os.Exit(1) + } + os.Exit(0) +} diff --git a/src/runtime/os3_plan9.go b/src/runtime/os3_plan9.go index dd1570561803f8..efc5a33af71764 100644 --- a/src/runtime/os3_plan9.go +++ b/src/runtime/os3_plan9.go @@ -149,7 +149,8 @@ Exit: func sigenable(sig uint32) { } -func sigdisable(sig uint32) { +func sigdisable(sig uint32) bool { + return false } func sigignore(sig uint32) { diff --git a/src/runtime/os_wasm.go b/src/runtime/os_wasm.go index 8046caf45ef133..296e0fd860f127 100644 --- a/src/runtime/os_wasm.go +++ b/src/runtime/os_wasm.go @@ -142,7 +142,7 @@ func getfp() uintptr { return 0 } func setProcessCPUProfiler(hz int32) {} func setThreadCPUProfiler(hz int32) {} -func sigdisable(uint32) {} +func sigdisable(uint32) bool { return false } func sigenable(uint32) {} func sigignore(uint32) {} diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go index 96628d6baae910..c8dcdbc87a1d9f 100644 --- a/src/runtime/signal_unix.go +++ b/src/runtime/signal_unix.go @@ -202,23 +202,28 @@ func sigenable(sig uint32) { enableSigChan <- sig <-maskUpdatedChan if atomic.Cas(&handlingSig[sig], 0, 1) { - atomic.Storeuintptr(&fwdSig[sig], getsig(sig)) + h := getsig(sig) + if h != _SIG_IGN { + atomic.Storeuintptr(&fwdSig[sig], h) + } setsig(sig, abi.FuncPCABIInternal(sighandler)) } } } -// sigdisable disables the Go signal handler for the signal sig. +// sigdisable resets the signal handler for the signal sig back to the default +// at program startup or the last custom handler registered by cgo. // It is only called while holding the os/signal.handlers lock, // via os/signal.disableSignal and signal_disable. -func sigdisable(sig uint32) { +// Reports whether the signal is ignored after the change. +func sigdisable(sig uint32) bool { if sig >= uint32(len(sigtable)) { - return + return false } // SIGPROF is handled specially for profiling. if sig == _SIGPROF { - return + return false } t := &sigtable[sig] @@ -227,14 +232,27 @@ func sigdisable(sig uint32) { disableSigChan <- sig <-maskUpdatedChan - // If initsig does not install a signal handler for a - // signal, then to go back to the state before Notify - // we should remove the one we installed. - if !sigInstallGoHandler(sig) { + if sigInstallGoHandler(sig) { + if atomic.Cas(&handlingSig[sig], 0, 1) { + h := getsig(sig) + // Preserve custom signal handlers installed with cgo. + if h != _SIG_IGN && h != _SIG_DFL { + atomic.Storeuintptr(&fwdSig[sig], h) + } + setsig(sig, abi.FuncPCABIInternal(sighandler)) + } + return false + } else { + // If initsig does not install a signal handler for a + // signal, then to go back to the state before Notify + // we should remove the one we installed. atomic.Store(&handlingSig[sig], 0) - setsig(sig, atomic.Loaduintptr(&fwdSig[sig])) + fs := atomic.Loaduintptr(&fwdSig[sig]) + setsig(sig, fs) + return fs == _SIG_IGN } } + return false } // sigignore ignores the signal sig. diff --git a/src/runtime/signal_windows.go b/src/runtime/signal_windows.go index b0c653ee46dd0b..67d68476513a3f 100644 --- a/src/runtime/signal_windows.go +++ b/src/runtime/signal_windows.go @@ -434,7 +434,8 @@ func initsig(preinit bool) { func sigenable(sig uint32) { } -func sigdisable(sig uint32) { +func sigdisable(sig uint32) bool { + return false } func sigignore(sig uint32) { diff --git a/src/runtime/sigqueue.go b/src/runtime/sigqueue.go index 62a8e8a70226fe..299e5ee87e28ef 100644 --- a/src/runtime/sigqueue.go +++ b/src/runtime/sigqueue.go @@ -230,11 +230,19 @@ func signal_disable(s uint32) { if s >= uint32(len(sig.wanted)*32) { return } - sigdisable(s) + ignored := sigdisable(s) w := sig.wanted[s/32] w &^= 1 << (s & 31) atomic.Store(&sig.wanted[s/32], w) + + i := sig.ignored[s/32] + if ignored { + i |= 1 << (s & 31) + } else { + i &^= 1 << (s & 31) + } + atomic.Store(&sig.ignored[s/32], i) } // Must only be called from a single goroutine at a time.