Repository navigation
[thread] Cross-heap unsoundness in perry/thread: global queue drains, timer queues, module globals — design decision needed #6185
Description
Problem
perry/thread's cross-thread machinery has several unsound edges (all code-verified in the 2026-07-09 GC audit, threading dimension):
- Await loop drains global queues on whatever thread runs it (
perry-codegen/src/expr/fs_await.rs:194-207→promise/microtasks.rs:110-122→thread.rs:1283-1307): a worker'sawaitcan steal another thread's completion, deserialize it into the worker's arena, and resolve a main-heap promise with a foreign pointer; the settle task then runs main-heap closures on the worker. Worker exit unmaps the arena → main thread reads freed memory. - Global timer queues carry raw closure/promise pointers across heaps (
timer.rs:25-40, 323, 367-380): any thread's await loop fires foreign-heap timer callbacks (the "only accessed from the pump thread" SAFETY comment is not enforced). - Module-level globals are process-wide but rooted/rewritten only by the thread that ran module init (
gc/roots.rs:63,425-434): worker closures calling top-level functions alias main-heap objects with no synchronization and no rewrite when a main-thread moving cycle relocates them. - Serializer silently converts Map/Set/Promise/Error/TypedArray/ArrayBuffer captures to
undefined(thread.rs:349-374) — contradicts the documented "views deep-copy" claim; should fail loudly. - Worker death leaks every malloc-tracked object — no
Drop for MallocState(gc/malloc.rs:112-128), so per-request spawn workers accrete promises/maps/closures forever.
Proposed fix (decision needed)
Either enforce the single-JS-thread discipline mechanically — owner-ThreadId tags on every queued completion/timer with drains skipping foreign entries, compiler rejection (or serialize-snapshot) of module-global access in thread closures, loud serializer errors, Drop for MallocState — or commit to real multi-heap isolation. The current halfway point is the least safe option.
Quick wins already shipping separately (Wave-1 PR3): ensure_gc_initialized() at worker entry, rooting the rebuilt worker closure, a scanner/pinned-gate for PENDING_THREAD_RESULTS, and the ws.rs deferred-resolution fix.
Context
2026-07-09 GC audit. Items 1-3 are P0-severity UAF classes when perry/thread is combined with await/timers/module state.
Activity
proggeramlug commentedon Jul 9, 2026
Two mechanical safety wins DONE (#6212): (1) cross-heap transfer of an unsupported type (Map/Set/Promise/Error/TypedArray/Buffer/Symbol/Temporal/native-handle/lazy-array) now throws a catchable TypeError naming the type instead of silently becoming
undefined; (2)Drop for MallocStatefrees a worker thread's malloc objects at exit (was: leaked — first malloc-count GC needs 100k objects, so per-request workers never collected). #3 (owner-tagged timer dispatch) DEFERRED with a real blocker: Android'snativePumpTickfires timers on the UI thread whilesetTimeout/setIntervalschedule on the perry-native thread — a blanket owner-skip would break ALL Android timer delivery. That's the 'timers legitimately fire on a different thread than created' case; it needs the full per-thread-queue rework (Wave 4), not a narrow slice. The remaining cross-heap P0s (await-loop global-queue draining, timer queues firing foreign-heap closures, module globals aliased into workers) are the threading-model DECISION this issue tracks.fix(thread): loud cross-heap transfer errors + worker MallocState cle…
proggeramlug commentedon Jul 10, 2026
This is a genuine design decision; here's a concrete recommendation with the current state.
Already fixed (landed): the serializer silently downgrading unsupported types to
undefinedand the workerMallocStatenever-freed leak — closed by #6188/#6212 (Drop for MallocState,SerializedValue::Unsupported+guard_transferable).Still open, all three sharing one trigger: the codegen
awaitloop (fs_await.rs) is emitted into every function containingawait, unconditional on which OS thread runs it. Nothing stops aspawn/parallelMap/parallelFilterclosure body from containingawaitor a nestedspawn— the only compile-time guard walks the body for outer writes, notawait/spawn/outer reads. A worker doing async work then pumps the process-globalPENDING_THREAD_RESULTS/TIMER_QUEUE/CALLBACK_TIMERS(allunsafe impl Sendwith "only touched on the pump thread" comments that aren't enforced) and can read module-global heap pointers (perry_global_*) from another arena with zero sync — UAF / cross-heap aliasing (details:microtasks.rs:129unconditionaljs_thread_process_pending;timer.rs:40/323;gc/roots.rsGLOBAL_ROOTSisthread_local!so workers get an empty root set).Recommended design — enforce single-JS-thread discipline mechanically, in two tiers:
perry/threadclosure-body walker (lower_call/native/mod.rs:130-190) to also reject, as a compile error, anExpr::Await, a nestedspawn/parallelMap/parallelFilter, or an outer read of a non-primitive module-scope binding inside the worker closure. Reuses the infrastructure already there; doesn't touch the AndroidnativePumpTicktimer path (which legitimately fires timers cross-thread, and is why a blanket runtime "skip foreign entries" guard was deferred). Ship with negative-compile test-parity cases.PendingThreadResult/Timer/CallbackTimerwith a pump-thread-vs-worker designation and have drains skip entries they don't own, special-casing the Android UI-thread pump explicitly.Tier 1 is a real risk reduction but not a soundness proof (an AST walk can't see through a named async callee). Tier 2 is what closes the gap. I'd file Tier 2 as a new "Wave 4 cross-heap" tracker and land Tier 1 as a scoped PR — but flagging that Tier 1 is a behavior change (previously-compiling worker closures now error), so it needs a call on whether to hard-error or warn first.
proggeramlug commentedon Jul 11, 2026
Tier 1 (compile-time containment) is up as PR #6276: the worker-closure walk now rejects async closures /
await(viais_asyncORModule::async_step_closures— the CPS rewrite clears the flag, so the registry is threaded intoFnCtx), nestedspawn/parallelMap/parallelFilter, and reads of heap-typed module-scope bindings (primitives allowed;SharedArrayBufferexempt as the documented escape hatch). Hard-error rather than warn — every rejected pattern is a live cross-heap UAF class, and each diagnostic names the sanctioned rewrite. One in-repo casualty wasfilehandle-thread-detached-returned.ts(module-scope FileHandle read from a worker), rewritten to the captured form with byte-identical output. Tier 2 (owner-tagged runtime dispatch, AndroidnativePumpTickspecial-case) still needs its own Wave-4 tracker.review: seed compile_static_method local_types from module_global_types
fix(thread): compile-time containment for perry/thread worker closures (
chore: v0.5.1258 — changelog + version bump for #6276 (#6185 Tier-1 p…
proggeramlug commentedon Jul 11, 2026
Tier 1 landed — PR #6276 merged as `0427d6868` (v0.5.1258). Worker closures passed to `spawn`/`parallelMap`/`parallelFilter` are now compile-rejected when they (1) are/contain an async closure or `await`, (2) nest a thread primitive, or (3) read a heap-typed module-scope binding. Primitive globals stay allowed; `SharedArrayBuffer` is exempt as the documented escape hatch.
Keeping this issue open for the remaining work, which is the Tier-2 decision this tracker was really about: owner-tagged runtime dispatch — tag every `PendingThreadResult`/`Timer`/`CallbackTimer` with its owning thread and have drains skip foreign entries, with the Android `nativePumpTick` UI-thread pump explicitly special-cased (the blocker that deferred the narrow slice earlier). Tier 1 is a real risk reduction but not a soundness proof — an AST walk can't see through a named callee defined outside the worker body, so items 1–3 remain reachable via indirection until Tier 2 closes them at runtime.
proggeramlug commentedon Jul 11, 2026
Tier 2 is up as PR #6294 — owner-tagged runtime dispatch, the decision this issue was tracking. With it, every bullet of the "Proposed fix" is done and this issue can close:
PENDING_THREAD_RESULTS, owner-filtered drainTIMER_QUEUE/CALLBACK_TIMERS/INTERVAL_TIMERS, owner-filtered ticksDrop for MallocState)The Android blocker is resolved, not special-cased. The reason the narrow slice was deferred was that a
ThreadId-keyed owner-skip would kill Android timer delivery (nativePumpTickfires on the UI thread;setTimeoutis called onperry-native). #6294 tags by agent — the heap that owns the queued pointers — not by thread. A thread with no agent of its own is by definition a pump acting for the primary agent, so the UI thread resolves to exactly the agent whose timersperry-nativescheduled, and Android delivery is unchanged. A unit test pins that invariant so a future refactor back toThreadIdcan't silently break it.It also fixes a related unsoundness the tag made visible:
scan_timer_roots_mutwalked every timer on every GC cycle, so a worker's collector marked through foreign arenas and, on an evacuating cycle, rewrote foreign promise/closure slots to forwarding addresses in the wrong arena. It now scans only the collecting agent's timers.Once #6294 merges I'll close this.
lint: split timer GC-root scanning into timer/gc_roots.rs; owner-filt…
fix(thread): owner-tagged cross-thread dispatch (#6185 Tier 2)
fix(thread): owner-tagged cross-thread dispatch (#6185 Tier 2) (#6294)
fix(runtime): cross boxed async-fn-local captures into perry/thread w…
fix(test-isolation): close the async_hooks / tui::state / timer-queue…
fix(test-isolation): close the async_hooks / tui::state / timer-queue…
fix(stdlib): worker_threads Workers must claim an agent id
turnloop P9: make the perry/thread claim checkable instead of quoted
feat(runtime): TextEncoder / TextDecoder instances are ordinary objec…
fix(runtime): a SharedArrayBuffer carries a real GcHeader (#10925)
fix(runtime): a SharedArrayBuffer carries a real GcHeader (#10925)
fix(runtime): a SharedArrayBuffer carries a real GcHeader (#10925)
fix(runtime): a SharedArrayBuffer carries a real GcHeader (#10925)