diff --git a/.github/workflows/pyre-ci.yml b/.github/workflows/pyre-ci.yml index d6b07d9f6ba..d3e0be5a906 100644 --- a/.github/workflows/pyre-ci.yml +++ b/.github/workflows/pyre-ci.yml @@ -573,9 +573,12 @@ jobs: name: CPython suite (gate) runs-on: ubuntu-24.04 # Artifacts are named for this Linux runner, so wait for the Linux prepare - # leg; waiting for macOS races the Linux upload. Separately, the baseline is - # darwin-arm64-specific (`CPYTHON_SUITE_BASELINE_HOST`); `PLATFORM_GATED` - # only handles modules CPython skips wholesale on this host. + # leg; waiting for macOS races the Linux upload. This job is the only place + # CI runs the suite — `pyre/check.py` keeps the stage behind + # `--cpython-suite`, which no job passes, so its wall time is paid once. + # Separately, the baseline is darwin-arm64-specific + # (`CPYTHON_SUITE_BASELINE_HOST`); `PLATFORM_GATED` only handles modules + # CPython skips wholesale on this host. needs: prepare-charon-llbc-linux if: ${{ !cancelled() && needs.prepare-charon-llbc-linux.result == 'success' }} timeout-minutes: 30 diff --git a/pyre/bench/synth/load_name_builtin_cell_fold.cranelift.jitstats b/pyre/bench/synth/load_name_builtin_cell_fold.cranelift.jitstats new file mode 100644 index 00000000000..c3bab7dbb4a --- /dev/null +++ b/pyre/bench/synth/load_name_builtin_cell_fold.cranelift.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=2 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/bench/synth/load_name_builtin_cell_fold.dynasm.jitstats b/pyre/bench/synth/load_name_builtin_cell_fold.dynasm.jitstats new file mode 100644 index 00000000000..c3bab7dbb4a --- /dev/null +++ b/pyre/bench/synth/load_name_builtin_cell_fold.dynasm.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=2 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/bench/synth/load_name_builtin_cell_fold.py b/pyre/bench/synth/load_name_builtin_cell_fold.py new file mode 100644 index 00000000000..19d5a684b61 --- /dev/null +++ b/pyre/bench/synth/load_name_builtin_cell_fold.py @@ -0,0 +1,21 @@ +# pyre-check: max-pypy-ratio=8 +# A module-scope LOAD_NAME whose name misses the module dict resolves through +# the frame's builtin module. The builtins cell folds under the module dict's +# version? (so a later global binding shadows the builtin) and the builtins +# dict's own version?. The second loop proves that invalidation is seen. +# Output verified against CPython and PyPy. + +N = 90000000 +M = 400000 +s = "xx" + +total = 0 +for i in range(N): + total = total + len(s) +print(total) + +len = lambda x: 100 +shadowed = 0 +for i in range(M): + shadowed = shadowed + len(s) +print(shadowed) diff --git a/pyre/bench/synth/load_name_builtin_cell_fold.wasm.jitstats b/pyre/bench/synth/load_name_builtin_cell_fold.wasm.jitstats new file mode 100644 index 00000000000..c3bab7dbb4a --- /dev/null +++ b/pyre/bench/synth/load_name_builtin_cell_fold.wasm.jitstats @@ -0,0 +1,15 @@ +bridges_compiled=0 +descr_set_absent=0 +descr_set_ambiguous=0 +descr_set_stale_absent=0 +fbw_blackhole_adopted_multi_frame=0 +fbw_blackhole_adopted_single_frame=0 +fbw_rolled_back_with_effects=0 +fbw_store_journal_rollback_failed=0 +field_pos_attached_misplaced=0 +field_pos_spec_misplaced=0 +guard_failures=2 +internal_compile_panics=0 +loops_aborted=0 +loops_compiled=2 +retraces_compiled=0 diff --git a/pyre/bench/synth/str_fstring.cranelift.jitstats b/pyre/bench/synth/str_fstring.cranelift.jitstats index e45d14ff4ca..5c093f4f38b 100644 --- a/pyre/bench/synth/str_fstring.cranelift.jitstats +++ b/pyre/bench/synth/str_fstring.cranelift.jitstats @@ -8,7 +8,7 @@ fbw_rolled_back_with_effects=0 fbw_store_journal_rollback_failed=0 field_pos_attached_misplaced=0 field_pos_spec_misplaced=0 -guard_failures=659 +guard_failures=658 internal_compile_panics=0 loops_aborted=0 loops_compiled=6 diff --git a/pyre/check.py b/pyre/check.py index 3b80e16b0f7..6474875b9fa 100644 --- a/pyre/check.py +++ b/pyre/check.py @@ -2432,9 +2432,14 @@ def run_cpython_suite(self): The baseline records one verdict per module per backend, observed on darwin-arm64, and dynasm's codegen is arch-specific -- so the - comparison only means anything there, which is also why the CI job - pins `runs-on: macos-latest`. On any other host the stage reports - that it did not run instead of counting as a pass. + comparison only means anything there. On any other host the stage + reports that it did not run instead of counting as a pass. + + Off by default and reached only through `--cpython-suite`: the suite + costs more wall time than every other stage here put together, and the + `cpython-tests` CI job already runs it on its own schedule. Pass the + flag locally when a change could move a verdict the synthetic corpus + does not cover. """ name = "cpython-suite" backend = "dynasm" @@ -2824,9 +2829,10 @@ def parse_backend_specs(specs): help="skip pyre/bench/synth feature-parity benchmarks", ) parser.add_argument( - "--no-cpython-suite", + "--cpython-suite", action="store_true", - help="skip the vendored CPython suite gate (pyre/cpython_tests)", + help="also run the vendored CPython suite gate (pyre/cpython_tests); " + "off by default because it dominates this script's wall time", ) parser.add_argument( "--synthetic-only", @@ -3011,7 +3017,7 @@ def main(): print() chk.run_synthetic_suite() - if not args.no_cpython_suite and not args.synthetic_only: + if args.cpython_suite and not args.synthetic_only: print() print(bold("vendored CPython suite")) chk.run_cpython_suite() diff --git a/pyre/cpython_tests/baseline.json b/pyre/cpython_tests/baseline.json index 3f9f8763877..7123313d750 100644 --- a/pyre/cpython_tests/baseline.json +++ b/pyre/cpython_tests/baseline.json @@ -1069,7 +1069,7 @@ "dynasm": "PASS" }, "test.test_struct": { - "dynasm": "FAIL" + "dynasm": "PASS" }, "test.test_structseq": { "dynasm": "PASS" @@ -1145,7 +1145,7 @@ "dynasm": "PASS" }, "test.test_threading": { - "dynasm": "FAIL" + "dynasm": "PASS" }, "test.test_threading_local": { "dynasm": "PASS" diff --git a/pyre/pyre-interpreter/src/importing.rs b/pyre/pyre-interpreter/src/importing.rs index 4916bb69861..c32dedf4ab2 100644 --- a/pyre/pyre-interpreter/src/importing.rs +++ b/pyre/pyre-interpreter/src/importing.rs @@ -1697,32 +1697,34 @@ pub fn remove_sys_module(name: &str) { } } -/// Drop the Python-visible import cache during process shutdown. -/// -/// CPython's `finalize_modules()` clears `sys.modules` before module-global -/// teardown; PyPy's object space likewise owns modules through -/// `space.sys.modules`, rather than a permanent side table. Keeping every -/// imported module in the cache until `process::exit` prevents its managed -/// module/dict cycles and their `__del__` objects from ever becoming -/// unreachable. Retain `sys` and `builtins` themselves because pyre's -/// unraisable-error path still consults their live dictionaries while the -/// released modules are finalized. -pub fn release_sys_modules_for_shutdown() -> Vec { +/// `finalize_remove_modules`: snapshot real modules, then detach import-cache +/// entries so module/dict cycles can become unreachable during shutdown. +pub fn release_sys_modules_for_shutdown() -> Vec<(Wtf8Buf, PyObjectRef)> { let dict = sys_modules_dict(); if dict.is_null() { return Vec::new(); } - let entries = unsafe { pyre_object::w_dict_str_entries(dict) }; + let entries = unsafe { pyre_object::w_dict_items(dict) }; let mut modules = Vec::new(); - for (name, module) in entries { - if matches!(name.as_str(), "sys" | "builtins") { - continue; - } + for (key, module) in entries { + let name = if unsafe { pyre_object::is_str(key) } { + Some(unsafe { pyre_object::w_str_get_wtf8(key) }.to_owned()) + } else { + None + }; if !module.is_null() && unsafe { pyre_object::is_module(module) } { - modules.push(module); + if let Some(name) = &name { + modules.push((name.clone(), module)); + } } - unsafe { - pyre_object::w_dict_delitem_str(dict, &name); + let keep_entry = name.as_deref().is_some_and(|name| { + let bytes = name.as_bytes(); + bytes == b"sys" || bytes == b"builtins" + }); + if !keep_entry { + unsafe { + pyre_object::w_dict_delitem(dict, key); + } } } modules diff --git a/pyre/pyre-interpreter/src/module/_blake2/_blake2_app.py b/pyre/pyre-interpreter/src/module/_blake2/_blake2_app.py index 8c424bdf08a..11ca6130a45 100644 --- a/pyre/pyre-interpreter/src/module/_blake2/_blake2_app.py +++ b/pyre/pyre-interpreter/src/module/_blake2/_blake2_app.py @@ -99,12 +99,6 @@ def __new__(cls, *args, **kwargs): "digest_size must be between 1 and %d bytes" % cls.MAX_DIGEST_SIZE ) - if len(key) > cls.MAX_KEY_SIZE: - raise ValueError("maximum key length is %d bytes" % cls.MAX_KEY_SIZE) - if len(salt) > cls.SALT_SIZE: - raise ValueError("maximum salt length is %d bytes" % cls.SALT_SIZE) - if len(person) > cls.PERSON_SIZE: - raise ValueError("maximum person length is %d bytes" % cls.PERSON_SIZE) if not 0 <= fanout <= 255: raise ValueError("fanout must be between 0 and 255") if not 1 <= depth <= 255: diff --git a/pyre/pyre-interpreter/src/module/_hashlib/mod.rs b/pyre/pyre-interpreter/src/module/_hashlib/mod.rs index bd6eca3ef02..9c8c3c5c5c2 100644 --- a/pyre/pyre-interpreter/src/module/_hashlib/mod.rs +++ b/pyre/pyre-interpreter/src/module/_hashlib/mod.rs @@ -473,7 +473,10 @@ pub unsafe fn w_hmac_dealloc(obj: PyObjectRef) { mod hmac_class { use super::*; - #[crate::pyre_methods] + // lib_pypy/_hashlib/__init__.py:232: `class HMAC(HASH)`. HMAC keeps its + // own native payload layout, but its Python type relationship is the same + // TypeDef inheritance PyPy exposes. + #[crate::pyre_methods(base = hash_state_class::type_object())] impl W_Hmac { #[staticmethod] fn __new__( @@ -566,13 +569,23 @@ mod hmac_class { fn resolve_hmac_digestmod(digestmod: PyObjectRef) -> Result<&'static str, crate::PyError> { let name_obj = if unsafe { is_str(digestmod) } { digestmod - } else if unsafe { - pyre_object::py_type_check(digestmod, &crate::function::BUILTIN_FUNCTION_TYPE) - } { - crate::baseobjspace::getattr_str(digestmod, "__name__")? } else { - return Err(unsupported_digestmod("unsupported hash type")); + match crate::baseobjspace::getattr_str(digestmod, "__name__") { + Ok(name) => name, + // PyPy's structural rule is to accept every object that exposes + // `__name__` (lib_pypy/_hashlib/__init__.py:547-554). CPython + // 3.14 additionally normalizes a missing name to the module's + // public UnsupportedDigestmodError; preserve both without + // restricting the accepted object type again. + Err(err) if err.kind == crate::PyErrorKind::AttributeError => { + return Err(unsupported_digestmod("unsupported hash type")); + } + Err(err) => return Err(err), + } }; + if !unsafe { is_str(name_obj) } { + return Err(unsupported_digestmod("unsupported hash type")); + } let name = unsafe { w_str_get_wtf8(name_obj) }; let bytes = name.as_bytes(); let bytes = bytes.strip_prefix(b"openssl_").unwrap_or(bytes); @@ -671,11 +684,8 @@ fn pbkdf2_hmac(args: &[PyObjectRef]) -> Result { check_digest_name(hash_name)?; let requested = unsafe { w_str_get_wtf8(hash_name) }; - let name = lookup_digest_name(requested.as_bytes()).ok_or_else(|| { - crate::PyError::value_error(format!( - "[digital envelope routines] unsupported: {requested}" - )) - })?; + let name = lookup_digest_name(requested.as_bytes()) + .ok_or_else(|| unsupported_digestmod("unsupported hash type"))?; let password = read_hash_buffer(password)?; let salt = read_hash_buffer(salt)?; let iterations = crate::baseobjspace::int_w(crate::baseobjspace::space_index(iterations)?)?; @@ -694,7 +704,7 @@ fn pbkdf2_hmac(args: &[PyObjectRef]) -> Result { _ => pyre_native::hash::digest_output_size(name).unwrap_or(0), }; let result = pyre_native::hash::compute_pbkdf2_hmac(name, &password, &salt, iterations, dklen) - .ok_or_else(|| crate::PyError::value_error("unsupported hash type"))?; + .ok_or_else(|| unsupported_digestmod("unsupported hash type"))?; Ok(w_bytes_from_bytes(&result)) } @@ -788,8 +798,11 @@ fn scrypt_kdf(args: &[PyObjectRef]) -> Result { } // RFC 7914's dominant allocation is V[N] with 128*r-byte entries. OpenSSL - // also needs B[p] and a working block. Honor an explicit caller limit; - // maxmem=0 retains OpenSSL's implementation-defined default behavior. + // also needs B[p] and a working block. PyPy passes maxmem=0 through to + // EVP_PBE_scrypt (lib_pypy/_hashlib/__init__.py:430-433), where OpenSSL + // applies its private 32 MiB default. The Rust backend has no such layer, + // so spell out that same default here rather than treating zero as + // unlimited memory. let memory = usize::try_from(n) .ok() .and_then(|n| n.checked_mul(r as usize)) @@ -799,9 +812,15 @@ fn scrypt_kdf(args: &[PyObjectRef]) -> Result { .ok_or_else(|| { crate::PyError::value_error("Invalid parameter combination for n, r, p, maxmem") })?; - if maxmem != 0 && memory > maxmem { + const OPENSSL_DEFAULT_SCRYPT_MAXMEM: usize = 32 * 1024 * 1024; + let effective_maxmem = if maxmem == 0 { + OPENSSL_DEFAULT_SCRYPT_MAXMEM + } else { + maxmem + }; + if memory > effective_maxmem { return Err(crate::PyError::value_error( - "Invalid parameter combination for n, r, p, maxmem", + "[digital envelope routines] memory limit exceeded", )); } let output = pyre_native::hash::compute_scrypt(&password, &salt, log_n, r, p, dklen) @@ -834,6 +853,26 @@ fn blake2_new(args: &[PyObjectRef]) -> Result { let key = read_hash_buffer(arg(3))?; let salt = read_hash_buffer(arg(4))?; let person = read_hash_buffer(arg(5))?; + let (max_key_size, salt_size, person_size) = match name { + "blake2b" => (64, 16, 16), + "blake2s" => (32, 8, 8), + _ => unreachable!(), + }; + if key.len() > max_key_size { + return Err(crate::PyError::value_error(format!( + "maximum key length is {max_key_size} bytes" + ))); + } + if salt.len() > salt_size { + return Err(crate::PyError::value_error(format!( + "maximum salt length is {salt_size} bytes" + ))); + } + if person.len() > person_size { + return Err(crate::PyError::value_error(format!( + "maximum person length is {person_size} bytes" + ))); + } let index = |position| crate::baseobjspace::int_w(crate::baseobjspace::space_index(arg(position))?); let digest_size = usize::try_from(index(2)?) diff --git a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs index ddbcbc3bfd1..9004ad911e2 100644 --- a/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs +++ b/pyre/pyre-jit-trace/src/jitcode_dispatch/specialize.rs @@ -13126,14 +13126,6 @@ pub(crate) fn try_walker_load_global_cell_fold( if emit_module_dict_cell_fold(ctx, op_pc, dst, dst_bank, w_globals, &name)? { return Ok(true); } - // `emit_module_dict_cell_fold` returns `false` for BOTH an absent name and - // a present-but-unfoldable one (`IntMutableCell` / strategy switched). - // Only an ABSENT name may fall through to the builtins fold — a - // present global shadows the builtin, so keep the residual (which reads the - // live globals slot) when the slot still exists. - if crate::state::module_dict_cell_slot_direct(w_globals, &name).is_some() { - return Ok(false); - } // Builtins fallback: the name is absent from the // `ns_ptr` module dict. Mirror `bh_load_global_fn`'s second leg — @@ -13176,6 +13168,43 @@ pub(crate) fn try_walker_load_global_cell_fold( unsafe { pyre_object::w_dict_getitem_str(w_globals, "__builtins__") } .unwrap_or(pyre_object::PY_NULL) }; + emit_builtins_cell_fold(ctx, op_pc, dst, dst_bank, w_globals, w_builtin, &name) +} + +/// Builtins-fallback half of the LOAD_GLOBAL and module-scope LOAD_NAME cell +/// folds: the name resolves through the frame's builtin module rather than the +/// module dict. Mirrors `_load_global`'s second leg, +/// `get_builtin().getdictvalue(varname)`, and is reached only once +/// [`emit_module_dict_cell_fold`] has declined. +/// +/// Two guards carry it. (a) The name must stay ABSENT from the module dict, +/// which pinning that dict's `version?` is what proves: the insert that would +/// shadow the builtin runs `mutated()` and fails GUARD_NOT_INVALIDATED. +/// (b) The builtins value itself folds through [`emit_namespace_cell_fold`], +/// whose `QUASIIMMUT_FIELD` on the builtins dict fails the loop on a rebind or +/// delete there. +/// +/// Returns `Ok(false)` — the caller then keeps the live residual — for a name +/// still present in the module dict, a missing or non-module builtin, an +/// unfoldable builtins slot (absent / null / `IntMutableCell` / movable), or a +/// movable builtins dict. +fn emit_builtins_cell_fold( + ctx: &mut WalkContext<'_, '_, Sym>, + op_pc: usize, + dst: usize, + dst_bank: char, + w_globals: pyre_object::PyObjectRef, + w_builtin: pyre_object::PyObjectRef, + name: &str, +) -> Result { + // `emit_module_dict_cell_fold` returns `false` for BOTH an absent name and + // a present-but-unfoldable one (`IntMutableCell` / strategy switched). + // Only an ABSENT name may fall through to the builtins fold — a + // present global shadows the builtin, so keep the residual (which reads the + // live globals slot) when the slot still exists. + if crate::state::module_dict_cell_slot_direct(w_globals, name).is_some() { + return Ok(false); + } if w_builtin.is_null() || !unsafe { pyre_object::is_module(w_builtin) } { return Ok(false); } @@ -13183,7 +13212,7 @@ pub(crate) fn try_walker_load_global_cell_fold( if w_builtin_dict.is_null() { return Ok(false); } - let Some(b_slot) = crate::state::module_dict_cell_slot_direct(w_builtin_dict, &name) else { + let Some(b_slot) = crate::state::module_dict_cell_slot_direct(w_builtin_dict, name) else { return Ok(false); }; let Some(b_stored) = crate::state::module_dict_cell_value_direct(w_builtin_dict, b_slot) else { @@ -13238,6 +13267,13 @@ pub(crate) fn try_walker_load_global_cell_fold( /// A non-module frame (class body / `exec(code, g, l)` with separate locals) /// has a non-null `w_locals`, so the gate routes it to the live /// residual `bh_load_name_fn`. +/// +/// Builtins fallback: when `name` is absent from the module dict, module-scope +/// `LOAD_NAME` falls through via `load_global_value` to +/// `frame.get_builtin().getdictvalue(name)`. The builtins cell fold pins the +/// module dict `version?` so a later global insertion that shadows the builtin +/// fails GUARD_NOT_INVALIDATED, then folds the builtins dict cell like the +/// LOAD_GLOBAL fallback. pub(crate) fn try_walker_load_name_cell_fold( ctx: &mut WalkContext<'_, '_, Sym>, op_pc: usize, @@ -13268,7 +13304,18 @@ pub(crate) fn try_walker_load_name_cell_fold( let name = unsafe { pyre_object::unicodeobject::w_str_get_value(w_name_ptr as pyre_object::PyObjectRef) }; - emit_module_dict_cell_fold(ctx, op_pc, dst, dst_bank, w_globals, name) + if emit_module_dict_cell_fold(ctx, op_pc, dst, dst_bank, w_globals, name)? { + return Ok(true); + } + emit_builtins_cell_fold( + ctx, + op_pc, + dst, + dst_bank, + w_globals, + frame.get_builtin(), + name, + ) } /// StoreName/StoreGlobal cell fold — module-scope store dual of diff --git a/pyre/pyrex/src/lib.rs b/pyre/pyrex/src/lib.rs index b306ec91132..0025c32f1fc 100644 --- a/pyre/pyrex/src/lib.rs +++ b/pyre/pyrex/src/lib.rs @@ -1144,13 +1144,173 @@ fn run_atexit_callbacks(canonical: pyre_object::PyObjectRef, ec_ptr: *const PyEx } } -/// PyPy `ObjSpace.finish()` (`baseobjspace.py:481-504`): join non-daemon -/// threads, run atexit callbacks, mark the runtime finalizing, flush standard -/// streams, and run started modules' shutdown hooks. PyPy does not force a full -/// collection or clear `__main__` globals here; doing either makes shutdown -/// traverse (and temporarily expand) an arbitrarily large heap and gives -/// module-global `__del__` methods CPython behavior that the PyPy oracle does -/// not have. +fn collect_and_run_finalizers(ec_ptr: *const PyExecutionContext) { + pyre_object::gc_hook::try_gc_collect(); + if !ec_ptr.is_null() { + unsafe { (&mut *(ec_ptr as *mut PyExecutionContext))._run_finalizers_now() }; + } +} + +/// Whether releasing this binding can remove anything from the reachable set, +/// and so whether the collection that follows it could find garbage an earlier +/// one did not. Answering `false` skips a full mark-and-sweep of the whole +/// heap, which is what the release loop below otherwise costs per name. +/// +/// Two values answer `false` outright: +/// +/// * An exact `int`, `float`, `bool`, `str`, `bytes` or `None`. It holds no +/// reference to another object, so nothing but the value itself can lose its +/// last referrer, and no builtin scalar type defines `__del__`, so nothing +/// observes it going. `is_exact_type` is what makes this safe rather than +/// `is_str`/`is_int`, which key off the layout `ob_type` a subclass keeps: a +/// `class MyStr(str)` instance retags `w_class` and is rejected here, so its +/// `__del__` and its own attributes stay on the collecting path. +/// * A module still registered in `sys.modules` under its own `__name__`. It +/// stays reachable from there no matter what `__main__` does, so the release +/// removes no object at all from the reachable set. This is every `import` +/// name. Reading the live `sys.modules` rather than assuming is what keeps a +/// program that replaced or deleted the entry on the collecting path. +/// +/// The point is not that these values are cheap to collect — it is that the +/// collection cannot reach a different answer, so every `__del__` still runs at +/// exactly the same place in the loop. +fn release_frees_nothing(value: pyre_object::PyObjectRef) -> bool { + if value.is_null() { + return true; + } + unsafe { + if pyre_object::is_none(value) + || pyre_object::is_exact_type(value, &pyre_object::INT_TYPE) + || pyre_object::is_exact_type(value, &pyre_object::BOOL_TYPE) + || pyre_object::is_exact_type(value, &pyre_object::FLOAT_TYPE) + || pyre_object::is_exact_type(value, &pyre_object::STR_TYPE) + || pyre_object::is_exact_type(value, &pyre_object::BYTES_TYPE) + { + return true; + } + if pyre_object::is_module(value) { + // `Module.name` is the interpreter's own field, not the + // program-writable `__name__` attribute, so it is always a string — + // but `types.ModuleType.__new__` leaves it empty until `__init__` + // seeds it. An anonymous module proves nothing about reachability, + // so it takes the collecting path rather than a `sys.modules[""]` + // lookup that only an adversarial program could satisfy. + let name = pyre_object::w_module_get_name(value); + return !name.is_empty() && importing::get_sys_module(name).is_some_and(|m| m == value); + } + } + false +} + +fn shutdown_module_private_name(name: &rustpython_wtf8::Wtf8) -> bool { + let bytes = name.as_bytes(); + bytes.first() == Some(&b'_') && bytes.get(1) != Some(&b'_') +} + +fn clear_shutdown_module_name(dict: pyre_object::PyObjectRef, name: &rustpython_wtf8::Wtf8) { + let should_clear = unsafe { pyre_object::w_dict_getitem_wtf8(dict, name) } + .is_some_and(|value| unsafe { !pyre_object::is_none(value) }); + if should_clear { + unsafe { + pyre_object::w_dict_setitem_wtf8(dict, name, pyre_object::w_none()); + } + } +} + +/// `_PyModule_ClearDict`: clear string-keyed module globals in two name passes. +fn clear_shutdown_module_dict(dict: pyre_object::PyObjectRef) { + if dict.is_null() { + return; + } + let keys: Vec = unsafe { pyre_object::w_dict_str_entries_wtf8(dict) } + .into_iter() + .map(|(name, _)| name) + .collect(); + for name in &keys { + if shutdown_module_private_name(name) { + clear_shutdown_module_name(dict, name); + } + } + for name in &keys { + if name.as_bytes() != b"__builtins__" { + clear_shutdown_module_name(dict, name); + } + } +} + +/// `finalize_modules`: clear detached modules newest-first while their peers +/// remain available to finalizers that run between module dictionaries. +fn clear_shutdown_modules( + modules: Vec<(rustpython_wtf8::Wtf8Buf, pyre_object::PyObjectRef)>, + ec_ptr: *const PyExecutionContext, +) { + let _roots = pyre_object::gc_roots::push_roots(); + let roots_start = pyre_object::gc_roots::shadow_stack_len(); + let mut names = Vec::with_capacity(modules.len()); + let mut sys_module_slot = None; + let mut builtins_module_slot = None; + for (name, module) in modules { + let index = names.len(); + let bytes = name.as_bytes(); + if bytes == b"sys" { + sys_module_slot = Some(index); + } else if bytes == b"builtins" { + builtins_module_slot = Some(index); + } + names.push(name); + pyre_object::gc_roots::pin_root(module); + } + collect_and_run_finalizers(ec_ptr); + for index in (0..names.len()).rev() { + let module = pyre_object::gc_roots::shadow_stack_get(roots_start + index); + let is_core_module = sys_module_slot.is_some_and(|slot| { + module == pyre_object::gc_roots::shadow_stack_get(roots_start + slot) + }) || builtins_module_slot.is_some_and(|slot| { + module == pyre_object::gc_roots::shadow_stack_get(roots_start + slot) + }); + if is_core_module { + continue; + } + if module.is_null() || !unsafe { pyre_object::is_module(module) } { + continue; + } + let dict = unsafe { pyre_object::w_module_get_w_dict(module) }; + clear_shutdown_module_dict(dict); + } + // One collection for the whole walk, not one per module. `finalize_modules` + // clears the module dictionaries and lets refcounting release what they + // held; a sweep per module buys no ordering here, because a finalizer that + // reads a global reaches its own already-cleared namespace either way, and + // it costs a full mark-and-sweep for each of the ~100 modules a bare + // `import unittest` loads. + collect_and_run_finalizers(ec_ptr); +} + +/// PyPy `ObjSpace.finish()` / module teardown ordering: join non-daemon +/// threads, collect already-unreachable cycles, then release `__main__` +/// globals from newest to oldest while the older globals their `__del__` +/// methods may reference are still present. +/// +/// Newest-to-oldest is what keeps those references working, and it is not +/// interchangeable with the insertion order `_PyModule_ClearDict` uses. A name +/// is bound before every name that could be finalized while reading it — most +/// of all `import sys`, which is usually the very first — so releasing in +/// insertion order strands `sys` at `None` and every finalizer that writes to +/// `sys.stderr` dies with an `AttributeError` instead of running. Refcounting +/// makes the question moot upstream: a finalizer there runs from the decref of +/// the name being released, while every other slot still holds its original +/// value, which is not reproducible without leaving dangling pointers in the +/// dict. +/// +/// The value is rebound to `None` rather than deleted, as `_PyModule_ClearDict` +/// does: a `__del__` that reads an already-released name then sees `None`, the +/// way it would upstream, instead of raising `NameError` at a name the program +/// can see is still defined. +/// +/// `__main__` is not the whole reachable set: an object stored in another +/// module's namespace stays alive through that module's dict. The +/// `finalize_modules` phase that follows detaches `sys.modules` and clears the +/// remaining module dictionaries newest-first. fn finalize_runtime(canonical: pyre_object::PyObjectRef, ec_ptr: *const PyExecutionContext) { run_threading_shutdown(); run_atexit_callbacks(canonical, ec_ptr); @@ -1160,14 +1320,48 @@ fn finalize_runtime(canonical: pyre_object::PyObjectRef, ec_ptr: *const PyExecut pyre_interpreter::module::thread::set_finalizing(); // baseobjspace.py:498-501 `finish()` runs every started module's shutdown // hook; `_io`'s (moduledef.py:37-40) flushes the streams that are still - // alive. + // alive. The per-global teardown below reaches only the ones `__main__` + // itself holds, so without this a stream owned by any other module loses + // its buffered writes. pyre_interpreter::module::_io::flush_all_streams(); + collect_and_run_finalizers(ec_ptr); + let mut entries = unsafe { pyre_object::w_dict_str_entries(canonical) }; + entries.reverse(); + for (name, _) in entries { + if name == "__builtins__" { + continue; + } + // Re-read rather than trusting the snapshot: a `__del__` already run by + // this loop may have rebound the name, and the decision below is only + // sound about the value actually being released. + let value = unsafe { pyre_object::w_dict_getitem_str(canonical, &name) }; + let frees_nothing = value.is_none_or(release_frees_nothing); + unsafe { + pyre_object::w_dict_setitem_str(canonical, &name, pyre_object::w_none()); + } + if !frees_nothing { + collect_and_run_finalizers(ec_ptr); + } + } + // The loop ends on a collection whenever it releases anything collectable; + // this is the one for the case where the last releases were all skipped, so + // teardown still finishes with the heap swept and the queue drained. + collect_and_run_finalizers(ec_ptr); + let shutdown_modules = pyre_interpreter::importing::release_sys_modules_for_shutdown(); + clear_shutdown_modules(shutdown_modules, ec_ptr); } /// Resolve a pending `SystemExit`'s status, then finalize and exit with it. /// -/// `app_main.py:114-129 handle_sys_exit` resolves `e.code` at application -/// level, before `targetpypystandalone.py:88 finally: space.finish()`. +/// The order matters: `app_main.py:114-129 handle_sys_exit` runs at +/// application level, i.e. *before* `targetpypystandalone.py:88 +/// finally: space.finish()`. Resolving `e.code` is an ordinary attribute +/// lookup that walks the exception's type and that type's dict, and +/// `finalize_runtime` collects — pinning the three raw `PyObjectRef` fields +/// of the Rust `PyError` (which, unlike PyPy's GC-visible `OperationError`, +/// the collector cannot see) does not keep that type's dict alive. Reading +/// the code after the collection spun forever inside the type dict's +/// `IndexMap` probe for a user-defined `SystemExit` subclass. fn finalize_system_exit( error: pyre_interpreter::PyError, canonical: pyre_object::PyObjectRef, @@ -1443,8 +1637,9 @@ fn run_source_with_context( } let is_keyboard_interrupt = is_keyboard_interrupt(&e); // targetpypystandalone.py:88 `finally: space.finish()` — finalize - // on every exit path, not only on SystemExit. Print the exception - // before shutdown, matching app_main.py's ordering. + // on every exit path, not only on SystemExit. Print first: the raw + // `PyObjectRef` fields of `e` are not GC-visible and + // `finalize_runtime` collects. pyre_interpreter::eprint_exception(&e, true); finalize_runtime(canonical, ec_ptr); maybe_print_jit_stats();