]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: handle SIGPIPE in c-archive and c-shared programs
authorElias Naur <elias.naur@gmail.com>
Sun, 29 Jan 2017 14:34:50 +0000 (15:34 +0100)
committerElias Naur <elias.naur@gmail.com>
Fri, 3 Feb 2017 20:07:36 +0000 (20:07 +0000)
Before this CL, Go programs in c-archive or c-shared buildmodes
would not handle SIGPIPE. That leads to surprising behaviour where
writes on a closed pipe or socket would raise SIGPIPE and terminate
the program. This CL changes the Go runtime to handle
SIGPIPE regardless of buildmode. In addition, SIGPIPE from non-Go
code is forwarded.

This is a refinement of CL 32796 that fixes the case where a non-default
handler for SIGPIPE is installed by the host C program.

Fixes #17393

Change-Id: Ia41186e52c1ac209d0a594bae9904166ae7df7de
Reviewed-on: https://go-review.googlesource.com/35960
Run-TryBot: Elias Naur <elias.naur@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
misc/cgo/testcarchive/carchive_test.go
misc/cgo/testcarchive/main2.c
misc/cgo/testcarchive/main3.c
misc/cgo/testcarchive/main5.c
misc/cgo/testcarchive/src/libgo2/libgo2.go
misc/cgo/testcarchive/src/libgo3/libgo3.go
src/os/signal/doc.go
src/runtime/cgocall.go
src/runtime/runtime2.go
src/runtime/signal_unix.go

index 499992977517ea7aacdcf86aa437562950696fac..3c768a0ef349a92925a70ade18caf5ba35b4fac9 100644 (file)
@@ -265,6 +265,25 @@ func TestSignalForwarding(t *testing.T) {
                t.Logf("%s", out)
                t.Errorf("got %v; expected SIGSEGV", ee)
        }
