]> Cypherpunks repositories - gostls13.git/commitdiff
runtime: work around Apple libc bugs to make exec stop hanging
authorRuss Cox <rsc@golang.org>
Thu, 17 Nov 2022 19:24:06 +0000 (14:24 -0500)
committerGopher Robot <gobot@golang.org>
Thu, 17 Nov 2022 21:15:36 +0000 (21:15 +0000)
For a while now, we've had intermittent reports about problems with
os/exec on macOS, but no clear way to reproduce them. Recent changes
in the os/exec package test seem to have aligned the stars just right,
at least on my two x86 and ARM MacBook Pro laptops, to make the
package test hang with roughly 50% probability. When it does hang, the
stacks I see in the hung process match the ones reported for the
Go-based hangs in #33565. (They do not match the ones reported in the
so-called C reproducer in that issue, but I think that reproducer is
actually reproducing a different race, between fork and exit.)

The stacks obtained from the hung child processes are in
libSystem_atfork_child, which is supposed to reinitialize various
parts of the C library in the new process.

One common stack dies in _notify_fork_child calling _notify_globals
(inlined) calling _os_alloc_once, because _os_alloc_once detects that
the once lock is held by the parent process and then calls
_os_once_gate_corruption_abort. The allocation is setting up the
globals for the notification subsystem. See the source code at [1].
To work around this, we can allocate the globals earlier in the Go
program's lifetime, before any execs are involved, by calling any
notify routine that is exported, calls _notify_globals, and doesn't do
anything too expensive otherwise. notify_is_valid_token(0) fits the bill.

The other common stack dies in xpc_atfork_child calling
_objc_msgSend_uncached which ends up in
WAITING_FOR_ANOTHER_THREAD_TO_FINISH_CALLING_+initialize. Of course,
whatever thread the child is waiting for is in the parent process and
is not going to finish anything in the child process. There is no
public source code for these routines, so it is unclear exactly what
the problem is. However, xpc_atfork_child turns out to be exported
(for use by libSystem_atfork_child, which is in a different library,
so xpc_atfork_child is unlikely to be unexported any time soon).
It also stands to reason that since xpc_atfork_child is called at the
start of any forked child process, it can't be too harmful to call at
the start of an ordinary Go process. And whatever caches it needs for
a non-deadlocking fast path during exec empirically do get initialized
by calling it at startup.

This CL introduces a function osinit_hack, called at osinit time,
which calls notify_is_valid_token(0) and xpc_atfork_child().
Doing so makes the os/exec test pass reliably on both my laptops -
I can run it successfully hundreds of times in a row when my previous
record was twice in a row.

Fixes #33565.
Fixes #56784.

[1] https://opensource.apple.com/source/Libnotify/Libnotify-241/notify_client.c.auto.html

Change-Id: I16a14a800893c40244678203532a3e8d6214b6bd
Reviewed-on: https://go-review.googlesource.com/c/go/+/451735
Run-TryBot: Russ Cox <rsc@golang.org>
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Cherry Mui <cherryyz@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/runtime/os_darwin.go
src/runtime/sys_darwin.go
src/runtime/sys_darwin_amd64.s
src/runtime/sys_darwin_arm64.s

