Skip to content

Commit d24b57a

Browse files
Elias Naurianlancetaylor
Elias Naur
authored andcommitted
runtime: handle SIGPIPE in c-archive and c-shared programs
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. Fixes #17393 Updates #16760 Change-Id: I155e82020a03a5cdc627a147c27da395662c3fe8 Reviewed-on: https://go-review.googlesource.com/32796 Run-TryBot: Elias Naur <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent e54662d commit d24b57a

File tree

8 files changed

+136
-9
lines changed

8 files changed

+136
-9
lines changed

misc/cgo/testcarchive/carchive_test.go

+19
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,25 @@ func TestSignalForwarding(t *testing.T) {
265265
t.Logf("%s", out)
266266
t.Errorf("got %v; expected SIGSEGV", ee)
267267
}
268+
269+
// Test SIGPIPE forwarding
270+
cmd = exec.Command(bin[0], append(bin[1:], "3")...)
271+
272+
out, err = cmd.CombinedOutput()
273+
274+
if err == nil {
275+
t.Logf("%s", out)
276+
t.Error("test program succeeded unexpectedly")
277+
} else if ee, ok := err.(*exec.ExitError); !ok {
278+
t.Logf("%s", out)
279+
t.Errorf("error (%v) has type %T; expected exec.ExitError", err, err)
280+
} else if ws, ok := ee.Sys().(syscall.WaitStatus); !ok {
281+
t.Logf("%s", out)
282+
t.Errorf("error.Sys (%v) has type %T; expected syscall.WaitStatus", ee.Sys(), ee.Sys())
283+
} else if !ws.Signaled() || ws.Signal() != syscall.SIGPIPE {
284+
t.Logf("%s", out)
285+
t.Errorf("got %v; expected SIGPIPE", ee)
286+
}
268287
}
269288

