Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,19 @@ jobs:
python3 scripts/gc_runtime_root_holders.py --self-test
python3 scripts/gc_runtime_root_holders.py

# #11471. The other half of the census above: a process-global cache
# KEYED by (or holding) a heap address is not a GC root, so that audit
# cannot see it, yet it goes stale the moment its thread exits and
# `Arena::drop` hands the blocks to another thread's arena. #11319,
# #11462 and #11470 were all this shape, each found by chasing a flake.
# Every such static must carry a thread-exit verdict; a new unclassified
# one fails, and so does a stale entry. Build-free, so it lives in `lint`.
- name: Thread-exit custody of address-keyed globals (#11471)
if: ${{ !cancelled() }}
run: |
python3 scripts/thread_exit_address_globals.py --self-test
python3 scripts/thread_exit_address_globals.py

# #8821. The poll-reach audit is static, build-free and sub-second, but it
# lived ONLY in `gc-root-dominance.yml`, which is label-gated behind
# `run-extended-tests` — so it is skipped on every PR and speaks only on
Expand Down
5 changes: 5 additions & 0 deletions changelog.d/11485-thread-exit-address-globals.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
- **runtime/stdlib: process-global tables forget an exiting thread's addresses (#11471).** A process-global table keyed by, or holding, a heap address outlived the thread whose arena owned it. After `Arena::drop` freed the blocks, another thread reused the addresses and the stale entry described a different object. #11319, #11462 and #11463/#11467 were three instances, each found by chasing a flake. An audit of every mutable process-global `static` in perry-runtime and perry-stdlib that can hold a 64-bit word (805 candidates, 234 of them counter-only atomics) found 83 more tables of the same shape. Examples: `SYMBOL_PROPERTIES`/`SYMBOL_POINTERS` (a new object at a reused address inherited a dead worker's symbol properties, or read as a Symbol), `OBJECT_PROTOTYPES`, `DOM_EXCEPTION_ERRORS` (a reused address passed `instanceof DOMException`), `CRYPTO_KEY_REGISTRY` and the external buffer registries, the Web Streams, zlib and worker registries, the stdin/readline/TLS/async_hooks/v8 listener tables, `GLOBAL_THIS_PTR`, and the child_process/pty/dgram/native-async reactors, which also kept the event loop alive for a dead thread.

New mechanism: `arena::thread_exit`. `Arena::drop` and, new, `MallocState::drop` (for `gc_malloc` blocks such as `Symbol()` headers) report the ranges they are about to free to `release_freed_ranges`. That calls one hook per perry-runtime table and every hook registered with `register_thread_exit_range_hook`, which is how perry-stdlib tables participate. `FreedRanges::holds_bits` decodes both NaN-boxed and raw addresses. `HANDLES` payload types register per-type releasers through `register_handle_payload_releaser`. A few statics that held a handle into a per-thread table were made per-thread instead, or are released at agent retirement: the diagnostics_channel console channel ids, `Buffer.poolSize`, and the turnloop signal subscription ids.

