]> Cypherpunks repositories - gostls13.git/commitdiff
os/signal: add Stop, be careful about SIGHUP
authorRuss Cox <rsc@golang.org>
Fri, 15 Mar 2013 04:00:02 +0000 (00:00 -0400)
committerRuss Cox <rsc@golang.org>
Fri, 15 Mar 2013 04:00:02 +0000 (00:00 -0400)
Fixes #4268.
Fixes #4491.

R=golang-dev, nightlyone, fullung, r
CC=golang-dev
https://golang.org/cl/7546048

12 files changed:
src/pkg/os/signal/sig.s
src/pkg/os/signal/signal.go
src/pkg/os/signal/signal_stub.go
src/pkg/os/signal/signal_test.go
src/pkg/os/signal/signal_unix.go
src/pkg/runtime/os_plan9_386.c
src/pkg/runtime/os_plan9_amd64.c
src/pkg/runtime/os_windows_386.c
src/pkg/runtime/os_windows_amd64.c
src/pkg/runtime/runtime.h
src/pkg/runtime/signal_unix.c
src/pkg/runtime/sigqueue.goc

index d1984cf886050c28d495bd5e02ecb03de3a01ef2..7d0c92b19508cd3e69d062e6c543a97110e79223 100644 (file)
@@ -8,6 +8,9 @@
 #define JMP B
 #endif
 
+TEXT ·signal_disable(SB),7,$0
+       JMP runtime·signal_disable(SB)
+
 TEXT ·signal_enable(SB),7,$0
        JMP runtime·signal_enable(SB)
 
