Skip to content

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

@proggeramlug

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_ID and tui::state::SLOTS both look like this: their assertions want per-test identity.
  • one lock domain (plugin::REGISTRY, and the thread-ownership latch in opt_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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions