fix(runtime): module-env binding creation vs worker resolve data race (#607) - #608
Merged
Conversation
…#607) Module-level code (main thread) creating a NEW global while spawned workers run could realloc the module env's names/values/assign_counts arrays or rebuild its hash buckets — freeing blocks a concurrent worker env_resolve_chain walk or post-resolve slot load was still reading. A real use-after-free class, latent since spawn landed, hidden by bucket-collision luck; #602's +3 builtin registrations shifted the module-env layout so the grow landed inside test_concurrent's MT window and the TSan gate flagged it deterministically (2/2 CI runs, reproduced locally with the full stacks). The guard, all gated on g_vm_multithreaded so single-threaded programs pay only predicted-false branches: - g_module_env_lock serializes module-env (parent == NULL) structural mutation (env_set_local_hashed / env_set_local_pre_interned_slot) against the module-env hop of every chain walker (env_resolve_chain, env_set_hashed[_slot], env_get_hashed[_slot], env_get_local_hashed, env_get_assign_count). - Grown-out arrays are RETIRED on the env (env_grow_retire / env_hash_rebuild_retire; freed at park/destroy) so a post-resolve values load OUTSIDE the lock can never touch freed memory. Waste is bounded < 2x the final array size by geometric doubling. - values/assign_counts pointers are published with release stores and read at the post-resolve VM/JIT sites via env_values_ptr / env_assign_counts_ptr (acquire), so the pointer word itself is synchronized. IC fast paths stay plain: a worker never fast-path-hits the module env (its ICs are never populated under MT per #297), and main is the grower itself. - env_store_slot takes the lock so a worker's global reassign cannot land in a retired copy (a lost update). Out of scope (pre-existing, separate class): two threads racing on the SAME slot's value or assign-count — slot-value semantics, not array memory safety. Red-then-green: with the #602 builtins present, test_concurrent reported 4 TSan warnings without this fix and 0 across 3 runs with it; the full tests/test_tsan.sh slice passes 10/10 including the seeded-race self-validation. Release suite 2880/2880; ASan+UBSan detect_leaks=1 leak tally 0; jit-smoke and freestanding-check green. Closes #607 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #607
The bug
Main-thread module code creating a new global while spawned workers run could
reallocthe module env'snames/values/assign_countsarrays or rebuild its hash buckets — freeing blocks a concurrent workerenv_resolve_chainwalk or post-resolve slot load was still reading. Use-after-free class, latent since spawn landed, hidden by bucket-collision luck; #602's +3 builtin registrations shifted the module-env layout so a grow landed inside test_concurrent's MT window and the TSan gate went deterministically red (2/2 CI runs on PR #606; reproduced locally with full stacks — readerenv_resolve_chaineigenscript.c:1882 /vm_run_exvm.c:2837 in a worker vs writerenv_set_local_pre_interned_sloteigenscript.c:1788-89 on main).The fix (all gated on
g_vm_multithreaded— single-threaded programs pay only predicted-false branches)g_module_env_lockserializes module-env (parent == NULL) structural mutation against the module-env hop of every chain walker.values/assign_countspointers are published with release stores and read at the post-resolve VM/JIT sites viaenv_values_ptr/env_assign_counts_ptr(acquire). IC fast paths stay plain — a worker never fast-path-hits the module env (ICs never populate under MT per Data races: parallel workers executing the same shared chunk (exec_count, JIT counters, inline caches, shared-env refcounts) #297) and main is the grower itself.env_store_slottakes the lock so a worker's global reassign can't land in a retired copy (lost update).Out of scope (pre-existing, separate class): two threads racing on the SAME slot's value/assign-count — slot-value semantics, not array memory safety.
Validation
test_concurrentreported 4 TSan warnings without the fix, 0 across 3 runs with it; fulltests/test_tsan.shslice 10/10 including the seeded-race self-validation.detect_leaks=1leak tally 0 (single [97] failure = the knowninvariant_weak60 s wall-guard marginality under ASan on the N3350, 71 s in isolation, unrelated).make jit-smokeandmake freestanding-checkgreen.Unblocks #606 (PCM codec kernels) and the #603 resample PR.
🤖 Generated with Claude Code