index 0861f59aa9deaf56e7f9de486a202034307e3906..30042754953e2d72181ff34fa1bfd8cef713f34d 100644 (file)
@@ -14,13 +14,20 @@ import (
 
 var handlers struct {
        sync.Mutex
-       list []handler
+       m   map[chan<- os.Signal]*handler
+       ref [numSig]int64
 }
 
 type handler struct {
-       c   chan<- os.Signal
-       sig os.Signal
-       all bool
+       mask [(numSig + 31) / 32]uint32
+}
+
+func (h *handler) want(sig int) bool {
+       return (h.mask[sig/32]>>uint(sig&31))&1 != 0
+}
+
+func (h *handler) set(sig int) {
+       h.mask[sig/32] |= 1 << uint(sig&31)
 }
 
 // Notify causes package signal to relay incoming signals to c.
@@ -32,6 +39,13 @@ type handler struct {
 // signal rate.  For a channel used for notification of just one signal value,
 // a buffer of size 1 is sufficient.
 //
+// It is allowed to call Notify multiple times with the same channel:
+// each call expands the set of signals sent to that channel.
+// The only way to remove signals from the set is to call Stop.
+//
+// It is allowed to call Notify multiple times with different channels
+// and the same signals: each channel receives copies of incoming
+// signals independently.
 func Notify(c chan<- os.Signal, sig ...os.Signal) {
        if c == nil {
                panic("os/signal: Notify using nil channel")
@@ -39,32 +53,77 @@ func Notify(c chan<- os.Signal, sig ...os.Signal) {
 
        handlers.Lock()
        defer handlers.Unlock()
+
+       h := handlers.m[c]
+       if h == nil {
+               if handlers.m == nil {
+                       handlers.m = make(map[chan<- os.Signal]*handler)
+               }
+               h = new(handler)
+               handlers.m[c] = h
+       }
+
+       add := func(n int) {
+               if n < 0 {
+                       return
+               }
+               if !h.want(n) {
+                       h.set(n)
+                       if handlers.ref[n] == 0 {
+                               enableSignal(n)
+                       }
+                       handlers.ref[n]++
+               }
+       }
+
        if len(sig) == 0 {
-               enableSignal(nil)
-               handlers.list = append(handlers.list, handler{c: c, all: true})
+               for n := 0; n < numSig; n++ {
+                       add(n)
+               }
        } else {
                for _, s := range sig {
-                       // We use nil as a special wildcard value for enableSignal,
-                       // so filter it out of the list of arguments.  This is safe because
-                       // we will never get an incoming nil signal, so discarding the
-                       // registration cannot affect the observed behavior.
-                       if s != nil {
-                               enableSignal(s)
-                               handlers.list = append(handlers.list, handler{c: c, sig: s})
+                       add(signum(s))
+               }
+       }
+}
+
+// Stop causes package signal to stop relaying incoming signals to c.
+// It undoes the effect of all prior calls to Notify using c.
+// When Stop returns, it is guaranteed that c will receive no more signals.
+func Stop(c chan<- os.Signal) {
+       handlers.Lock()
+       defer handlers.Unlock()
+
+       h := handlers.m[c]
+       if h == nil {
+               return
+       }
+       delete(handlers.m, c)
+
+       for n := 0; n < numSig; n++ {
+               if h.want(n) {
+                       handlers.ref[n]--
+                       if handlers.ref[n] == 0 {
+                               disableSignal(n)
                        }
                }
        }
 }
 
 func process(sig os.Signal) {
+       n := signum(sig)
+       if n < 0 {
+               return
+       }
+
        handlers.Lock()
        defer handlers.Unlock()
 
-       for _, h := range handlers.list {
-               if h.all || h.sig == sig {
+       for c, h := range handlers.m {
+               if h.want(n) {
                        // send but do not block for it
                        select {
-                       case h.c <- sig:
+                       case c <- sig:
                        default:
                        }
                }
index fc227cf4c2d1861b5ec1c26f010cea776d0dd101..d0a6935ff2cddbc6b51178b1c8db250358e02b89 100644 (file)
@@ -8,4 +8,10 @@ package signal
 
 import "os"
 
-func enableSignal(sig os.Signal) {}
+const numSig = 0
+
+func signum(sig os.Signal) int { return -1 }
+
+func disableSignal(int) {}
+
+func enableSignal(int) {}
index 93e5ab9fad1ca6a223f40c2f9802721e99b5377e..d13833306f87ed357b321bdadd05a71a3ef9dd23 100644 (file)
@@ -7,15 +7,17 @@
 package signal
 
 import (
+       "flag"
+       "io/ioutil"
        "os"
+       "os/exec"
        "runtime"
+       "strconv"
        "syscall"
        "testing"
        "time"
 )
 
-const sighup = syscall.SIGHUP
-
 func waitSig(t *testing.T, c <-chan os.Signal, sig os.Signal) {
        select {
        case s := <-c:
@@ -27,15 +29,17 @@ func waitSig(t *testing.T, c <-chan os.Signal, sig os.Signal) {
        }
 }
 
+// Test that basic signal handling works.
 func TestSignal(t *testing.T) {
        // Ask for SIGHUP
        c := make(chan os.Signal, 1)
-       Notify(c, sighup)
+       Notify(c, syscall.SIGHUP)
+       defer Stop(c)
 
        t.Logf("sighup...")
        // Send this process a SIGHUP
-       syscall.Kill(syscall.Getpid(), sighup)
-       waitSig(t, c, sighup)
+       syscall.Kill(syscall.Getpid(), syscall.SIGHUP)
+       waitSig(t, c, syscall.SIGHUP)
 
        // Ask for everything we can get.
        c1 := make(chan os.Signal, 1)
@@ -71,6 +75,7 @@ func TestStress(t *testing.T) {
        go func() {
                sig := make(chan os.Signal, 1)
                Notify(sig, syscall.SIGUSR1)
+               defer Stop(sig)
        Loop:
                for {
                        select {
@@ -103,3 +108,101 @@ func TestStress(t *testing.T) {
        // Sleep for a while to reduce the possibility of the failure.
        time.Sleep(10 * time.Millisecond)
 }
+
+var sendUncaughtSighup = flag.Int("send_uncaught_sighup", 0, "send uncaught SIGHUP during TestStop")
+
+// Test that Stop cancels the channel's registrations.
+func TestStop(t *testing.T) {
+       sigs := []syscall.Signal{
+               syscall.SIGWINCH,
+               syscall.SIGHUP,
+       }
+
+       for _, sig := range sigs {
+               // Send the signal.
+               // If it's SIGWINCH, we should not see it.
+               // If it's SIGHUP, maybe we'll die. Let the flag tell us what to do.
+               if sig != syscall.SIGHUP || *sendUncaughtSighup == 1 {
+                       syscall.Kill(syscall.Getpid(), sig)
+               }
+               time.Sleep(10 * time.Millisecond)
+
+               // Ask for signal
+               c := make(chan os.Signal, 1)
+               Notify(c, sig)
+               defer Stop(c)
+
+               // Send this process that signal
+               syscall.Kill(syscall.Getpid(), sig)
+               waitSig(t, c, sig)
+
+               Stop(c)
+               select {
+               case s := <-c:
+                       t.Fatalf("unexpected signal %v", s)
+               case <-time.After(10 * time.Millisecond):
+                       // nothing to read - good
+               }
+
+               // Send the signal.
+               // If it's SIGWINCH, we should not see it.
+               // If it's SIGHUP, maybe we'll die. Let the flag tell us what to do.
+               if sig != syscall.SIGHUP || *sendUncaughtSighup == 2 {
+                       syscall.Kill(syscall.Getpid(), sig)
+               }
+
+               select {
+               case s := <-c:
+                       t.Fatalf("unexpected signal %v", s)
+               case <-time.After(10 * time.Millisecond):
+                       // nothing to read - good
+               }
+       }
+}
+
+// Test that when run under nohup, an uncaught SIGHUP does not kill the program,
+// but a
+func TestNohup(t *testing.T) {
+       // Ugly: ask for SIGHUP so that child will not have no-hup set
+       // even if test is running under nohup environment.
+       // We have no intention of reading from c.
+       c := make(chan os.Signal, 1)
+       Notify(c, syscall.SIGHUP)
+
+       // When run without nohup, the test should crash on an uncaught SIGHUP.
+       // When run under nohup, the test should ignore uncaught SIGHUPs,
+       // because the runtime is not supposed to be listening for them.
+       // Either way, TestStop should still be able to catch them when it wants them
+       // and then when it stops wanting them, the original behavior should resume.
+       //
+       // send_uncaught_sighup=1 sends the SIGHUP before starting to listen for SIGHUPs.
+       // send_uncaught_sighup=2 sends the SIGHUP after no longer listening for SIGHUPs.
+       //
+       // Both should fail without nohup and succeed with nohup.
+
+       for i := 1; i <= 2; i++ {
+               out, err := exec.Command(os.Args[0], "-test.run=TestStop", "-send_uncaught_sighup="+strconv.Itoa(i)).CombinedOutput()
+               if err == nil {
+                       t.Fatalf("ran test with -send_uncaught_sighup=%d and it succeeded: expected failure.\nOutput:\n%s", i, out)
+               }
+       }
+
+       Stop(c)
+
+       // Again, this time with nohup, assuming we can find it.
+       _, err := os.Stat("/usr/bin/nohup")
+       if err != nil {
+               t.Skip("cannot find nohup; skipping second half of test")
+       }
+
+       for i := 1; i <= 2; i++ {
+               os.Remove("nohup.out")
+               out, err := exec.Command("/usr/bin/nohup", os.Args[0], "-test.run=TestStop", "-send_uncaught_sighup="+strconv.Itoa(i)).CombinedOutput()
+
+               data, _ := ioutil.ReadFile("nohup.out")
+               os.Remove("nohup.out")
+               if err != nil {
+                       t.Fatalf("ran test with -send_uncaught_sighup=%d under nohup and it failed: expected success.\nError: %v\nOutput:\n%s%s", i, err, out, data)
+               }
+       }
+}
index 20ee5f26aaa1f8b773be5f4a681f74436094c232..6b4c8ab662e41cb91e558913b7873e78bce98a8a 100644 (file)
@@ -12,6 +12,7 @@ import (
 )
 
 // In assembly.
+func signal_disable(uint32)
 func signal_enable(uint32)
 func signal_recv() uint32
 
@@ -26,13 +27,27 @@ func init() {
        go loop()
 }
 
-func enableSignal(sig os.Signal) {
+const (
+       numSig = 65 // max across all systems
+)
+
+func signum(sig os.Signal) int {
        switch sig := sig.(type) {
-       case nil:
-               signal_enable(^uint32(0))
        case syscall.Signal:
-               signal_enable(uint32(sig))
+               i := int(sig)
+               if i < 0 || i >= numSig {
+                       return -1
+               }
+               return i
        default:
-               // Can ignore: this signal (whatever it is) will never come in.
+               return -1
        }
 }
+
+func enableSignal(sig int) {
+       signal_enable(uint32(sig))
+}
+
+func disableSignal(sig int) {
+       signal_disable(uint32(sig))
+}
index 17bc117496a8d141c3bd61a095102e5ba3bdee72..3d8b43a5da7aa360151a481ad394d04c4d7744b0 100644 (file)
@@ -111,6 +111,12 @@ runtime·sigenable(uint32 sig)
        USED(sig);
 }
 
+void
+runtime·sigdisable(uint32 sig)
+{
+       USED(sig);
+}
+
 void
 runtime·resetcpuprofiler(int32 hz)
 {
index e4f946abce0b354ab8de2aa9beb35fe092722dc2..acdf65d00cb46d243992ee8ec278d4f71e37c34b 100644 (file)
@@ -118,6 +118,12 @@ runtime·sigenable(uint32 sig)
        USED(sig);
 }
 
+void
+runtime·sigdisable(uint32 sig)
+{
+       USED(sig);
+}
+
 void
 runtime·resetcpuprofiler(int32 hz)
 {
index d76d5bf4bd8ce02cf3c95f115770c0ba90b9abf3..fc75eb3af007e2cc2bf47f35a6b09412bc2baab8 100644 (file)
@@ -90,6 +90,12 @@ runtime·sigenable(uint32 sig)
        USED(sig);
 }
 
+void
+runtime·sigdisable(uint32 sig)
+{
+       USED(sig);
+}
+
 void
 runtime·dosigprof(Context *r, G *gp)
 {
index 3729aa57b76f8601e2d5dae99801a6fa76051179..7ed33465e30a17be6d55e7ad42d398d12affc13e 100644 (file)
@@ -97,6 +97,12 @@ runtime·sigenable(uint32 sig)
        USED(sig);
 }
 
+void
+runtime·sigdisable(uint32 sig)
+{
+       USED(sig);
+}
+
 void
 runtime·dosigprof(Context *r, G *gp)
 {
index e2c35e1657c11ddc05aa215e230e967506ddb8f8..e0da57bb0ab9e5f89dd508b957f9ac52d7794802 100644 (file)
@@ -384,6 +384,8 @@ enum
        SigThrow = 1<<2,        // if signal.Notify doesn't take it, exit loudly
        SigPanic = 1<<3,        // if the signal is from the kernel, panic
        SigDefault = 1<<4,      // if the signal isn't explicitly requested, don't monitor it
+       SigHandling = 1<<5,     // our signal handler is registered
+       SigIgnored = 1<<6,      // the signal was ignored before we registered for it
 };
 
 // NOTE(rsc): keep in sync with extern.go:/type.Func.
@@ -696,6 +698,7 @@ String      runtime·gostringnocopy(byte*);
 String runtime·gostringw(uint16*);
 void   runtime·initsig(void);
 void   runtime·sigenable(uint32 sig);
+void   runtime·sigdisable(uint32 sig);
 int32  runtime·gotraceback(void);
 void   runtime·goroutineheader(G*);
 void   runtime·traceback(uint8 *pc, uint8 *sp, uint8 *lr, G* gp);
index f3542acbae320f890c4b4bb45a2688efb94ec4b7..5d0bcbd2d51ab956eeb5c11003f9c1bd1b3457f5 100644 (file)
@@ -22,6 +22,20 @@ runtime·initsig(void)
                t = &runtime·sigtab[i];
                if((t->flags == 0) || (t->flags & SigDefault))
                        continue;
+
+               // For some signals, we respect an inherited SIG_IGN handler
+               // rather than insist on installing our own default handler.
+               // Even these signals can be fetched using the os/signal package.
+               switch(i) {
+               case SIGHUP:
+               case SIGINT:
+                       if(runtime·getsig(i) == SIG_IGN) {
+                               t->flags = SigNotify | SigIgnored;
+                               continue;
+                       }
+               }
+
+               t->flags |= SigHandling;
                runtime·setsig(i, runtime·sighandler, true);
        }
 }
@@ -29,18 +43,35 @@ runtime·initsig(void)
 void
 runtime·sigenable(uint32 sig)
 {
-       int32 i;
        SigTab *t;
 
-       for(i = 0; i<NSIG; i++) {
-               // ~0 means all signals.
-               if(~sig == 0 || i == sig) {
-                       t = &runtime·sigtab[i];
-                       if(t->flags & SigDefault) {
-                               runtime·setsig(i, runtime·sighandler, true);
-                               t->flags &= ~SigDefault;  // make this idempotent
-                       }
-               }
+       if(sig >= NSIG)
+               return;
+
+       t = &runtime·sigtab[sig];
+       if((t->flags & SigNotify) && !(t->flags & SigHandling)) {
+               t->flags |= SigHandling;
+               if(runtime·getsig(sig) == SIG_IGN)
+                       t->flags |= SigIgnored;
+               runtime·setsig(sig, runtime·sighandler, true);
+       }
+}
+
+void
+runtime·sigdisable(uint32 sig)
+{
+       SigTab *t;
+
+       if(sig >= NSIG)
+               return;
+
+       t = &runtime·sigtab[sig];
+       if((t->flags & SigNotify) && (t->flags & SigHandling)) {
+               t->flags &= ~SigHandling;
+               if(t->flags & SigIgnored)
+                       runtime·setsig(sig, SIG_IGN, true);
+               else
+                       runtime·setsig(sig, SIG_DFL, true);
        }
 }
 
index ab5f312e420134640e3d2d52c5d04d3cc881c8df..7e083685d057c597dba0e71931ae35059a4f54b2 100644 (file)
@@ -133,8 +133,6 @@ done:;
 
 // Must only be called from a single goroutine at a time.
 func signal_enable(s uint32) {
-       int32 i;
-
        if(!sig.inuse) {
                // The first call to signal_enable is for us
                // to use for initialization.  It does not pass
@@ -144,16 +142,16 @@ func signal_enable(s uint32) {
                return;
        }
        
-       if(~s == 0) {
-               // Special case: want everything.
-               for(i=0; i<nelem(sig.wanted); i++)
-                       sig.wanted[i] = ~(uint32)0;
-               runtime·sigenable(s);
-               return;
-       }
-
        if(s >= nelem(sig.wanted)*32)
                return;
        sig.wanted[s/32] |= 1U<<(s&31);
        runtime·sigenable(s);
 }
+
+// Must only be called from a single goroutine at a time.
+func signal_disable(s uint32) {
+       if(s >= nelem(sig.wanted)*32)
+               return;
+       sig.wanted[s/32] &= ~(1U<<(s&31));
+       runtime·sigdisable(s);
+}