Follow-up to #565, which is the NULL guard that makes the no hook installed case safe. This is the question that guard deliberately left open, raised by @vbmithr in the PR.
The gap
ray_ipc_close → ray_poll_deregister → ipc_on_close → hook_call_lifecycle(IPC_HOOK_CLOSE) → hook_lookup → ray_env_get.
With #565 applied, a VM-less thread walking that path skips the local scopes and falls through to the global env. If no hook is bound, hook_lookup returns NULL, hook_call_lifecycle's if (!fn) return; fires, and teardown is the no-op it was always meant to be. That is the case #565 fixes.
But globals do resolve. So if a .ipc.on.close hook is bound and the close happens on a thread that never called ray_runtime_create, hook_lookup now finds the lambda and call_fn1 runs it with __VM == NULL.
Why this needs a decision rather than another guard
Whatever the evaluator does with a null __VM under a hook body is not something to discover by crashing. The honest options:
- Don't dispatch. Lifecycle hooks only run on threads with a VM; a VM-less close silently skips the hook. Cheapest, but a user-visible hook silently not firing is its own trap.
- Bind a VM for the duration. Dispatch acquires a VM (or a scratch one) around the hook call. Correct, but raises lifetime and cost questions on a teardown path.
- Refuse at the boundary. Document
ray_ipc_close as owning-thread-only and fail loudly off it.
runtime.c:50-54 already states the project's stance for the error path — FFI callers do reach these paths on threads that never bound a VM — which argues against (3) as a general answer.
Not urgent
Reaching this needs both a bound .ipc.on.close hook and a close driven from a VM-less thread. #565 covers the common case. Filing so the decision is recorded rather than found later.
Follow-up to #565, which is the NULL guard that makes the no hook installed case safe. This is the question that guard deliberately left open, raised by @vbmithr in the PR.
The gap
ray_ipc_close→ray_poll_deregister→ipc_on_close→hook_call_lifecycle(IPC_HOOK_CLOSE)→hook_lookup→ray_env_get.With #565 applied, a VM-less thread walking that path skips the local scopes and falls through to the global env. If no hook is bound,
hook_lookupreturns NULL,hook_call_lifecycle'sif (!fn) return;fires, and teardown is the no-op it was always meant to be. That is the case #565 fixes.But globals do resolve. So if a
.ipc.on.closehook is bound and the close happens on a thread that never calledray_runtime_create,hook_lookupnow finds the lambda andcall_fn1runs it with__VM == NULL.Why this needs a decision rather than another guard
Whatever the evaluator does with a null
__VMunder a hook body is not something to discover by crashing. The honest options:ray_ipc_closeas owning-thread-only and fail loudly off it.runtime.c:50-54already states the project's stance for the error path — FFI callers do reach these paths on threads that never bound a VM — which argues against (3) as a general answer.Not urgent
Reaching this needs both a bound
.ipc.on.closehook and a close driven from a VM-less thread. #565 covers the common case. Filing so the decision is recorded rather than found later.