Gate: `scripts/thread_exit_address_globals.py` (in `lint`) enumerates the candidates and requires a thread-exit verdict for each one in `scripts/thread_exit_address_globals.json`. It fails on an unclassified static, a stale entry, or a `thread_exit_invalidated` verdict whose hook no longer runs at thread exit. `--self-test` plants each shape. Regression tests are in `crates/perry-stdlib/src/runtime_thread_exit_tests/`, one per fixed table where feasible. Each one populates the table from a spawned thread, asserts the entry exists while the thread lives, joins, and asserts it is gone. Each test was also run with its hook disabled and failed.
3 changes: 3 additions & 0 deletions crates/perry-runtime/src/arena/block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -537,6 +537,9 @@ impl Drop for Arena {
.map(|block| (block.data as usize, block.data as usize + block.size))
.collect();
crate::closure::release_closure_side_table_owners_in_ranges(&ranges);
// #11471: every other process-global table keyed by, or holding,
// an address in these blocks (see `arena::thread_exit`).
super::thread_exit::release_freed_ranges(&ranges);
}
for block in &self.blocks {
// Skip tombstoned slots (gen-GC Phase C4b-δ): C4b-δ
Expand Down
2 changes: 2 additions & 0 deletions crates/perry-runtime/src/arena/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ mod promote;
mod quarantine;
mod reset;
mod stats;
/// #11471: thread-exit release of process-global address-keyed tables.
pub mod thread_exit;
mod walk;
pub(crate) use map_allocations::walk_map_allocations;

Expand Down
181 changes: 181 additions & 0 deletions crates/perry-runtime/src/arena/thread_exit.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,181 @@
//! Thread-exit release of process-global address-keyed state (#11471).
//!
//! When a thread exits, [`Arena::drop`](super::Arena) hands its arena blocks
//! back to the allocator, and another thread's arena may reuse the same
//! addresses. A process-global table keyed by (or holding) an address inside
//! those blocks then describes a DIFFERENT object: #11319 (closure side
//! tables), #11462 (`tls.rs` roots) and #11470 (typed-array kind caches) each
//! shipped as a flake before anyone traced it here.
//!
//! This module is the one place those tables hear about a thread exit.
//! `Arena::drop` calls [`release_freed_ranges`] with the blocks it is about to
//! free; that runs every hook registered through
//! [`register_thread_exit_range_hook`]. A hook drops every entry whose key or
//! stored value lies in [`FreedRanges`].
//!
//! # Rules for a hook
//!
//! It runs inside a TLS destructor of the exiting thread:
//!
//! * no thread-local state (it may already be gone), no GC allocation, no
//! collection;
//! * process-global locks are fine (the tables it touches are exactly those),
//! but take them with `lock()`/`try_lock()` and tolerate poisoning;
//! * it must not call back into JS.
//!
//! `scripts/thread_exit_address_globals.py` requires every process-global
//! static that can hold an arena address to carry a verdict; a
//! `thread_exit_invalidated` verdict names its hook, and the gate checks the
//! hook is registered here or called by `Arena::drop`.

use std::sync::{Mutex, PoisonError};

/// The address ranges `[start, end)` of the arena blocks an exiting thread is
/// about to free, sorted by start. Arena blocks never overlap, so membership
/// is one binary search.
pub struct FreedRanges {
sorted: Vec<(usize, usize)>,
}

impl FreedRanges {
pub fn new(ranges: &[(usize, usize)]) -> Self {
let mut sorted: Vec<(usize, usize)> = ranges
.iter()
.copied()
.filter(|&(start, end)| start < end)
.collect();
sorted.sort_unstable_by_key(|&(start, _)| start);
Self { sorted }
}

pub fn is_empty(&self) -> bool {
self.sorted.is_empty()
}

/// Does `addr` lie inside one of the freed blocks?
#[inline]
pub fn contains(&self, addr: usize) -> bool {
if addr == 0 {
return false;
}
let after = self.sorted.partition_point(|&(start, _)| start <= addr);
after > 0 && addr < self.sorted[after - 1].1
}

/// Does this 64-bit word name freed memory, read either as a NaN-boxed
/// heap reference (pointer, string or bigint tag) or as a raw address
/// (closure pointers and `*mut Promise` are stored raw in several
/// tables)? A plain number never decodes to a block address unless it is
/// one, so erring toward "names it" can only drop an entry that pointed at
/// the dead thread's heap anyway.
#[inline]
pub fn holds_bits(&self, bits: u64) -> bool {
let tag = bits >> 48;
if matches!(tag, 0x7FFD | 0x7FFF | 0x7FFA) {
return self.contains((bits & 0x0000_FFFF_FFFF_FFFF) as usize);
}
tag == 0 && self.contains(bits as usize)
}

/// [`holds_bits`](Self::holds_bits) for an `f64`-typed JS value.
#[inline]
pub fn holds_value(&self, value: f64) -> bool {
self.holds_bits(value.to_bits())
}

/// [`holds_bits`](Self::holds_bits) for an `i64`-typed word.
#[inline]
pub fn holds_i64(&self, word: i64) -> bool {
self.holds_bits(word as u64)
}
}

/// Hooks for process-global tables, in this crate or downstream
/// (`perry-stdlib`). Function pointers only: no heap address lives here.
static RANGE_HOOKS: Mutex<Vec<fn(&FreedRanges)>> = Mutex::new(Vec::new());

/// Register `hook` to run whenever a thread's arena frees its blocks.
/// Idempotent per function. Call it before the table's first insert (a
/// `Once` on the insert path is the usual shape), so no entry predates it.
pub fn register_thread_exit_range_hook(hook: fn(&FreedRanges)) {
let mut hooks = RANGE_HOOKS.lock().unwrap_or_else(PoisonError::into_inner);
if !hooks
.iter()
.any(|existing| std::ptr::fn_addr_eq(*existing, hook))
{
hooks.push(hook);
}
}

/// Run every registered hook over `ranges`. Called by `Arena::drop` (arena
/// blocks) and `MallocState::drop` (`gc_malloc` blocks) at thread exit.
pub(crate) fn release_freed_ranges(ranges: &[(usize, usize)]) {
let freed = FreedRanges::new(ranges);
if freed.is_empty() {
return;
}
// -- perry-runtime tables (#11471): one call per owning module --
crate::geisterhand_registry::release_geisterhand_in_freed_ranges(&freed);
crate::ui_text_registry::release_ui_text_registry_in_freed_ranges(&freed);
crate::frame::release_frame_callbacks_in_freed_ranges(&freed);
crate::tui::input::release_tui_input_handler_in_freed_ranges(&freed);
crate::tui::state::release_tui_state_slots_in_freed_ranges(&freed);
crate::tui::hooks::release_tui_hook_slots_in_freed_ranges(&freed);
#[cfg(feature = "full")]
crate::plugin::release_plugin_registry_in_freed_ranges(&freed);
#[cfg(feature = "ohos-napi")]
crate::media_playback::release_media_callbacks_in_freed_ranges(&freed);
crate::symbol::release_symbol_tables_in_freed_ranges(&freed);
crate::symbol::release_symbol_accessors_in_freed_ranges(&freed);
crate::object::prototype_chain::release_object_prototypes_in_freed_ranges(&freed);
crate::event_target::release_dom_exception_errors_in_freed_ranges(&freed);
crate::node_vm::release_vm_owner_entries_in_freed_ranges(&freed);
crate::messaging::release_port_states_in_freed_ranges(&freed);
crate::v8::release_promise_hooks_in_freed_ranges(&freed);
crate::async_hooks::release_async_hooks_in_freed_ranges(&freed);
crate::object::release_global_this_ptr_in_freed_ranges(&freed);
crate::tls::release_tls_client_metadata_in_freed_ranges(&freed);
#[cfg(feature = "mod-dgram")]
crate::dgram_reactor::release_dgram_sockets_in_freed_ranges(&freed);
#[cfg(feature = "mod-dgram")]
crate::dgram::release_dgram_bindings_in_freed_ranges(&freed);
crate::child_process::reactor::release_cp_children_in_freed_ranges(&freed);
#[cfg(any(unix, windows))]
crate::pty::reactor::release_ptys_in_freed_ranges(&freed);
crate::promise::native_async::release_native_async_tokens_in_freed_ranges(&freed);
crate::typed_feedback::release_typed_feedback_in_freed_ranges(&freed);
// -- end perry-runtime tables --
// Copied out so a hook may itself take locks without holding this one.
let hooks: Vec<fn(&FreedRanges)> = RANGE_HOOKS
.lock()
.unwrap_or_else(PoisonError::into_inner)
.clone();
for hook in hooks {
hook(&freed);
}
}

#[cfg(test)]
mod tests {
use super::FreedRanges;

#[test]
fn membership_and_value_decoding() {
let freed = FreedRanges::new(&[(0x2000, 0x3000), (0x1000, 0x1800)]);
assert!(freed.contains(0x1000));
assert!(freed.contains(0x17ff));
assert!(!freed.contains(0x1800));
assert!(freed.contains(0x2fff));
assert!(!freed.contains(0x3000));
assert!(!freed.contains(0));
assert!(freed.holds_bits(0x7FFD_0000_0000_2010));
assert!(freed.holds_bits(0x7FFF_0000_0000_1010));
assert!(freed.holds_bits(0x2010));
assert!(!freed.holds_bits(0x7FFD_0000_0000_4000));
assert!(!freed.holds_value(1.5));
assert!(
!freed.holds_bits(0x7FFE_0000_0000_2010),
"an int32 is not an address"
);
}
}
9 changes: 9 additions & 0 deletions crates/perry-runtime/src/async_hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1969,6 +1969,15 @@ pub fn scan_async_hooks_roots_mut(visitor: &mut crate::gc::RuntimeRootVisitor<'_
}
}

