Skip to content

fix(go-port,node-loader): fix macOS Go port race conditions and deadlocks (#942) - #963

Open
TheDoomBoy-jab wants to merge 2 commits into
metacall:developfrom
TheDoomBoy-jab:fix/go-port-macos-race-conditions
Open

TheDoomBoy-jab wants to merge 2 commits into
metacall:developfrom
TheDoomBoy-jab:fix/go-port-macos-race-conditions

Conversation

@TheDoomBoy-jab

@TheDoomBoy-jab TheDoomBoy-jab commented Nov 14, 2026 •

Copy link
Copy Markdown

Summary

This PR addresses the race conditions, crashes, and shutdown deadlocks observed when running the Go port test suite and benchmarks on macOS (Darwin).

Motive & Root Causes

  1. Dynamic library load failure (Reason: no LC_RPATH's found):
    On macOS, System Integrity Protection (SIP) automatically strips DYLD_LIBRARY_PATH from spawned child processes. Because CGO_LDFLAGS only passed -L${PROJECT_OUTPUT_DIR} -lmetacall, the compiled Go test binary lacked an embedded runpath to libmetacall.dylib.
  2. Crash/Abort in future_node_interface_delete:
    future_node_interface_delete uses loader_impl_async_safe_type on the stack to wait for the V8 thread to run the deletion callback via uv_cond_wait(&cond, &mutex). On macOS, Go's runtime routinely dispatches SIGURG signals across threads for non-cooperative goroutine preemption. Because uv_cond_wait was not wrapped in a predicate loop, SIGURG induced spurious condition wakeups, causing the caller stack to unwind and destroy the uv_mutex_t before the V8 thread executed uv_mutex_lock (triggering an abort trap).
  3. Promise reference lifecycle in node future:
    In node_loader_impl_napi_to_value, node_future->promise_ref was not initialized prior to future_create, and a failure in value_create_future did not clean up f.
  4. Premature shutdown in TestGoRoutineLeaks:
    TestGoRoutineLeaks explicitly calls Destroy() (metacall_destroy()) to test concurrent queue shutdown. In Go, go test runs unit tests before benchmarks in the same process. Terminating the runtime in TestGoRoutineLeaks left subsequent BenchmarkNodeJS runs failing with shutdown errors.

Approach & Changes

  • source/ports/go_port/CMakeLists.txt:
    • Add -Wl,-rpath,${PROJECT_OUTPUT_DIR} to GO_PORT_CGO_LDFLAGS so the Go test binary embeds the rpath to libmetacall.dylib.
    • Re-enable the goroutine_leaks profile generation on macOS.
  • source/loaders/node_loader/source/node_loader_impl.cpp:
    • Add predicate loop (while (!completed) { uv_cond_wait(&cond, &mutex); }) and completed = true flag in loader_impl_async_safe_type to eliminate spurious wakeups from OS signals (SIGURG).
    • Correctly initialize node_future->promise_ref = nullptr and loader references prior to future_create, and destroy f on allocation failure.
  • source/ports/go_port/source/go_port_test.go:
    • Isolate TestGoRoutineLeaks in a dedicated subprocess (GO_PORT_SUBPROCESS_LEAKS=1), preventing its Destroy() invocation from terminating the parent process's runtime.
    • Re-enable TestGoRoutineLeaks, BenchmarkNodeJSSequential, and BenchmarkNodeJSParallel on Darwin by removing the runtime.GOOS == "darwin" skips.

@TheDoomBoy-jab
TheDoomBoy-jab force-pushed the fix/go-port-macos-race-conditions branch from 0c017a5 to 1bc5392 Compare November 14, 2026 05:35

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant