Skip to content

Commit

Permalink
runtime: handle SIGPIPE in c-archive and c-shared programs
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Elias Naur committed Feb 3, 2017
1 parent b612ab3 commit 78074f6
Show file tree
Hide file tree
Showing 10 changed files with 165 additions and 11 deletions.
19 changes: 19 additions & 0 deletions misc/cgo/testcarchive/carchive_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
35 changes: 34 additions & 1 deletion misc/cgo/testcarchive/main2.c
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include <unistd.h>
#include <sched.h>
#include <time.h>
#include <errno.h>

#include "libgo2.h"

Expand All @@ -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) {
Expand All @@ -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];
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
}
}
Expand Down
33 changes: 33 additions & 0 deletions misc/cgo/testcarchive/main3.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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");
}
Expand Down
18 changes: 18 additions & 0 deletions misc/cgo/testcarchive/main5.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
30 changes: 30 additions & 0 deletions misc/cgo/testcarchive/src/libgo2/libgo2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -46,5 +70,11 @@ func TestSEGV() {
func Noop() {
}

// Raise SIGPIPE.
//export GoRaiseSIGPIPE
func GoRaiseSIGPIPE() {
C.CRaiseSIGPIPE()
}

func main() {
}
12 changes: 12 additions & 0 deletions misc/cgo/testcarchive/src/libgo3/libgo3.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
}
9 changes: 5 additions & 4 deletions src/os/signal/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 4 additions & 0 deletions src/runtime/cgocall.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -151,6 +152,7 @@ func cgocall(fn, arg unsafe.Pointer) int32 {

//go:nosplit
func endcgo(mp *m) {
mp.incgo = false
mp.ncgo--

if raceenabled {
Expand Down Expand Up @@ -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))

Expand Down
1 change: 1 addition & 0 deletions src/runtime/runtime2.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 9 additions & 6 deletions src/runtime/signal_unix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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.
Expand Down

0 comments on commit 78074f6

Please sign in to comment.