270289
func TestSignalForwardingExternal(t *testing.T) {

misc/cgo/testcarchive/main2.c

+35-1
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include <unistd.h>
1818
#include <sched.h>
1919
#include <time.h>
20+
#include <errno.h>
2021

2122
#include "libgo2.h"
2223

@@ -26,6 +27,7 @@ static void die(const char* msg) {
2627
}
2728

2829
static volatile sig_atomic_t sigioSeen;
30+
static volatile sig_atomic_t sigpipeSeen;
2931

3032
// Use up some stack space.
3133
static void recur(int i, char *p) {
@@ -37,6 +39,11 @@ static void recur(int i, char *p) {
3739
}
3840
}
3941

42+
// Signal handler that uses up more stack space than a goroutine will have.
43+
static void pipeHandler(int signo, siginfo_t* info, void* ctxt) {
44+
sigpipeSeen = 1;
45+
}
46+
4047
// Signal handler that uses up more stack space than a goroutine will have.
4148
static void ioHandler(int signo, siginfo_t* info, void* ctxt) {
4249
char a[1024];
@@ -106,6 +113,10 @@ static void init() {
106113
die("sigaction");
107114
}
108115

116+
sa.sa_sigaction = pipeHandler;
117+
if (sigaction(SIGPIPE, &sa, NULL) < 0) {
118+
die("sigaction");
119+
}
109120
}
110121

111122
int main(int argc, char** argv) {
@@ -167,7 +178,30 @@ int main(int argc, char** argv) {
167178
nanosleep(&ts, NULL);
168179
i++;
169180
if (i > 5000) {
170-
fprintf(stderr, "looping too long waiting for signal\n");
181+
fprintf(stderr, "looping too long waiting for SIGIO\n");
182+
exit(EXIT_FAILURE);
183+
}
184+
}
185+
186+
if (verbose) {
187+
printf("provoking SIGPIPE\n");
188+
}
189+
190+
GoRaiseSIGPIPE();
191+
192+
if (verbose) {
193+
printf("waiting for sigpipeSeen\n");
194+
}
195+
196+
// Wait until the signal has been delivered.
197+
i = 0;
198+
while (!sigpipeSeen) {
199+
ts.tv_sec = 0;
200+
ts.tv_nsec = 1000000;
201+
nanosleep(&ts, NULL);
202+
i++;
203+
if (i > 1000) {
204+
fprintf(stderr, "looping too long waiting for SIGPIPE\n");
171205
exit(EXIT_FAILURE);
172206
}
173207
}

misc/cgo/testcarchive/main3.c

+7
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,13 @@ int main(int argc, char** argv) {
3434
verbose = argc > 2;
3535
setvbuf(stdout, NULL, _IONBF, 0);
3636

37+
if (verbose) {
38+
printf("raising SIGPIPE\n");
39+
}
40+
41+
// Test that the Go runtime handles SIGPIPE.
42+
ProvokeSIGPIPE();
43+
3744
if (verbose) {
3845
printf("calling sigaction\n");
3946
}

misc/cgo/testcarchive/main5.c

+18
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,24 @@ int main(int argc, char** argv) {
6868

6969
break;
7070
}
71+
case 3: {
72+
if (verbose) {
73+
printf("attempting SIGPIPE\n");
74+
}
75+
76+
int fd[2];
77+
if (pipe(fd) != 0) {
78+
printf("pipe(2) failed\n");
79+
return 0;
80+
}
81+
// Close the reading end.
82+
close(fd[0]);
83+
// Expect that write(2) fails (EPIPE)
84+
if (write(fd[1], "some data", 9) != -1) {
85+
printf("write(2) unexpectedly succeeded\n");
86+
return 0;
87+
}
88+
}
7189
default:
7290
printf("Unknown test: %d\n", test);
7391
return 0;

misc/cgo/testcarchive/src/libgo2/libgo2.go

+30
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,30 @@
44

55
package main
66

7+
/*
8+
#include <signal.h>
9+
#include <unistd.h>
10+
#include <stdlib.h>
11+
#include <stdio.h>
12+
13+
// Raise SIGPIPE.
14+
static void CRaiseSIGPIPE() {
15+
int fds[2];
16+
17+
if (pipe(fds) == -1) {
18+
perror("pipe");
19+
exit(EXIT_FAILURE);
20+
}
21+
// Close the reader end
22+
close(fds[0]);
23+
// Write to the writer end to provoke a SIGPIPE
24+
if (write(fds[1], "some data", 9) != -1) {
25+
fprintf(stderr, "write to a closed pipe succeeded\n");
26+
exit(EXIT_FAILURE);
27+
}
28+
close(fds[1]);
29+
}
30+
*/
731
import "C"
832

933
import (
@@ -46,5 +70,11 @@ func TestSEGV() {
4670
func Noop() {
4771
}
4872

73+
// Raise SIGPIPE.
74+
//export GoRaiseSIGPIPE
75+
func GoRaiseSIGPIPE() {
76+
C.CRaiseSIGPIPE()
77+
}
78+
4979
func main() {
5080
}

misc/cgo/testcarchive/src/libgo3/libgo3.go

+12
Original file line numberDiff line numberDiff line change
@@ -40,5 +40,17 @@ func SawSIGIO() C.int {
4040
}
4141
}
4242

43+
// ProvokeSIGPIPE provokes a kernel-initiated SIGPIPE
44+
//export ProvokeSIGPIPE
45+
func ProvokeSIGPIPE() {
46+
r, w, err := os.Pipe()
47+
if err != nil {
48+
panic(err)
49+
}
50+
r.Close()
51+
defer w.Close()
52+
w.Write([]byte("some data"))
53+
}
54+
4355
func main() {
4456
}

src/os/signal/doc.go

+5-4
Original file line numberDiff line numberDiff line change
@@ -181,10 +181,11 @@ If the Go runtime sees an existing signal handler for the SIGCANCEL or
181181
SIGSETXID signals (which are used only on GNU/Linux), it will turn on
182182
the SA_ONSTACK flag and otherwise keep the signal handler.
183183
184-
For the synchronous signals, the Go runtime will install a signal
185-
handler. It will save any existing signal handler. If a synchronous
186-
signal arrives while executing non-Go code, the Go runtime will invoke
187-
the existing signal handler instead of the Go signal handler.
184+
For the synchronous signals and SIGPIPE, the Go runtime will install a
185+
signal handler. It will save any existing signal handler. If a
186+
synchronous signal arrives while executing non-Go code, the Go runtime
187+
will invoke the existing signal handler instead of the Go signal
188+
handler.
188189
189190
Go code built with -buildmode=c-archive or -buildmode=c-shared will
190191
not install any other signal handlers by default. If there is an

src/runtime/signal_unix.go

+10-4
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,8 @@ func sigInstallGoHandler(sig uint32) bool {
111111
}
112112

113113
// When built using c-archive or c-shared, only install signal
114-
// handlers for synchronous signals.
115-
if (isarchive || islibrary) && t.flags&_SigPanic == 0 {
114+
// handlers for synchronous signals and SIGPIPE.
115+
if (isarchive || islibrary) && t.flags&_SigPanic == 0 && sig != _SIGPIPE {
116116
return false
117117
}
118118

@@ -492,9 +492,15 @@ func sigfwdgo(sig uint32, info *siginfo, ctx unsafe.Pointer) bool {
492492
return true
493493
}
494494

495-
// Only forward synchronous signals.
496495
c := &sigctxt{info, ctx}
497-
if c.sigcode() == _SI_USER || flags&_SigPanic == 0 {
496+
// Only forward signals from the kernel.
497+
// On Linux and Darwin there is no way to distinguish a SIGPIPE raised by a write
498+
// to a closed socket or pipe from a SIGPIPE raised by kill or pthread_kill
499+
// so we'll treat every SIGPIPE as kernel-generated.
500+
userSig := c.sigcode() == _SI_USER &&
501+
(sig != _SIGPIPE || GOOS != "linux" && GOOS != "android" && GOOS != "darwin")
502+
// Only forward synchronous signals and SIGPIPE.
503+
if userSig || flags&_SigPanic == 0 && sig != _SIGPIPE {
498504
return false
499505
}
500506
// Determine if the signal occurred inside Go code. We test that:

0 commit comments

Comments
 (0)