Skip to content

Commit 91bc4cd

Browse files
committed
[release-branch.go1.19] runtime: call __fork instead of fork on darwin
Issues #33565 and #56784 were caused by hangs in the child process after fork, while it ran atfork handlers that ran into slow paths that didn't work in the child. CL 451735 worked around those two issues by calling a couple functions at startup to try to warm up those child paths. That mostly worked, but it broke programs using cgo with certain macOS frameworks (#57263). CL 459175 reverted CL 451735. This CL introduces a different fix: bypass the atfork child handlers entirely. For a general fork call where the child and parent are both meant to keep executing the original program, atfork handlers can be necessary to fix any state that would otherwise be tied to the parent process. But Go only uses fork as preparation for exec, and it takes care to limit what it attempts to do in the child between the fork and exec. In particular it doesn't use any of the things that the macOS atfork handlers are trying to fix up (malloc, xpc, others). So we can use the low-level fork system call (__fork) instead of the atfork-wrapped one. The full list of functions that can be called in a child after fork in exec_libc2.go is: - ptrace - setsid - setpgid - getpid - ioctl - chroot - setgroups - setgid - setuid - chdir - dup2 - fcntl - close - execve - write - exit I disassembled all of these while attached to a hung exec.test binary and confirmed that nearly all of them are making direct kernel calls, not using anything that the atfork handler needs to fix up. The exceptions are ioctl, fcntl, and exit. The ioctl and fcntl implementations do some extra work around the kernel call but don't call any other functions, so they should still be OK. (If not, we could use __ioctl and __fcntl instead, but without a good reason, we should keep using the standard entry points.) The exit implementation calls atexit handlers. That is almost certainly inappropriate in a failed fork child, so this CL changes that call to __exit on darwin. To avoid making unnecessary changes at this point in the release cycle, this CL leaves OpenBSD calling plain exit, even though that is probably a bug in the OpenBSD port (filed #57446). Fixes #33565. Fixes #56784. Fixes #57263. Fixes #56837. Change-Id: I26812c26a72bdd7fcf72ec41899ba11cf6b9c4ab Reviewed-on: https://go-review.googlesource.com/c/go/+/459176 Reviewed-by: David Chase <[email protected]> Reviewed-by: Cherry Mui <[email protected]> TryBot-Result: Gopher Robot <[email protected]> Run-TryBot: Russ Cox <[email protected]> Reviewed-on: https://go-review.googlesource.com/c/go/+/459178
1 parent 41eb70a commit 91bc4cd

7 files changed

+81
-17
lines changed

src/syscall/exec_libc2.go

+2-2
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ func forkAndExecInChild(argv0 *byte, argv, envv []*byte, chroot, dir *byte, attr
7878
// About to call fork.
7979
// No more allocation or calls of non-assembly functions.
8080
runtime_BeforeFork()
81-
r1, _, err1 = rawSyscall(abi.FuncPCABI0(libc_fork_trampoline), 0, 0, 0)
81+
r1, _, err1 = rawSyscall(forkTrampoline, 0, 0, 0)
8282
if err1 != 0 {
8383
runtime_AfterFork()
8484
return 0, err1
@@ -276,6 +276,6 @@ childerror:
276276
// send error code on pipe
277277
rawSyscall(abi.FuncPCABI0(libc_write_trampoline), uintptr(pipe), uintptr(unsafe.Pointer(&err1)), unsafe.Sizeof(err1))
278278
for {
279-
rawSyscall(abi.FuncPCABI0(libc_exit_trampoline), 253, 0, 0)
279+
rawSyscall(exitTrampoline, 253, 0, 0)
280280
}
281281
}

src/syscall/syscall_darwin.go

+30-2
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,34 @@ func Syscall6(trap, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2 uintptr, err Errno)
2222
func RawSyscall(trap, a1, a2, a3 uintptr) (r1, r2 uintptr, err Errno)
2323
func RawSyscall6(trap, a1, a2, a3, a4, a5, a6 uintptr) (r1, r2 uintptr, err Errno)
2424

25-
var dupTrampoline = abi.FuncPCABI0(libc_dup2_trampoline)
25+
// These are called from exec_libc2.go in the child of fork.
26+
// The names differ between macOS and OpenBSD, so we need
27+
// to declare the specific ones used here to keep the exec_libc2.go
28+
// code portable.
29+
//
30+
// We use __fork and __exit, not fork and exit, to avoid the libc atfork
31+
// and atexit handlers. The atfork handlers have caused fork child
32+
// hangs in the past (see #33565, #56784). The atexit handlers have
33+
// not, but the non-libc ports all invoke the system call, so doing
34+
// the same here makes sense. In general we wouldn't expect
35+
// atexit handlers to work terribly well in a fork child anyway.
36+
// (Also, perhaps the atfork handlers clear the atexit handlers,
37+
// in which case we definitely need to avoid calling the libc exit
38+
// if we bypass the libc fork.)
39+
//
40+
// Other calls that are made in the child after the fork are
41+
// ptrace, setsid, setpgid, getpid, ioctl, chroot, setgroups,
42+
// setgid, setuid, chdir, dup2, fcntl, close, execve, and write.
43+
// Those are all simple kernel wrappers that should be safe
44+
// to be called directly. The fcntl and ioctl functions do run
45+
// some code around the kernel call, but they don't call any
46+
// other functions, so for now we keep using them instead of
47+
// calling the lower-level __fcntl and __ioctl functions.
48+
var (
49+
dupTrampoline = abi.FuncPCABI0(libc_dup2_trampoline)
50+
exitTrampoline = abi.FuncPCABI0(libc___exit_trampoline)
51+
forkTrampoline = abi.FuncPCABI0(libc___fork_trampoline)
52+
)
2653

2754
type SockaddrDatalink struct {
2855
Len uint8
@@ -209,11 +236,12 @@ func Kill(pid int, signum Signal) (err error) { return kill(pid, int(signum), 1)
209236
//sys writev(fd int, iovecs []Iovec) (cnt uintptr, err error)
210237
//sys mmap(addr uintptr, length uintptr, prot int, flag int, fd int, pos int64) (ret uintptr, err error)
211238
//sys munmap(addr uintptr, length uintptr) (err error)
212-
//sysnb fork() (pid int, err error)
239+
//sysnb __fork() (pid int, err error)
213240
//sysnb ioctl(fd int, req int, arg int) (err error)
214241
//sysnb ioctlPtr(fd int, req uint, arg unsafe.Pointer) (err error) = SYS_ioctl
215242
//sysnb execve(path *byte, argv **byte, envp **byte) (err error)
216243
//sysnb exit(res int) (err error)
244+
//sysnb __exit(res int) (err error)
217245
//sys sysctl(mib []_C_int, old *byte, oldlen *uintptr, new *byte, newlen uintptr) (err error)
218246
//sys fcntlPtr(fd int, cmd int, arg unsafe.Pointer) (val int, err error) = SYS_fcntl
219247
//sys unlinkat(fd int, path string, flags int) (err error)

src/syscall/syscall_openbsd_libc.go

+5-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,11 @@ import (
1010
"internal/abi"
1111
)
1212

13-
var dupTrampoline = abi.FuncPCABI0(libc_dup3_trampoline)
13+
var (
14+
dupTrampoline = abi.FuncPCABI0(libc_dup3_trampoline)
15+
exitTrampoline = abi.FuncPCABI0(libc_exit_trampoline)
16+
forkTrampoline = abi.FuncPCABI0(libc_fork_trampoline)
17+
)
1418

1519
func init() {
1620
execveOpenBSD = execve

src/syscall/zsyscall_darwin_amd64.go

+18-4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/syscall/zsyscall_darwin_amd64.s

+4-2
Original file line numberDiff line numberDiff line change
@@ -219,14 +219,16 @@ TEXT ·libc_mmap_trampoline(SB),NOSPLIT,$0-0
219219
JMP libc_mmap(SB)
220220
TEXT ·libc_munmap_trampoline(SB),NOSPLIT,$0-0
221221
JMP libc_munmap(SB)
222-
TEXT ·libc_fork_trampoline(SB),NOSPLIT,$0-0
223-
JMP libc_fork(SB)
222+
TEXT ·libc___fork_trampoline(SB),NOSPLIT,$0-0
223+
JMP libc___fork(SB)
224224
TEXT ·libc_ioctl_trampoline(SB),NOSPLIT,$0-0
225225
JMP libc_ioctl(SB)
226226
TEXT ·libc_execve_trampoline(SB),NOSPLIT,$0-0
227227
JMP libc_execve(SB)
228228
TEXT ·libc_exit_trampoline(SB),NOSPLIT,$0-0
229229
JMP libc_exit(SB)
230+
TEXT ·libc___exit_trampoline(SB),NOSPLIT,$0-0
231+
JMP libc___exit(SB)
230232
TEXT ·libc_sysctl_trampoline(SB),NOSPLIT,$0-0
231233
JMP libc_sysctl(SB)
232234
TEXT ·libc_unlinkat_trampoline(SB),NOSPLIT,$0-0

src/syscall/zsyscall_darwin_arm64.go

+18-4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/syscall/zsyscall_darwin_arm64.s

+4-2
Original file line numberDiff line numberDiff line change
@@ -219,14 +219,16 @@ TEXT ·libc_mmap_trampoline(SB),NOSPLIT,$0-0
219219
JMP libc_mmap(SB)
220220
TEXT ·libc_munmap_trampoline(SB),NOSPLIT,$0-0
221221
JMP libc_munmap(SB)
222-
TEXT ·libc_fork_trampoline(SB),NOSPLIT,$0-0
223-
JMP libc_fork(SB)
222+
TEXT ·libc___fork_trampoline(SB),NOSPLIT,$0-0
223+
JMP libc___fork(SB)
224224
TEXT ·libc_ioctl_trampoline(SB),NOSPLIT,$0-0
225225
JMP libc_ioctl(SB)
226226
TEXT ·libc_execve_trampoline(SB),NOSPLIT,$0-0
227227
JMP libc_execve(SB)
228228
TEXT ·libc_exit_trampoline(SB),NOSPLIT,$0-0
229229
JMP libc_exit(SB)
230+
TEXT ·libc___exit_trampoline(SB),NOSPLIT,$0-0
231+
JMP libc___exit(SB)
230232
TEXT ·libc_sysctl_trampoline(SB),NOSPLIT,$0-0
231233
JMP libc_sysctl(SB)
232234
TEXT ·libc_unlinkat_trampoline(SB),NOSPLIT,$0-0

0 commit comments

Comments
 (0)