+
+       // Test SIGPIPE forwarding
+       cmd = exec.Command(bin[0], append(bin[1:], "3")...)
+
+       out, err = cmd.CombinedOutput()
+
+       if err == nil {
+               t.Logf("%s", out)
+               t.Error("test program succeeded unexpectedly")
+       } else if ee, ok := err.(*exec.ExitError); !ok {
+               t.Logf("%s", out)
+               t.Errorf("error (%v) has type %T; expected exec.ExitError", err, err)
+       } else if ws, ok := ee.Sys().(syscall.WaitStatus); !ok {
+               t.Logf("%s", out)
+               t.Errorf("error.Sys (%v) has type %T; expected syscall.WaitStatus", ee.Sys(), ee.Sys())
+       } else if !ws.Signaled() || ws.Signal() != syscall.SIGPIPE {
+               t.Logf("%s", out)
+               t.Errorf("got %v; expected SIGPIPE", ee)
+       }
 }
 
 func TestSignalForwardingExternal(t *testing.T) {
index 774e014a1620967269a99eba962b175b69382c8f..769cd497e6c1d2e1a8431db41675f38feaf2e8ff 100644 (file)
@@ -17,6 +17,7 @@
 #include <unistd.h>
 #include <sched.h>
 #include <time.h>
+#include <errno.h>
 
 #include "libgo2.h"
 
@@ -26,6 +27,7 @@ static void die(const char* msg) {
 }
 
 static volatile sig_atomic_t sigioSeen;
+static volatile sig_atomic_t sigpipeSeen;
 
 // Use up some stack space.
 static void recur(int i, char *p) {
@@ -37,6 +39,10 @@ static void recur(int i, char *p) {
        }
 }
 
+static void pipeHandler(int signo, siginfo_t* info, void* ctxt) {
+       sigpipeSeen = 1;
+}
+
 // Signal handler that uses up more stack space than a goroutine will have.
 static void ioHandler(int signo, siginfo_t* info, void* ctxt) {
        char a[1024];
@@ -106,6 +112,10 @@ static void init() {
                die("sigaction");
        }
 
+       sa.sa_sigaction = pipeHandler;
+       if (sigaction(SIGPIPE, &sa, NULL) < 0) {
+               die("sigaction");
+       }
 }
 
 int main(int argc, char** argv) {
@@ -167,7 +177,30 @@ int main(int argc, char** argv) {
                nanosleep(&ts, NULL);
                i++;
                if (i > 5000) {
-                       fprintf(stderr, "looping too long waiting for signal\n");
+                       fprintf(stderr, "looping too long waiting for SIGIO\n");
+                       exit(EXIT_FAILURE);
+               }
+       }
+
+       if (verbose) {
+               printf("provoking SIGPIPE\n");
+       }
+
+       GoRaiseSIGPIPE();
+
+       if (verbose) {
+               printf("waiting for sigpipeSeen\n");
+       }
+
+       // Wait until the signal has been delivered.
+       i = 0;
+       while (!sigpipeSeen) {
+               ts.tv_sec = 0;
+               ts.tv_nsec = 1000000;
+               nanosleep(&ts, NULL);
+               i++;
+               if (i > 5000) {
+                       fprintf(stderr, "looping too long waiting for SIGPIPE\n");
                        exit(EXIT_FAILURE);
                }
        }
index 0a6c0d3f74e077b01fa7834d9f415f7acb1ea9d0..5a1a60d4cd200f397937d722eb17981ccebe2c28 100644 (file)
@@ -25,6 +25,31 @@ static void ioHandler(int signo, siginfo_t* info, void* ctxt) {
        sigioSeen = 1;
 }
 
+// Set up the SIGPIPE signal handler in a high priority constructor, so
+// that it is installed before the Go code starts.
+
+static void pipeHandler(int signo, siginfo_t* info, void* ctxt) {
+       const char *s = "unexpected SIGPIPE\n";
+       write(2, s, strlen(s));
+       exit(EXIT_FAILURE);
+}
+
+static void init(void) __attribute__ ((constructor (200)));
+
+static void init() {
+    struct sigaction sa;
+
+       memset(&sa, 0, sizeof sa);
+       sa.sa_sigaction = pipeHandler;
+       if (sigemptyset(&sa.sa_mask) < 0) {
+               die("sigemptyset");
+       }
+       sa.sa_flags = SA_SIGINFO;
+       if (sigaction(SIGPIPE, &sa, NULL) < 0) {
+               die("sigaction");
+       }
+}
+
 int main(int argc, char** argv) {
        int verbose;
        struct sigaction sa;
@@ -34,6 +59,14 @@ int main(int argc, char** argv) {
        verbose = argc > 2;
        setvbuf(stdout, NULL, _IONBF, 0);
 
+       if (verbose) {
+               printf("raising SIGPIPE\n");
+       }
+
+       // Test that the Go runtime handles SIGPIPE, even if we installed
+       // a non-default SIGPIPE handler before the runtime initializes.
+       ProvokeSIGPIPE();
+
        if (verbose) {
                printf("calling sigaction\n");
        }
index 9fadf0801e10c996b2f48b6876d5b491a9984030..2437bf07c58f8f530bf42e5b6dac9396fdf26b34 100644 (file)
@@ -68,6 +68,24 @@ int main(int argc, char** argv) {
 
                        break;
                }
+               case 3: {
+                       if (verbose) {
+                               printf("attempting SIGPIPE\n");
+                       }
+
+                       int fd[2];
+                       if (pipe(fd) != 0) {
+                               printf("pipe(2) failed\n");
+                               return 0;
+                       }
+                       // Close the reading end.
+                       close(fd[0]);
+                       // Expect that write(2) fails (EPIPE)
+                       if (write(fd[1], "some data", 9) != -1) {
+                               printf("write(2) unexpectedly succeeded\n");
+                               return 0;
+                       }
+               }
                default:
                        printf("Unknown test: %d\n", test);
                        return 0;
index fbed493b93ff20463d25cf71f81f554a0ac728f3..19c8e1a6dcb8e8dd8c13d24f778a8685f3c113e8 100644 (file)
@@ -4,6 +4,30 @@
 
 package main
 
+/*
+#include <signal.h>
+#include <unistd.h>
+#include <stdlib.h>
+#include <stdio.h>
+
+// Raise SIGPIPE.
+static void CRaiseSIGPIPE() {
+       int fds[2];
+
+       if (pipe(fds) == -1) {
+               perror("pipe");
+               exit(EXIT_FAILURE);
+       }
+       // Close the reader end
+       close(fds[0]);
+       // Write to the writer end to provoke a SIGPIPE
+       if (write(fds[1], "some data", 9) != -1) {
+               fprintf(stderr, "write to a closed pipe succeeded\n");
+               exit(EXIT_FAILURE);
+       }
+       close(fds[1]);
+}
+*/
 import "C"
 
 import (
@@ -46,5 +70,11 @@ func TestSEGV() {
 func Noop() {
 }
 
+// Raise SIGPIPE.
+//export GoRaiseSIGPIPE
+func GoRaiseSIGPIPE() {
+       C.CRaiseSIGPIPE()
+}
+
 func main() {
 }
index 94e5d21c14a83c3019ef4ba09b17fc3222e7e7be..e276a3c347ae2b680c0a689beaf0b5ce02c74142 100644 (file)
@@ -40,5 +40,17 @@ func SawSIGIO() C.int {
        }
 }
 
+// ProvokeSIGPIPE provokes a kernel-initiated SIGPIPE.
+//export ProvokeSIGPIPE
+func ProvokeSIGPIPE() {
+       r, w, err := os.Pipe()
+       if err != nil {
+               panic(err)
+       }
+       r.Close()
+       defer w.Close()
+       w.Write([]byte("some data"))
+}
+
 func main() {
 }
index 73b01a2764de60667979964bb3cc3ad21d677f6d..16f49c7ab8be17d0e9b24261ebc20ed99228f839 100644 (file)
@@ -181,10 +181,11 @@ If the Go runtime sees an existing signal handler for the SIGCANCEL or
 SIGSETXID signals (which are used only on GNU/Linux), it will turn on
 the SA_ONSTACK flag and otherwise keep the signal handler.
 
-For the synchronous signals, the Go runtime will install a signal
-handler. It will save any existing signal handler. If a synchronous
-signal arrives while executing non-Go code, the Go runtime will invoke
-the existing signal handler instead of the Go signal handler.
+For the synchronous signals and SIGPIPE, the Go runtime will install a
+signal handler. It will save any existing signal handler. If a
+synchronous signal arrives while executing non-Go code, the Go runtime
+will invoke the existing signal handler instead of the Go signal
+handler.
 
 Go code built with -buildmode=c-archive or -buildmode=c-shared will
 not install any other signal handlers by default. If there is an
index 879e786231389010090248523de5efa68116f78d..755269ebd2959f78e5d041f363cf47a171fb2d3a 100644 (file)
@@ -110,6 +110,7 @@ func cgocall(fn, arg unsafe.Pointer) int32 {
        mp := getg().m
        mp.ncgocall++
        mp.ncgo++
+       mp.incgo = true
 
        // Reset traceback.
        mp.cgoCallers[0] = 0
@@ -151,6 +152,7 @@ func cgocall(fn, arg unsafe.Pointer) int32 {
 
 //go:nosplit
 func endcgo(mp *m) {
+       mp.incgo = false
        mp.ncgo--
 
        if raceenabled {
@@ -180,9 +182,11 @@ func cgocallbackg(ctxt uintptr) {
        savedsp := unsafe.Pointer(gp.syscallsp)
        savedpc := gp.syscallpc
        exitsyscall(0) // coming out of cgo call
+       gp.m.incgo = false
 
        cgocallbackg1(ctxt)
 
+       gp.m.incgo = true
        // going back to cgo call
        reentersyscall(savedpc, uintptr(savedsp))
 
index 1ceab0ad8c4a0b43d28d63cdd2921ae7da0e1454..c164c0f7b426e03eadeee0911299613398355f5a 100644 (file)
@@ -429,6 +429,7 @@ type m struct {
        inwb          bool // m is executing a write barrier
        newSigstack   bool // minit on C thread called sigaltstack
        printlock     int8
+       incgo         bool // m is executing a cgo call
        fastrand      uint32
        ncgocall      uint64      // number of cgo calls in total
        ncgo          int32       // number of cgo calls currently in progress
index 49c7579f275d3fb8c608bb7bdfc0658792e9582a..0bf5a752a9ee51018dfb82c411629f1abc5af27b 100644 (file)
@@ -111,8 +111,8 @@ func sigInstallGoHandler(sig uint32) bool {
        }
 
        // When built using c-archive or c-shared, only install signal
-       // handlers for synchronous signals.
-       if (isarchive || islibrary) && t.flags&_SigPanic == 0 {
+       // handlers for synchronous signals and SIGPIPE.
+       if (isarchive || islibrary) && t.flags&_SigPanic == 0 && sig != _SIGPIPE {
                return false
        }
 
@@ -518,16 +518,19 @@ func sigfwdgo(sig uint32, info *siginfo, ctx unsafe.Pointer) bool {
                return true
        }
 
-       // Only forward synchronous signals.
        c := &sigctxt{info, ctx}
-       if c.sigcode() == _SI_USER || flags&_SigPanic == 0 {
+       // Only forward synchronous signals and SIGPIPE.
+       // Unfortunately, user generated SIGPIPEs will also be forwarded, because si_code
+       // is set to _SI_USER even for a SIGPIPE raised from a write to a closed socket
+       // or pipe.
+       if (c.sigcode() == _SI_USER || flags&_SigPanic == 0) && sig != _SIGPIPE {
                return false
        }
        // Determine if the signal occurred inside Go code. We test that:
        //   (1) we were in a goroutine (i.e., m.curg != nil), and
-       //   (2) we weren't in CGO (i.e., m.curg.syscallsp == 0).
+       //   (2) we weren't in CGO.
        g := getg()
-       if g != nil && g.m != nil && g.m.curg != nil && g.m.curg.syscallsp == 0 {
+       if g != nil && g.m != nil && g.m.curg != nil && !g.m.incgo {
                return false
        }
        // Signal not handled by Go, forward it.