From 8c1db77a92b1d17d3fe07999c5f20602a2080be9 Mon Sep 17 00:00:00 2001 From: Ian Lance Taylor Date: Thu, 7 May 2020 21:34:54 -0700 Subject: 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 #36644, #38033, and #38836. Also, #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 #22838 Fixes #20400 Fixes #36644 Fixes #38033 Fixes #38836 Change-Id: I7e46ca8cafed0429c7a2386cc9edc9d9d47a6896 Reviewed-on: https://go-review.googlesource.com/c/go/+/232862 Run-TryBot: Ian Lance Taylor TryBot-Result: Gobot Gobot Reviewed-by: Bryan C. Mills --- src/os/exec_unix.go | 15 ++++++++++++--- src/os/wait_wait6.go | 23 ++++++++++++++--------- src/os/wait_waitid.go | 8 +++++++- 3 files changed, 33 insertions(+), 13 deletions(-) (limited to 'src/os') diff --git a/src/os/exec_unix.go b/src/os/exec_unix.go index 6e4ffe82d2..7759a2d2ea 100644 --- a/src/os/exec_unix.go +++ b/src/os/exec_unix.go @@ -33,9 +33,18 @@ func (p *Process) wait() (ps *ProcessState, err error) { p.sigMu.Unlock() } - var status syscall.WaitStatus - var rusage syscall.Rusage - pid1, e := syscall.Wait4(p.Pid, &status, 0, &rusage) + var ( + status syscall.WaitStatus + rusage syscall.Rusage + pid1 int + e error + ) + for { + pid1, e = syscall.Wait4(p.Pid, &status, 0, &rusage) + if e != syscall.EINTR { + break + } + } if e != nil { return nil, NewSyscallError("wait", e) } diff --git a/src/os/wait_wait6.go b/src/os/wait_wait6.go index 45bf649015..5420b2db73 100644 --- a/src/os/wait_wait6.go +++ b/src/os/wait_wait6.go @@ -18,15 +18,20 @@ const _P_PID = 0 // It does not actually call p.Wait. func (p *Process) blockUntilWaitable() (bool, error) { var errno syscall.Errno - // The arguments on 32-bit FreeBSD look like the following: - // - freebsd32_wait6_args{ idtype, id1, id2, status, options, wrusage, info } or - // - freebsd32_wait6_args{ idtype, pad, id1, id2, status, options, wrusage, info } when PAD64_REQUIRED=1 on ARM, MIPS or PowerPC - if runtime.GOARCH == "386" { - _, _, errno = syscall.Syscall9(syscall.SYS_WAIT6, _P_PID, uintptr(p.Pid), 0, 0, syscall.WEXITED|syscall.WNOWAIT, 0, 0, 0, 0) - } else if runtime.GOARCH == "arm" { - _, _, errno = syscall.Syscall9(syscall.SYS_WAIT6, _P_PID, 0, uintptr(p.Pid), 0, 0, syscall.WEXITED|syscall.WNOWAIT, 0, 0, 0) - } else { - _, _, errno = syscall.Syscall6(syscall.SYS_WAIT6, _P_PID, uintptr(p.Pid), 0, syscall.WEXITED|syscall.WNOWAIT, 0, 0) + for { + // The arguments on 32-bit FreeBSD look like the following: + // - freebsd32_wait6_args{ idtype, id1, id2, status, options, wrusage, info } or + // - freebsd32_wait6_args{ idtype, pad, id1, id2, status, options, wrusage, info } when PAD64_REQUIRED=1 on ARM, MIPS or PowerPC + if runtime.GOARCH == "386" { + _, _, errno = syscall.Syscall9(syscall.SYS_WAIT6, _P_PID, uintptr(p.Pid), 0, 0, syscall.WEXITED|syscall.WNOWAIT, 0, 0, 0, 0) + } else if runtime.GOARCH == "arm" { + _, _, errno = syscall.Syscall9(syscall.SYS_WAIT6, _P_PID, 0, uintptr(p.Pid), 0, 0, syscall.WEXITED|syscall.WNOWAIT, 0, 0, 0) + } else { + _, _, errno = syscall.Syscall6(syscall.SYS_WAIT6, _P_PID, uintptr(p.Pid), 0, syscall.WEXITED|syscall.WNOWAIT, 0, 0) + } + if errno != syscall.EINTR { + break + } } runtime.KeepAlive(p) if errno != 0 { diff --git a/src/os/wait_waitid.go b/src/os/wait_waitid.go index 6c904e54db..9c56eb2d41 100644 --- a/src/os/wait_waitid.go +++ b/src/os/wait_waitid.go @@ -27,7 +27,13 @@ func (p *Process) blockUntilWaitable() (bool, error) { // We don't care about the values it returns. var siginfo [16]uint64 psig := &siginfo[0] - _, _, e := syscall.Syscall6(syscall.SYS_WAITID, _P_PID, uintptr(p.Pid), uintptr(unsafe.Pointer(psig)), syscall.WEXITED|syscall.WNOWAIT, 0, 0) + var e syscall.Errno + for { + _, _, e = syscall.Syscall6(syscall.SYS_WAITID, _P_PID, uintptr(p.Pid), uintptr(unsafe.Pointer(psig)), syscall.WEXITED|syscall.WNOWAIT, 0, 0) + if e != syscall.EINTR { + break + } + } runtime.KeepAlive(p) if e != 0 { // waitid has been available since Linux 2.6.9, but -- cgit v1.2.1