Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os/signal: let Reset reset Ignored signals #71708

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/os/signal/signal.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
109 changes: 106 additions & 3 deletions src/os/signal/signal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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)
}
3 changes: 2 additions & 1 deletion src/runtime/os3_plan9.go
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ Exit:
func sigenable(sig uint32) {
}

func sigdisable(sig uint32) {
func sigdisable(sig uint32) bool {
return false
}

func sigignore(sig uint32) {
Expand Down
2 changes: 1 addition & 1 deletion src/runtime/os_wasm.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {}

Expand Down
38 changes: 28 additions & 10 deletions src/runtime/signal_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand All @@ -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.
Expand Down
3 changes: 2 additions & 1 deletion src/runtime/signal_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
10 changes: 9 additions & 1 deletion src/runtime/sigqueue.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down