From: Russ Cox Date: Thu, 22 Dec 2022 14:21:03 +0000 (-0500) Subject: runtime: revert Apple libc atfork workaround X-Git-Tag: go1.20rc2~1^2~7 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=6c9b661867;p=gostls13.git runtime: revert Apple libc atfork workaround Revert CL 451735 (1f4394a0c926), which fixed #33565 and #56784 but also introduced #57263. I have a different fix to apply instead. Since the first fix was never backported, it will be easiest to backport the new fix if the new fix is done in a separate CL from the revert. Change-Id: I6c8ea3a46e542ee4702675bbc058e29ccd2723e0 Reviewed-on: https://go-review.googlesource.com/c/go/+/459175 Reviewed-by: Cherry Mui TryBot-Result: Gopher Robot Run-TryBot: Russ Cox --- diff --git a/src/runtime/os_darwin.go b/src/runtime/os_darwin.go index c4f3bb6a81..af5c18c301 100644 --- a/src/runtime/os_darwin.go +++ b/src/runtime/os_darwin.go @@ -136,8 +136,6 @@ func osinit() { ncpu = getncpu() physPageSize = getPageSize() - - osinit_hack() } func sysctlbynameInt32(name []byte) (int32, int32) { diff --git a/src/runtime/sys_darwin.go b/src/runtime/sys_darwin.go index 28dc291596..61b7f8c728 100644 --- a/src/runtime/sys_darwin.go +++ b/src/runtime/sys_darwin.go @@ -179,51 +179,6 @@ 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. @@ -593,6 +548,3 @@ 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" diff --git a/src/runtime/sys_darwin_amd64.s b/src/runtime/sys_darwin_amd64.s index 16783a4819..369b12e8f9 100644 --- a/src/runtime/sys_darwin_amd64.s +++ b/src/runtime/sys_darwin_amd64.s @@ -597,15 +597,6 @@ 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 { diff --git a/src/runtime/sys_darwin_arm64.s b/src/runtime/sys_darwin_arm64.s index 3cbac77394..4fa99cc0f9 100644 --- a/src/runtime/sys_darwin_arm64.s +++ b/src/runtime/sys_darwin_arm64.s @@ -458,12 +458,6 @@ 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 {