fix(ipc): bind a VM for lifecycle hooks on VM-less threads

[singaraiona]

Closes #569. Follow-up to #565, which fixed the no hook installed case and deliberately left this one open.

What was wrong

__VM is bound only by ray_runtime_create, so any thread an embedder drives IPC from has none. #565 let ray_env_get fall through to globals on such a thread — which means that with a hook actually bound, hook_lookup finds the lambda and call_fn1 runs user code with __VM == NULL.

The mutation check reproduces it exactly:

src/lang/eval.c:124:16: runtime error: member access within null pointer
of type 'struct ray_vm_t'

Why bind rather than skip or refuse

Both alternatives were worse:

• Skipping the hook is silent. The same close behaves differently depending on which thread drove it, with nothing in the log to say why, and nothing visible at the call site.
• Refusing the close contradicts the stance already written down at runtime.c:50-54, where a thread-local error shadow was added precisely so FFI callers on VM-less threads keep working.

A hook is user code; it should behave the same wherever the event came from.

A second bug fixed by the same change

ipc_ctx_set stores through __VM and returns early without one. So on a VM-less thread the hook — when it ran at all — saw .ipc.handle as -1. Binding restores it, and the test asserts on that specifically rather than only on "the hook fired".

Cost

ray_vm_t is ~68 KB, too large for a teardown-path stack, so the temporary comes from the buddy heap — process-wide and already live, since the heap belongs to the runtime rather than the thread.

Threads that already have a VM pay one predicted branch and allocate nothing; the allocation happens only on threads that would previously have crashed. Teardown mirrors ray_runtime_destroy — release raise_val and trace, then free — including in not walking grown scope frames, which that teardown does not either.

Tests

ipc/close_hook_vmless_thread stands up a real server via the shared harness, connects, then drives ray_poll_deregister from a thread that never called ray_runtime_create — the shape of an embedder's own teardown, which is how this was found.

It asserts both that the hook ran and that it observed a real .ipc.handle. Mutation-verified against the pre-fix behaviour, which produces the UBSan error quoted above rather than a clean failure.

make test: 3899 of 3899 passed (0 skipped, 0 failed). make fuzz-smoke clean.

[github-actions]

Rayforce targeted audit passed

The required Rayforce audit gate passed on the latest run.

Workflow run: https://github.com/RayforceDB/rayforce/actions/runs/35382931194

[singaraiona]

Both blockers were correct, and the first one corrects a claim I made in the PR description. Fixed in c96beb21.

1. The test did not gate the bug

Your reading is exactly right: .ipc.on.close is one process-global binding, ipc_on_close fires it for both ends, and the server thread has a VM — so _vmless_fired >= 1 and _vmless_h >= 0 were both satisfied by the server-side firing alone.

And my "mutation-verified" claim was weaker than I stated. The mutant was caught, but by UBSan aborting on the NULL dereference at eval.c:124 (fn_is_restricted reading __VM->restricted), not by the assertions. In a build without UBSan, or whenever the server processed EOF first, the test would have passed with the fix reverted. You were right to check rather than take the claim at face value.

Fixed by stopping the server before installing the hook. With no hook bound during shutdown there is no server-side firing, so the single firing afterwards is unambiguously the client-side teardown driven from the VM-less thread. The assertions are now exact rather than existential — fired == 1, and the handle equals this connection's h rather than merely being non-negative.

I did not use "assert _vmless_h == h" alone, as you suggested, because the two ends are selector ids in different polls and could coincide numerically; removing the server-side firing entirely is the discriminator that cannot go wrong.

Re-verified with a mutant that does not crash, so only the assertions can catch it — hook_vm_bind refusing to bind, i.e. the "skip silently" alternative:

test/test_ipc.c:731: fired->i64 != 1  (got 0, expected 1)

2. The g_env race

Also correct, and the same change resolves it: the server thread is joined and its poll destroyed before anything reads those globals. Your observation that fixing the race alone would have worsened finding 1 — by guaranteeing the server-side increment — is what made it clear the two had to be fixed together rather than in sequence.

Non-blocking — all three taken

Silent OOM skip. Now logs before skipping, matching how this file reports every other hook failure.

Unbound hook paths. hook_call_auth and the sync/async dispatch each carry a note saying they are unbound because they run only under ray_poll_run, so a future embedder-driven path knows it needs the same treatment rather than rediscovering this class of bug.

Alignment. Fixed.

Thanks in particular for the "verified sound" section — the re-entrancy analysis and the point that ray_execute unwinds to scope_base on every path are both things I had reasoned about but not written down, and it is useful to have them confirmed independently.

make test: 3900 of 3900 passed. make fuzz-smoke clean.