index af5c18c30195c1b70c4aa173c79972b709d34789..c4f3bb6a818585ca78284c39cc4011fbeaabe141 100644 (file)
@@ -136,6 +136,8 @@ func osinit() {
 
        ncpu = getncpu()
        physPageSize = getPageSize()
+
+       osinit_hack()
 }
 
 func sysctlbynameInt32(name []byte) (int32, int32) {
index 61b7f8c728ffab4197c9c60ed09a2303d654e7e2..28dc2915966c4b2655610d5338a10662b28d74dd 100644 (file)
@@ -179,6 +179,51 @@ func pthread_kill(t pthread, sig uint32) {
 }
 func pthread_kill_trampoline()
 
+// osinit_hack is a clumsy hack to work around Apple libc bugs
+// causing fork+exec to hang in the child process intermittently.
+// See go.dev/issue/33565 and go.dev/issue/56784 for a few reports.
+//
+// The stacks obtained from the hung child processes are in
+// libSystem_atfork_child, which is supposed to reinitialize various
+// parts of the C library in the new process.
+//
+// One common stack dies in _notify_fork_child calling _notify_globals
+// (inlined) calling _os_alloc_once, because _os_alloc_once detects that
+// the once lock is held by the parent process and then calls
+// _os_once_gate_corruption_abort. The allocation is setting up the
+// globals for the notification subsystem. See the source code at [1].
+// To work around this, we can allocate the globals earlier in the Go
+// program's lifetime, before any execs are involved, by calling any
+// notify routine that is exported, calls _notify_globals, and doesn't do
+// anything too expensive otherwise. notify_is_valid_token(0) fits the bill.
+//
+// The other common stack dies in xpc_atfork_child calling
+// _objc_msgSend_uncached which ends up in
+// WAITING_FOR_ANOTHER_THREAD_TO_FINISH_CALLING_+initialize. Of course,
+// whatever thread the child is waiting for is in the parent process and
+// is not going to finish anything in the child process. There is no
+// public source code for these routines, so it is unclear exactly what
+// the problem is. However, xpc_atfork_child turns out to be exported
+// (for use by libSystem_atfork_child, which is in a different library,
+// so xpc_atfork_child is unlikely to be unexported any time soon).
+// It also stands to reason that since xpc_atfork_child is called at the
+// start of any forked child process, it can't be too harmful to call at
+// the start of an ordinary Go process. And whatever caches it needs for
+// a non-deadlocking fast path during exec empirically do get initialized
+// by calling it at startup.
+//
+// So osinit_hack_trampoline (in sys_darwin_$GOARCH.s) calls
+// notify_is_valid_token(0) and xpc_atfork_child(), which makes the
+// fork+exec hangs stop happening. If Apple fixes the libc bug in
+// some future version of macOS, then we can remove this awful code.
+//
+//go:nosplit
+func osinit_hack() {
+       libcCall(unsafe.Pointer(abi.FuncPCABI0(osinit_hack_trampoline)), nil)
+       return
+}
+func osinit_hack_trampoline()
+
 // mmap is used to do low-level memory allocation via mmap. Don't allow stack
 // splits, since this function (used by sysAlloc) is called in a lot of low-level
 // parts of the runtime and callers often assume it won't acquire any locks.
@@ -548,3 +593,6 @@ func setNonblock(fd int32) {
 //go:cgo_import_dynamic libc_pthread_cond_wait pthread_cond_wait "/usr/lib/libSystem.B.dylib"
 //go:cgo_import_dynamic libc_pthread_cond_timedwait_relative_np pthread_cond_timedwait_relative_np "/usr/lib/libSystem.B.dylib"
 //go:cgo_import_dynamic libc_pthread_cond_signal pthread_cond_signal "/usr/lib/libSystem.B.dylib"
+
+//go:cgo_import_dynamic libc_notify_is_valid_token notify_is_valid_token "/usr/lib/libSystem.B.dylib"
+//go:cgo_import_dynamic libc_xpc_atfork_child xpc_atfork_child "/usr/lib/libSystem.B.dylib"
index 369b12e8f9f86d86b3b9f63d432c5abaa01f9a11..16783a481941eec1881a704d41af791288978b47 100644 (file)
@@ -597,6 +597,15 @@ TEXT runtime·pthread_kill_trampoline(SB),NOSPLIT,$0
        POPQ    BP
        RET
 
+TEXT runtime·osinit_hack_trampoline(SB),NOSPLIT,$0
+       PUSHQ   BP
+       MOVQ    SP, BP
+       MOVQ    $0, DI  // arg 1 val
+       CALL    libc_notify_is_valid_token(SB)
+       CALL    libc_xpc_atfork_child(SB)
+       POPQ    BP
+       RET
+
 // syscall calls a function in libc on behalf of the syscall package.
 // syscall takes a pointer to a struct like:
 // struct {
index 4fa99cc0f98cff8d8f02b9a8eb446239aeb67e47..3cbac773941060585c6933d8820881c703e9e70c 100644 (file)
@@ -458,6 +458,12 @@ TEXT runtime·pthread_setspecific_trampoline(SB),NOSPLIT,$0
        BL      libc_pthread_setspecific(SB)
        RET
 
+TEXT runtime·osinit_hack_trampoline(SB),NOSPLIT,$0
+       MOVD    $0, R0  // arg 1 val
+       BL      libc_notify_is_valid_token(SB)
+       BL      libc_xpc_atfork_child(SB)
+       RET
+
 // syscall calls a function in libc on behalf of the syscall package.
 // syscall takes a pointer to a struct like:
 // struct {