From: Ian Lance Taylor Date: Sat, 4 Feb 2017 05:13:30 +0000 (-0800) Subject: runtime: use atomic ops for fwdSig, make sigtable immutable X-Git-Tag: go1.9beta1~1683 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=87ad863f359de3760578acb7f7a4d7e333c9cdc8;p=gostls13.git runtime: use atomic ops for fwdSig, make sigtable immutable The fwdSig array is accessed by the signal handler, which may run in parallel with other threads manipulating it via the os/signal package. Use atomic accesses to ensure that there are no problems. Move the _SigHandling flag out of the sigtable array. This makes sigtable immutable and safe to read from the signal handler. Change-Id: Icfa407518c4ebe1da38580920ced764898dfc9ad Reviewed-on: https://go-review.googlesource.com/36321 Run-TryBot: Ian Lance Taylor Reviewed-by: Austin Clements TryBot-Result: Gobot Gobot --- diff --git a/src/runtime/runtime2.go b/src/runtime/runtime2.go index 3b649761c9..61c8bd91b9 100644 --- a/src/runtime/runtime2.go +++ b/src/runtime/runtime2.go @@ -610,7 +610,6 @@ const ( _SigThrow // if signal.Notify doesn't take it, exit loudly _SigPanic // if the signal is from the kernel, panic _SigDefault // if the signal isn't explicitly requested, don't monitor it - _SigHandling // our signal handler is registered _SigGoExit // cause all runtime procs to exit (only used on Plan 9). _SigSetStack // add SA_ONSTACK to libc handler _SigUnblock // unblocked in minit diff --git a/src/runtime/signal_unix.go b/src/runtime/signal_unix.go index 0bf5a752a9..040b5e38dc 100644 --- a/src/runtime/signal_unix.go +++ b/src/runtime/signal_unix.go @@ -7,6 +7,7 @@ package runtime import ( + "runtime/internal/atomic" "runtime/internal/sys" "unsafe" ) @@ -31,11 +32,18 @@ const ( // Stores the signal handlers registered before Go installed its own. // These signal handlers will be invoked in cases where Go doesn't want to // handle a particular signal (e.g., signal occurred on a non-Go thread). -// See sigfwdgo() for more information on when the signals are forwarded. +// See sigfwdgo for more information on when the signals are forwarded. // -// Signal forwarding is currently available only on Darwin and Linux. +// This is read by the signal handler; accesses should use +// atomic.Loaduintptr and atomic.Storeuintptr. var fwdSig [_NSIG]uintptr +// handlingSig is indexed by signal number and is non-zero if we are +// currently handling the signal. Or, to put it another way, whether +// the signal handler is currently set to the Go signal handler or not. +// This is uint32 rather than bool so that we can use atomic instructions. +var handlingSig [_NSIG]uint32 + // channels for synchronizing signal mask updates with the signal mask // thread var ( @@ -76,6 +84,9 @@ func initsig(preinit bool) { if t.flags == 0 || t.flags&_SigDefault != 0 { continue } + + // We don't need to use atomic operations here because + // there shouldn't be any other goroutines running yet. fwdSig[i] = getsig(i) if !sigInstallGoHandler(i) { @@ -87,7 +98,7 @@ func initsig(preinit bool) { continue } - t.flags |= _SigHandling + handlingSig[i] = 1 setsig(i, funcPC(sighandler)) } } @@ -100,7 +111,7 @@ func sigInstallGoHandler(sig uint32) bool { // Even these signals can be fetched using the os/signal package. switch sig { case _SIGHUP, _SIGINT: - if fwdSig[sig] == _SIG_IGN { + if atomic.Loaduintptr(&fwdSig[sig]) == _SIG_IGN { return false } } @@ -119,6 +130,9 @@ func sigInstallGoHandler(sig uint32) bool { return true } +// sigenable enables the Go signal handler to catch the signal sig. +// It is only called while holding the os/signal.handlers lock, +// via os/signal.enableSignal and signal_enable. func sigenable(sig uint32) { if sig >= uint32(len(sigtable)) { return @@ -129,14 +143,16 @@ func sigenable(sig uint32) { ensureSigM() enableSigChan <- sig <-maskUpdatedChan - if t.flags&_SigHandling == 0 { - t.flags |= _SigHandling - fwdSig[sig] = getsig(sig) + if atomic.Cas(&handlingSig[sig], 0, 1) { + atomic.Storeuintptr(&fwdSig[sig], getsig(sig)) setsig(sig, funcPC(sighandler)) } } } +// sigdisable disables the Go signal handler for the signal sig. +// It is only called while holding the os/signal.handlers lock, +// via os/signal.disableSignal and signal_disable. func sigdisable(sig uint32) { if sig >= uint32(len(sigtable)) { return @@ -152,12 +168,15 @@ func sigdisable(sig uint32) { // signal, then to go back to the state before Notify // we should remove the one we installed. if !sigInstallGoHandler(sig) { - t.flags &^= _SigHandling - setsig(sig, fwdSig[sig]) + atomic.Store(&handlingSig[sig], 0) + setsig(sig, atomic.Loaduintptr(&fwdSig[sig])) } } } +// sigignore ignores the signal sig. +// It is only called while holding the os/signal.handlers lock, +// via os/signal.ignoreSignal and signal_ignore. func sigignore(sig uint32) { if sig >= uint32(len(sigtable)) { return @@ -165,7 +184,7 @@ func sigignore(sig uint32) { t := &sigtable[sig] if t.flags&_SigNotify != 0 { - t.flags &^= _SigHandling + atomic.Store(&handlingSig[sig], 0) setsig(sig, _SIG_IGN) } } @@ -348,7 +367,7 @@ func raisebadsignal(sig uint32, c *sigctxt) { if sig >= _NSIG { handler = _SIG_DFL } else { - handler = fwdSig[sig] + handler = atomic.Loaduintptr(&fwdSig[sig]) } // Reset the signal handler and raise the signal. @@ -490,7 +509,7 @@ func sigfwdgo(sig uint32, info *siginfo, ctx unsafe.Pointer) bool { if sig >= uint32(len(sigtable)) { return false } - fwdFn := fwdSig[sig] + fwdFn := atomic.Loaduintptr(&fwdSig[sig]) if !signalsOK { // The only way we can get here is if we are in a @@ -505,19 +524,23 @@ func sigfwdgo(sig uint32, info *siginfo, ctx unsafe.Pointer) bool { return true } - flags := sigtable[sig].flags - // If there is no handler to forward to, no need to forward. if fwdFn == _SIG_DFL { return false } // If we aren't handling the signal, forward it. - if flags&_SigHandling == 0 { + // Really if we aren't handling the signal, we shouldn't get here, + // but on Darwin setsigstack can lead us here because it sets + // the sa_tramp field. The sa_tramp field is not returned by + // sigaction, so the fix for that is non-obvious. + if atomic.Load(&handlingSig[sig]) == 0 { sigfwd(fwdFn, sig, info, ctx) return true } + flags := sigtable[sig].flags + c := &sigctxt{info, ctx} // Only forward synchronous signals and SIGPIPE. // Unfortunately, user generated SIGPIPEs will also be forwarded, because si_code @@ -533,10 +556,12 @@ func sigfwdgo(sig uint32, info *siginfo, ctx unsafe.Pointer) bool { if g != nil && g.m != nil && g.m.curg != nil && !g.m.incgo { return false } + // Signal not handled by Go, forward it. if fwdFn != _SIG_IGN { sigfwd(fwdFn, sig, info, ctx) } + return true }