ReviewOS

zig-utils/zig-js

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

#293
Closed chris opened this 10 days ago ·
10 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.
10 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.

10 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.

10 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.