Skip to content

Commit a69f977

Browse files
ianlancetaylortmm1
authored andcommitted
internal/poll, os: loop on EINTR
Historically we've assumed that we can install all signal handlers with the SA_RESTART flag set, and let the system restart slow functions if a signal is received. Therefore, we don't have to worry about EINTR. This is only partially true, and we've added EINTR checks already for connect, and open/read on Darwin, and sendfile on Solaris. Other cases have turned up in golang#36644, golang#38033, and golang#38836. Also, golang#20400 points out that when Go code is included in a C program, the C program may install its own signal handlers without SA_RESTART. In that case, Go code will see EINTR no matter what it does. So, go ahead and check for EINTR. We don't check in the syscall package; people using syscalls directly may want to check for EINTR themselves. But we do check for EINTR in the higher level APIs in os and net, and retry the system call if we see it. This change looks safe, but of course we may be missing some cases where we need to check for EINTR. As such cases turn up, we can add tests to runtime/testdata/testprogcgo/eintr.go, and fix the code. If there are any such cases, their handling after this change will be no worse than it is today. For golang#22838 Fixes golang#20400 Fixes golang#36644 Fixes golang#38033 Fixes golang#38836 Change-Id: I7e46ca8cafed0429c7a2386cc9edc9d9d47a6896 Reviewed-on: https://go-review.googlesource.com/c/go/+/232862 Run-TryBot: Ian Lance Taylor <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Bryan C. Mills <[email protected]> (cherry picked from commit 8c1db77)
1 parent fda1e04 commit a69f977

File tree

13 files changed

+370
-29
lines changed

13 files changed

+370
-29
lines changed

src/internal/poll/fd_unix.go

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ package poll
88