// #11471: thread-exit release of this module's process-global tables.
mod thread_exit;
pub(crate) use thread_exit::release_async_hooks_in_freed_ranges;
#[doc(hidden)]
pub use thread_exit::{
cached_singletons_for_test, clear_cached_singletons_for_test, context_snapshot_holds_for_test,
hook_callback_registered_for_test, resource_tracked_for_test,
};

#[cfg(test)]
mod test_support;
#[cfg(test)]
Expand Down
131 changes: 131 additions & 0 deletions crates/perry-runtime/src/async_hooks/thread_exit.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
//! #11471: thread-exit release of `async_hooks`' process-global tables.

use std::sync::atomic::Ordering;

use super::{
HookCallbacks, ASYNC_WRAP_PROVIDERS, CONTEXT_SNAPSHOTS, HOOKS, HOOKS_ACTIVE,
PROMISE_HOOKS_ACTIVE, RESOURCES, TOP_LEVEL_RESOURCE,
};

/// #11471: release every async_hooks entry naming an exiting thread's arena.
///
/// All six tables are process-global (`per_test_global!` is a plain static
/// outside perry-runtime's own unit tests) while their writers run on any
/// thread, and none is otherwise told about a thread exit:
///
/// * `HOOKS` — a `createHook` record whose callback is freed is disabled
/// exactly as `set_hook_enabled(_, false)` does (keeping `HOOKS_ACTIVE` /
/// `PROMISE_HOOKS_ACTIVE` consistent) and its callbacks nulled. The record
/// stays: `AsyncHookHandle` names it by index.
/// * `RESOURCES` — an entry whose resource value or captured context store is
/// freed is dropped without a destroy event (the thread that could observe
/// it is gone).
/// * `CONTEXT_SNAPSHOTS` — a snapshot holding a freed store is dropped; only
/// the dead thread's `AsyncLocalStorage.snapshot()` closures named it.
/// * `TOP_LEVEL_RESOURCE` / `ASYNC_WRAP_PROVIDERS` — lazily allocated by the
/// first thread to ask; cleared if that thread's arena is going away, so the
/// next caller allocates a fresh one on its own heap.
pub(crate) fn release_async_hooks_in_freed_ranges(freed: &crate::arena::thread_exit::FreedRanges) {
let snapshot_is_dead = |snapshot: &crate::async_context::AsyncContextSnapshot| {
let mut dead = false;
crate::async_context::scan_snapshot_roots(snapshot, &mut |store| {
dead |= freed.holds_value(store);
});
dead
};
{
let mut hooks = HOOKS
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner);
for record in hooks.iter_mut() {
let c = record.callbacks;
let dead = [c.init, c.before, c.after, c.destroy, c.promise_resolve]
.iter()
.any(|callback| freed.contains(*callback as usize));
if !dead {
continue;
}
if record.enabled && c.has_any() {
HOOKS_ACTIVE.fetch_sub(1, Ordering::Relaxed);
if record.track_promises {
PROMISE_HOOKS_ACTIVE.fetch_sub(1, Ordering::Relaxed);
}
}
record.enabled = false;
record.callbacks = HookCallbacks::empty();
}
}
RESOURCES
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.retain(|_, meta| !freed.holds_value(meta.resource) && !snapshot_is_dead(&meta.context));
CONTEXT_SNAPSHOTS
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.retain(|_, snapshot| !snapshot_is_dead(snapshot));
for slot in [&ASYNC_WRAP_PROVIDERS, &TOP_LEVEL_RESOURCE] {
let bits = slot.load(Ordering::Acquire);
if bits != 0 && freed.holds_bits(bits) {
let _ = slot.compare_exchange(bits, 0, Ordering::AcqRel, Ordering::Acquire);
}
}
}

