Repository navigation
Three more process-global sinks with split lock domains, outside the GC guards' clear list (async_hooks, tui::state, agent_dispatch timer queues) #7680
Description
Follow-on to #7672. That issue's survey covered the tables gc::tests::support::reset_copying_nursery_runtime_test_state() clears, and #7674 converts all 23 of them to per-thread storage in test builds plus a lint gate over that clear list.
A wider sweep of crates/ found three more process-global sinks with the same signature — a split lock domain — that the guards' clear list does not reach, so #7674's gate is structurally blind to them. All three already have an assertion of the shape that made the first three flakes diagnosable (a wrong VALUE, not a timing symptom), which is what makes them worth naming now rather than after they bite.
1. async_hooks — four disjoint serialization domains over three statics
crates/perry-runtime/src/async_hooks.rs:90 static HOOKS: LazyLock<Mutex<Vec<HookRecord>>>
crates/perry-runtime/src/async_hooks.rs:91 static RESOURCES: LazyLock<Mutex<HashMap<u64, ResourceMeta>>>
crates/perry-runtime/src/async_hooks.rs:31 static NEXT_ASYNC_ID: AtomicU64 = AtomicU64::new(2)
Four domains touch them:
| domain | where |
|---|---|
a private TEST_LOCK |
async_hooks.rs:1431 (tests at 1436, 1448) |
AsyncHookRuntimeTestGuard's private ASYNC_HOOK_RUNTIME_TEST_LOCK |
gc/tests/runtime_roots.rs:508 |
the global side-table lock, via CopyingNurseryTestGuard |
gc/tests/alloc.rs:817 |
| none at all | gc/tests/alloc.rs:836 test_async_hooks_promise_alloc_remains_malloc_tracked |
The exposed assertion is resource_ids_are_monotonic_even_without_hooks: b.async_id == a.async_id + 1. Any concurrent id allocation on another thread makes that +2, and the failure reads as "async ids are not monotonic" rather than "a neighbour allocated one".
AsyncHookRuntimeTestGuard additionally calls test_clear_object_cache_roots() under its private lock, which gives the object caches a second clearing domain on top of the guards' — #7674 makes the caches per-thread, so that particular overlap is now harmless, but the split remains.
2. tui::state::SLOTS — cleared under three different locks
crates/perry-runtime/src/tui/state.rs:35 static SLOTS: Mutex<Vec<u64>> = Mutex::new(Vec::new())
crates/perry-runtime/src/tui/state.rs:29 pub static STATE_DIRTY: AtomicBool
tui/state.rs:145's reset() takes a module-private TEST_LOCK; gc/tests/roots.rs (10 sites) takes lock_safe_runtime_scanner_test_guard(); gc/tests/cycle_state.rs:620,671 and callback_scanners.rs:1257,1489 take the global lock via CopyingNurseryTestGuard.
The exposed assertion is tui/state.rs:153 alloc_returns_sequential_handles: h0 == 0, h1 == 1, h2 == 2. A concurrent SLOTS.clear() or a concurrent alloc breaks it outright, and it fails as "handles are not sequential".
3. agent_dispatch_tests.rs — a private lock over tables the guards also clear
crates/perry-runtime/src/agent_dispatch_tests.rs:20
static TIMER_QUEUE_TESTS: std::sync::Mutex<()> = std::sync::Mutex::new(());
Three tests (lines 68, 124, 169) populate the timer queues and assert on active_timeout_resource_count() / scan_timer_roots, serialized only among themselves; line 162 calls test_clear_all_timer_scanner_roots() without the global lock, so it damages GC tests as well as being damaged by them. This is the opt_report / ext_registry shape (#7665) exactly. #7674 makes the timer queues per-thread, which removes the data hazard, but the lock-domain split — and the unguarded clear — remain.
Also noted, lower severity: event_pump.rs:741 uses a private SERIAL lock and works around the race with a 40-iteration retry loop, and array/tests.rs:235,1659 carry 256-iteration retry loops with comments naming the object-cache race directly. Retry loops around a shared-state race are the symptom being described here, written down by a previous author.
Suggested shape
Two in-tree precedents, and the choice is per table:
- per-thread storage (
guard_cleared_global!, fix(test-isolation): the GC guards' clear can no longer reach another test's data (#7672) #7674) — right where the state is genuinely per-agent and the clear exists only to give a test a clean slate.NEXT_ASYNC_IDandtui::state::SLOTSboth look like this: their assertions want per-test identity. - one lock domain (
plugin::REGISTRY, and the thread-ownership latch inopt_report/ext_registry) — right where cross-thread visibility is load-bearing.
Whichever is chosen, the thing worth keeping from #7674 is that the check lands on the table author, who is a finite population, and never on the reader, who is not.
Activity
fix(test-isolation): close the async_hooks / tui::state / timer-queue…
fix(test-isolation): close the async_hooks / tui::state / timer-queue…