99
import (
1010
"io"
11-
"runtime"
1211
"sync/atomic"
1312
"syscall"
1413
)
@@ -162,20 +161,14 @@ func (fd *FD) Read(p []byte) (int, error) {
162161
p = p[:maxRW]
163162
}
164163
for {
165-
n, err := syscall.Read(fd.Sysfd, p)
164+
n, err := ignoringEINTR(syscall.Read, fd.Sysfd, p)
166165
if err != nil {
167166
n = 0
168167
if err == syscall.EAGAIN && fd.pd.pollable() {
169168
if err = fd.pd.waitRead(fd.isFile); err == nil {
170169
continue
171170
}
172171
}
173-
174-
// On MacOS we can see EINTR here if the user
175-
// pressed ^Z. See issue #22838.
176-
if runtime.GOOS == "darwin" && err == syscall.EINTR {
177-
continue
178-
}
179172
}
180173
err = fd.eofError(n, err)
181174
return n, err
@@ -193,7 +186,16 @@ func (fd *FD) Pread(p []byte, off int64) (int, error) {
193186
if fd.IsStream && len(p) > maxRW {
194187
p = p[:maxRW]
195188
}
196-
n, err := syscall.Pread(fd.Sysfd, p, off)
189+
var (
190+
n int
191+
err error
192+
)
193+
for {
194+
n, err = syscall.Pread(fd.Sysfd, p, off)
195+
if err != syscall.EINTR {
196+
break
197+
}
198+
}
197199
if err != nil {
198200
n = 0
199201
}
@@ -214,6 +216,9 @@ func (fd *FD) ReadFrom(p []byte) (int, syscall.Sockaddr, error) {
214216
for {
215217
n, sa, err := syscall.Recvfrom(fd.Sysfd, p, 0)
216218
if err != nil {
219+
if err == syscall.EINTR {
220+
continue
221+
}
217222
n = 0
218223
if err == syscall.EAGAIN && fd.pd.pollable() {
219224
if err = fd.pd.waitRead(fd.isFile); err == nil {
@@ -238,6 +243,9 @@ func (fd *FD) ReadMsg(p []byte, oob []byte) (int, int, int, syscall.Sockaddr, er
238243
for {
239244
n, oobn, flags, sa, err := syscall.Recvmsg(fd.Sysfd, p, oob, 0)
240245
if err != nil {
246+
if err == syscall.EINTR {
247+
continue
248+
}
241249
// TODO(dfc) should n and oobn be set to 0
242250
if err == syscall.EAGAIN && fd.pd.pollable() {
243251
if err = fd.pd.waitRead(fd.isFile); err == nil {
@@ -265,7 +273,7 @@ func (fd *FD) Write(p []byte) (int, error) {
265273
if fd.IsStream && max-nn > maxRW {
266274
max = nn + maxRW
267275
}
268-
n, err := syscall.Write(fd.Sysfd, p[nn:max])
276+
n, err := ignoringEINTR(syscall.Write, fd.Sysfd, p[nn:max])
269277
if n > 0 {
270278
nn += n
271279
}
@@ -302,6 +310,9 @@ func (fd *FD) Pwrite(p []byte, off int64) (int, error) {
302310
max = nn + maxRW
303311
}
304312
n, err := syscall.Pwrite(fd.Sysfd, p[nn:max], off+int64(nn))
313+
if err == syscall.EINTR {
314+
continue
315+
}
305316
if n > 0 {
306317
nn += n
307318
}
@@ -328,6 +339,9 @@ func (fd *FD) WriteTo(p []byte, sa syscall.Sockaddr) (int, error) {
328339
}
329340
for {
330341
err := syscall.Sendto(fd.Sysfd, p, 0, sa)
342+
if err == syscall.EINTR {
343+
continue
344+
}
331345
if err == syscall.EAGAIN && fd.pd.pollable() {
332346
if err = fd.pd.waitWrite(fd.isFile); err == nil {
333347
continue
@@ -351,6 +365,9 @@ func (fd *FD) WriteMsg(p []byte, oob []byte, sa syscall.Sockaddr) (int, int, err
351365
}
352366
for {
353367
n, err := syscall.SendmsgN(fd.Sysfd, p, oob, sa, 0)
368+
if err == syscall.EINTR {
369+
continue
370+
}
354371
if err == syscall.EAGAIN && fd.pd.pollable() {
355372
if err = fd.pd.waitWrite(fd.isFile); err == nil {
356373
continue
@@ -379,6 +396,8 @@ func (fd *FD) Accept() (int, syscall.Sockaddr, string, error) {
379396
return s, rsa, "", err
380397
}
381398
switch err {
399+
case syscall.EINTR:
400+
continue
382401
case syscall.EAGAIN:
383402
if fd.pd.pollable() {
384403
if err = fd.pd.waitRead(fd.isFile); err == nil {
@@ -413,7 +432,7 @@ func (fd *FD) ReadDirent(buf []byte) (int, error) {
413432
}
414433
defer fd.decref()
415434
for {
416-
n, err := syscall.ReadDirent(fd.Sysfd, buf)
435+
n, err := ignoringEINTR(syscall.ReadDirent, fd.Sysfd, buf)
417436
if err != nil {
418437
n = 0
419438
if err == syscall.EAGAIN && fd.pd.pollable() {
@@ -504,7 +523,7 @@ func (fd *FD) WriteOnce(p []byte) (int, error) {
504523
return 0, err
505524
}
506525
defer fd.writeUnlock()
507-
return syscall.Write(fd.Sysfd, p)
526+
return ignoringEINTR(syscall.Write, fd.Sysfd, p)
508527
}
509528

510529
// RawControl invokes the user-defined function f for a non-IO
@@ -555,3 +574,19 @@ func (fd *FD) RawWrite(f func(uintptr) bool) error {
555574
}
556575
}
557576
}
577+
578+
// ignoringEINTR makes a function call and repeats it if it returns
579+
// an EINTR error. This appears to be required even though we install
580+
// all signal handlers with SA_RESTART: see #22838, #38033, #38836.
581+
// Also #20400 and #36644 are issues in which a signal handler is
582+
// installed without setting SA_RESTART. None of these are the common case,
583+
// but there are enough of them that it seems that we can't avoid
584+
// an EINTR loop.
585+
func ignoringEINTR(fn func(fd int, p []byte) (int, error), fd int, p []byte) (int, error) {
586+
for {
587+
n, err := fn(fd, p)
588+
if err != syscall.EINTR {
589+
return n, err
590+
}
591+
}
592+
}

src/internal/poll/fd_writev_unix.go

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,18 @@ import (
1212
)
1313

1414
func writev(fd int, iovecs []syscall.Iovec) (uintptr, error) {
15-
r, _, e := syscall.Syscall(syscall.SYS_WRITEV, uintptr(fd), uintptr(unsafe.Pointer(&iovecs[0])), uintptr(len(iovecs)))
15+
var (
16+
r uintptr
17+
e syscall.Errno
18+
)
19+
for {
20+
r, _, e = syscall.Syscall(syscall.SYS_WRITEV, uintptr(fd), uintptr(unsafe.Pointer(&iovecs[0])), uintptr(len(iovecs)))
21+
if e != syscall.EINTR {
22+
break
23+
}
24+
}
1625
if e != 0 {
17-
return r, syscall.Errno(e)
26+
return r, e
1827
}
1928
return r, nil
2029
}

src/internal/poll/sendfile_bsd.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,9 @@ func SendFile(dstFD *FD, src int, pos, remain int64) (int64, error) {
3535
} else if n == 0 && err1 == nil {
3636
break
3737
}
38+
if err1 == syscall.EINTR {
39+
continue
40+
}
3841
if err1 == syscall.EAGAIN {
3942
if err1 = dstFD.pd.waitWrite(dstFD.isFile); err1 == nil {
4043
continue

src/internal/poll/sendfile_linux.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,9 @@ func SendFile(dstFD *FD, src int, remain int64) (int64, error) {
3232
} else if n == 0 && err1 == nil {
3333
break
3434
}
35+
if err1 == syscall.EINTR {
36+
continue
37+
}
3538
if err1 == syscall.EAGAIN {
3639
if err1 = dstFD.pd.waitWrite(dstFD.isFile); err1 == nil {
3740
continue

src/internal/poll/splice_linux.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,9 @@ func spliceDrain(pipefd int, sock *FD, max int) (int, error) {
8787
}
8888
for {
8989
n, err := splice(pipefd, sock.Sysfd, max, spliceNonblock)
90+
if err == syscall.EINTR {
91+
continue
92+
}
9093
if err != syscall.EAGAIN {
9194
return n, err
9295
}

src/internal/poll/writev.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,10 @@ func (fd *FD) Writev(v *[][]byte) (int64, error) {
6868
iovecs[i] = syscall.Iovec{}
6969
}
7070
if err != nil {
71-
if err.(syscall.Errno) == syscall.EAGAIN {
71+
if err == syscall.EINTR {
72+
continue
73+
}
74+
if err == syscall.EAGAIN {
7275
if err = fd.pd.waitWrite(fd.isFile); err == nil {
7376
continue
7477
}

src/os/exec_unix.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -33,9 +33,18 @@ func (p *Process) wait() (ps *ProcessState, err error) {
3333
p.sigMu.Unlock()
3434
}
3535

36-
var status syscall.WaitStatus
37-
var rusage syscall.Rusage
38-
pid1, e := syscall.Wait4(p.Pid, &status, 0, &rusage)
36+
var (
37+
status syscall.WaitStatus
38+
rusage syscall.Rusage
39+
pid1 int
40+
e error
41+
)
42+
for {
43+
pid1, e = syscall.Wait4(p.Pid, &status, 0, &rusage)
44+
if e != syscall.EINTR {
45+
break
46+
}
47+
}
3948
if e != nil {
4049
return nil, NewSyscallError("wait", e)
4150
}

src/os/wait_wait6.go

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,20 @@ const _P_PID = 0
1818
// It does not actually call p.Wait.
1919
func (p *Process) blockUntilWaitable() (bool, error) {
2020
var errno syscall.Errno
21-
// The arguments on 32-bit FreeBSD look like the following:
22-
// - freebsd32_wait6_args{ idtype, id1, id2, status, options, wrusage, info } or
23-
// - freebsd32_wait6_args{ idtype, pad, id1, id2, status, options, wrusage, info } when PAD64_REQUIRED=1 on ARM, MIPS or PowerPC
24-
if runtime.GOARCH == "386" {
25-
_, _, errno = syscall.Syscall9(syscall.SYS_WAIT6, _P_PID, uintptr(p.Pid), 0, 0, syscall.WEXITED|syscall.WNOWAIT, 0, 0, 0, 0)
26-
} else if runtime.GOARCH == "arm" {
27-
_, _, errno = syscall.Syscall9(syscall.SYS_WAIT6, _P_PID, 0, uintptr(p.Pid), 0, 0, syscall.WEXITED|syscall.WNOWAIT, 0, 0, 0)
28-
} else {
29-
_, _, errno = syscall.Syscall6(syscall.SYS_WAIT6, _P_PID, uintptr(p.Pid), 0, syscall.WEXITED|syscall.WNOWAIT, 0, 0)
21+
for {
22+
// The arguments on 32-bit FreeBSD look like the following:
23+
// - freebsd32_wait6_args{ idtype, id1, id2, status, options, wrusage, info } or
24+
// - freebsd32_wait6_args{ idtype, pad, id1, id2, status, options, wrusage, info } when PAD64_REQUIRED=1 on ARM, MIPS or PowerPC
25+
if runtime.GOARCH == "386" {
26+
_, _, errno = syscall.Syscall9(syscall.SYS_WAIT6, _P_PID, uintptr(p.Pid), 0, 0, syscall.WEXITED|syscall.WNOWAIT, 0, 0, 0, 0)
27+
} else if runtime.GOARCH == "arm" {
28+
_, _, errno = syscall.Syscall9(syscall.SYS_WAIT6, _P_PID, 0, uintptr(p.Pid), 0, 0, syscall.WEXITED|syscall.WNOWAIT, 0, 0, 0)
29+
} else {
30+
_, _, errno = syscall.Syscall6(syscall.SYS_WAIT6, _P_PID, uintptr(p.Pid), 0, syscall.WEXITED|syscall.WNOWAIT, 0, 0)
31+
}
32+
if errno != syscall.EINTR {
33+
break
34+
}
3035
}
3136
runtime.KeepAlive(p)
3237
if errno != 0 {

src/os/wait_waitid.go

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,13 @@ func (p *Process) blockUntilWaitable() (bool, error) {
2828
// We don't care about the values it returns.
2929
var siginfo [16]uint64
3030
psig := &siginfo[0]
31-
_, _, e := syscall.Syscall6(syscall.SYS_WAITID, _P_PID, uintptr(p.Pid), uintptr(unsafe.Pointer(psig)), syscall.WEXITED|syscall.WNOWAIT, 0, 0)
31+
var e syscall.Errno
32+
for {
33+
_, _, e = syscall.Syscall6(syscall.SYS_WAITID, _P_PID, uintptr(p.Pid), uintptr(unsafe.Pointer(psig)), syscall.WEXITED|syscall.WNOWAIT, 0, 0)
34+
if e != syscall.EINTR {
35+
break
36+
}
37+
}
3238
runtime.KeepAlive(p)
3339
if e != 0 {
3440
// waitid has been available since Linux 2.6.9, but

src/runtime/crash_cgo_test.go

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -549,3 +549,48 @@ func findTrace(text, top string) []string {
549549
}
550550
return nil
551551
}
552+
553+
func TestSegv(t *testing.T) {
554+
switch runtime.GOOS {
555+
case "plan9", "windows":
556+
t.Skipf("no signals on %s", runtime.GOOS)
557+
}
558+
559+
for _, test := range []string{"Segv", "SegvInCgo"} {
560+
t.Run(test, func(t *testing.T) {
561+
t.Parallel()
562+
got := runTestProg(t, "testprogcgo", test)
563+
t.Log(got)
564+
if !strings.Contains(got, "SIGSEGV") {
565+
t.Errorf("expected crash from signal")
566+
}
567+
})
568+
}
569+
}
570+
571+
// TestEINTR tests that we handle EINTR correctly.
572+
// See issue #20400 and friends.
573+
func TestEINTR(t *testing.T) {
574+
switch runtime.GOOS {
575+
case "plan9", "windows":
576+
t.Skipf("no EINTR on %s", runtime.GOOS)
577+
case "linux":
578+
if runtime.GOARCH == "386" {
579+
// On linux-386 the Go signal handler sets
580+
// a restorer function that is not preserved
581+
// by the C sigaction call in the test,
582+
// causing the signal handler to crash when
583+
// returning the normal code. The test is not
584+
// architecture-specific, so just skip on 386
585+
// rather than doing a complicated workaround.
586+
t.Skip("skipping on linux-386; C sigaction does not preserve Go restorer")
587+
}
588+
}
589+
590+
t.Parallel()
591+
output := runTestProg(t, "testprogcgo", "EINTR")
592+
want := "OK\n"
593+
if output != want {
594+
t.Fatalf("want %s, got %s\n", want, output)
595+
}
596+
}

0 commit comments

Comments
 (0)