/// #11471 test probe: the cached `executionAsyncResource()` bootstrap object
/// and `asyncWrapProviders` bits (0 when unset).
#[doc(hidden)]
pub fn cached_singletons_for_test() -> (u64, u64) {
(
TOP_LEVEL_RESOURCE.load(Ordering::Acquire),
ASYNC_WRAP_PROVIDERS.load(Ordering::Acquire),
)
}

/// #11471 test probe: forget both lazily cached singletons, so the calling
/// thread allocates its own on next use.
#[doc(hidden)]
pub fn clear_cached_singletons_for_test() {
TOP_LEVEL_RESOURCE.store(0, Ordering::Release);
ASYNC_WRAP_PROVIDERS.store(0, Ordering::Release);
}

/// #11471 test probe: does any `createHook` record hold `callback`?
#[doc(hidden)]
pub fn hook_callback_registered_for_test(callback: usize) -> bool {
HOOKS
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.iter()
.any(|record| {
let c = record.callbacks;
[c.init, c.before, c.after, c.destroy, c.promise_resolve]
.iter()
.any(|p| *p as usize == callback)
})
}

/// #11471 test probe: is `async_id` tracked in `RESOURCES`?
#[doc(hidden)]
pub fn resource_tracked_for_test(async_id: u64) -> bool {
RESOURCES
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.contains_key(&async_id)
}

/// #11471 test probe: does any registered context snapshot hold a store with
/// these bits?
#[doc(hidden)]
pub fn context_snapshot_holds_for_test(store_bits: u64) -> bool {
CONTEXT_SNAPSHOTS
.lock()
.unwrap_or_else(std::sync::PoisonError::into_inner)
.values()
.any(|snapshot| {
let mut found = false;
crate::async_context::scan_snapshot_roots(snapshot, &mut |store| {
found |= store.to_bits() == store_bits;
});
found
})
}
Loading
Loading