ReviewOS

zig-utils/zig-js

Worker inspector: make detach publication and queued client ownership race-free

#293
Closed chris opened this 9 days ago ·
9 days ago

CI run 29647981277 exposed a Linux unit-shard UAF in finishInspectorClient while the module-worker inspector target was closing.

Root cause:

  • runtime_detached is published before the final event-queue pushes/close, so the owner can wake and destroy InspectorClient.events while the worker still uses it.
  • queued inspector commands retain raw InspectorClient pointers without ownership, so commands already behind a terminal detach/failure can outlive the owner reference.

Acceptance:

  • Publish runtime detach only after every worker-side event access is complete, with a final wake.
  • Give every queued command explicit client ownership and release it on every drain/error path.
  • Keep owner-thread queue destruction after runtime detach without leaks or deadlock.
  • Re-run the exact failing module-worker inspector test repeatedly and under TSan.
  • Confirm the normal/TSan CI shards are green.
9 days ago

Implemented and pushed as ddfa1c66.

The worker now publishes terminal detach while holding the event-queue mutex, after its last event write, and every queued command retains/releases the client explicitly on success, allocation failure, drain, and teardown paths.

Focused evidence with the exact CI compiler (0.17.0-dev.1417+20befa4e6):

  • formerly crashing module-worker inspector test: 12/12 normal repetitions green;
  • same test: 4/4 TSan repetitions green;
  • every repetition: 3 passed, 0 skipped, 0 failed, 0 leaked; no TSan report.

Keeping the issue open only for the replacement normal/TSan CI shards.

9 days ago

CI-confirmation status: the fix (ddfa1c66) remains on main, but the final checkbox is still unconfirmed. Every CI run since has either been cancelled by newer pushes or failed in unrelated in-flight areas — the last completed run (b2d3f248, 29652658235) failed all unit/tsan-unit shards and three tsan-nogil-corpus shards on the current src/wasm/exec.zig test-switch compile error and the gc.test 128/256-byte slab layout assertion, both from the active GC/WASM work, not from this inspector fix. Leaving open until a full normal/TSan run goes green.

9 days ago

Replacement run 29697559754 passed all four normal and all four TSan unit shards, plus focused engine and lifecycle stress jobs. Inspector detach publication and queued-client ownership are green in CI.

Sign in to comment on this issue.