Fix MT use-after-free: module-env observer-slot growth vs unlocked reads (#694)

[InauguralPhysicist]

observer_slot_update_e grew env->obs with a bare realloc + non-atomic pointer store while holding no lock, and the report of / when is / predicate opcodes read a chain-resolved env's obs array outside any lock. A worker reading a module binding's observer slot concurrently with the main thread first-observing a new module binding therefore walked a freed array.

This is precisely the use-after-free class #607 fixed for names/values/assign_counts — obs was simply never brought into that scheme. It survived because module obs grows only on a binding's first observation, and only on the main thread, so the window is narrow enough that it never bit in practice.

The fix

obs now follows the same #607 discipline:

	before	after
grow (shared module env)	bare realloc, no lock	copy into a fresh block under g_module_env_lock
old block	freed immediately	retired on the env, freed at park/destroy
publish	plain store	pointer release-store, then cap release-store
read	idx < e->obs_cap && e->obs[idx], unlocked	env_obs_slot() — cap acquire, then pointer acquire
single-threaded / frame envs	plain realloc	unchanged, behind one predicted-false branch

The ordering is load-bearing, and is commented as such at both ends. Reading pointer-before-cap would admit new-cap-with-old-block — an out-of-bounds read of the retired array — which would have made the "fix" a different bug. The writer therefore publishes pointer-then-cap (release) and the reader loads cap-then-pointer (acquire), so a reader either sees the old cap (valid against both blocks, since the new one is a superset copy and the old one is retired) or the new cap (which by the release/acquire chain guarantees the new block is already visible).

Nothing off the MT path pays: single-threaded runs and thread-local frame/loop envs keep the plain realloc and plain loads behind one predicted-false g_vm_multithreaded branch, the same gating #607 uses.

Scope

Same-slot value races — two threads writing one binding's slot — stay out of scope, exactly as #607 declared them ("slot-value semantics, not memory safety of the arrays"). Retirement keeps both blocks alive, so this is not a safety hole; I did not silently widen the claim. This PR is about array memory safety only.

All 17 obs access sites are converted. Worth flagging: an initial survey found only 9, because the grep pattern excluded lines containing obs_cap. The remaining raw accesses are the writer itself and teardown.

Validation

New regression tests/test_obs_mt_race.eigs — three workers spinning report of a module binding while the main thread first-observes 600 new module bindings — added to test_tsan.sh's race-free slice:

gate	result
test_obs_mt_race.eigs without the fix	5 TSan data races, rc 66 — at observer_slot_update_e and vm_run_ex, exactly as the issue predicted
test_obs_mt_race.eigs with the fix	0 races, rc 0
tests/test_tsan.sh	11 passed, 0 failed — and the seeded race is still detected, so the gate has not rotted into a vacuous pass
release suite	3183/3183
ASan + detect_leaks=1	3187/3187, leak tally 0, section [87] leak-clean

The red-then-green pair is the point: the test is only worth having because it fails without the fix.

Closes #694

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR fixes a multithreaded use-after-free in the EigenScript runtime where the module environment’s env->obs observer-slot array could be realloc’d (freeing the old block) while worker threads concurrently read from the old array during report of / when is / predicate operations. The fix aligns obs growth with the existing #607 “retire-and-publish” scheme used for module-env structural arrays under MT.

Changes:

• Introduces env_obs_slot() to safely read obs_cap + obs with acquire ordering, and migrates VM/JIT report/predicate paths to use it.
• Updates observer_slot_update_e to grow env->obs via a new observer_obs_grow() that retires old blocks and publishes pointer/cap with release ordering under g_module_env_lock for shared module envs.
• Adds a new TSAN regression (test_obs_mt_race.eigs) and includes it in the “race-free slice” in tests/test_tsan.sh.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tests/test_tsan.sh	Adds the new TSAN regression to the race-free concurrency slice.
tests/test_obs_mt_race.eigs	New regression program that reproduces the module-env obs grow vs read race under TSAN.
src/vm.c	Converts obs slot reads to go through env_obs_slot() for MT safety.
src/eigenscript.h	Adds env_obs_slot() accessor with cap/pointer acquire ordering.
src/eigenscript.c	Implements MT-safe obs growth with retire-and-publish semantics; routes observer updates through the accessor.
CHANGELOG.md	Documents the fix and the new TSAN regression.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.