From 5c341d263e9b14263fa2b77046c2d46a2bd44f2c Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 13 Aug 2026 22:27:12 +0900 Subject: [PATCH 01/20] vm: add per-interpreter runtime state and interpreter registry - vm/runtime.rs: process-global interpreter registry (monotonic ids, weak entries), InterpreterWhence/InterpreterInfo, process main id recording via main_interpreter_id(), a threading-gated owner map (store_owned_interpreter/ take_owned_interpreter/is_owned_interpreter/owned_interpreter_count), and the SUPPORTS_ISOLATED_INTERPRETERS constant. - PyGlobalState gains interpreter_id/whence/is_main and is_main_interpreter(); PyConfig/Settings derive Clone so a subinterpreter can clone parent config. - Interpreter: id()/whence()/is_main()/is_process_main(), create_subinterpreter() and create_owned_subinterpreter(); unregister on Drop. - thread.rs: per-interpreter thread slots (INTERP_THREAD_SLOTS), slot swap when switching interpreters on one OS thread, cleanup keyed by interpreter id. - Install signal handlers and init the main-thread ident only on the main interpreter; _thread._is_main_interpreter reflects the current interpreter. - sys.implementation.supports_isolated_interpreters reads the constant. - Guard the registry static for non-threading builds where rc::Weak is !Send. Assisted-by: Claude Code:claude-opus-4-8 --- crates/vm/src/lib.rs | 6 +- crates/vm/src/stdlib/_signal.rs | 4 +- crates/vm/src/stdlib/_thread.rs | 5 +- crates/vm/src/stdlib/sys.rs | 3 +- crates/vm/src/vm/interpreter.rs | 749 +++++++++++++++++++++++++++++--- crates/vm/src/vm/mod.rs | 39 +- crates/vm/src/vm/runtime.rs | 241 ++++++++++ crates/vm/src/vm/setting.rs | 2 + crates/vm/src/vm/thread.rs | 157 +++++-- 9 files changed, 1090 insertions(+), 116 deletions(-) create mode 100644 crates/vm/src/vm/runtime.rs diff --git a/crates/vm/src/lib.rs b/crates/vm/src/lib.rs index df67c979739..a15e30c34a3 100644 --- a/crates/vm/src/lib.rs +++ b/crates/vm/src/lib.rs @@ -109,7 +109,11 @@ pub use self::object::{ AsObject, Py, PyAtomicRef, PyExact, PyObject, PyObjectRef, PyPayload, PyRef, PyRefExact, PyResult, PyStackRef, PyWeakRef, }; -pub use self::vm::{Context, Interpreter, InterpreterBuilder, Settings, VirtualMachine}; +pub use self::vm::runtime; +pub use self::vm::{ + Context, Interpreter, InterpreterBuilder, InterpreterInfo, InterpreterWhence, + MAIN_INTERPRETER_ID, Settings, VirtualMachine, +}; pub use rustpython_common as common; pub use rustpython_compiler_core::{bytecode, frozen}; diff --git a/crates/vm/src/stdlib/_signal.rs b/crates/vm/src/stdlib/_signal.rs index 5879f877676..5abfd327553 100644 --- a/crates/vm/src/stdlib/_signal.rs +++ b/crates/vm/src/stdlib/_signal.rs @@ -177,7 +177,9 @@ pub(crate) mod _signal { module: &Py, vm: &VirtualMachine, ) { - if vm.state.config.settings.install_signal_handlers { + // Process-global signal disposition is owned by the main interpreter only. + // Subinterpreters (PEP 734) must not reinstall SIGINT / probe handlers. + if vm.state.is_main_interpreter() && vm.state.config.settings.install_signal_handlers { let sig_dfl = vm.new_pyobj(SIG_DFL as u8); let sig_ign = vm.new_pyobj(SIG_IGN as u8); diff --git a/crates/vm/src/stdlib/_thread.rs b/crates/vm/src/stdlib/_thread.rs index 377c68dca74..fe753cd78b7 100644 --- a/crates/vm/src/stdlib/_thread.rs +++ b/crates/vm/src/stdlib/_thread.rs @@ -796,9 +796,8 @@ pub(crate) mod _thread { } #[pyfunction] - fn _is_main_interpreter() -> bool { - // RustPython only has one interpreter - true + fn _is_main_interpreter(vm: &VirtualMachine) -> bool { + vm.state.is_main_interpreter() } /// Initialize the main thread ident. Should be called once at interpreter startup. diff --git a/crates/vm/src/stdlib/sys.rs b/crates/vm/src/stdlib/sys.rs index 66257806e22..5ee36b450d4 100644 --- a/crates/vm/src/stdlib/sys.rs +++ b/crates/vm/src/stdlib/sys.rs @@ -669,7 +669,8 @@ pub mod sys { "_multiarch" => ctx.new_str(multiarch()), "version" => PyVersionInfo::from_data(VersionInfoData::IMPLEMENTATION, vm), "hexversion" => ctx.new_int(version::VERSION_HEX_IMPL), - "supports_isolated_interpreters" => ctx.new_bool(false), + "supports_isolated_interpreters" => + ctx.new_bool(crate::vm::runtime::SUPPORTS_ISOLATED_INTERPRETERS), }) } diff --git a/crates/vm/src/vm/interpreter.rs b/crates/vm/src/vm/interpreter.rs index f456e8587ea..5d76b57cb0b 100644 --- a/crates/vm/src/vm/interpreter.rs +++ b/crates/vm/src/vm/interpreter.rs @@ -1,6 +1,11 @@ #[cfg(feature = "threading")] use super::StopTheWorldState; -use super::{Context, PyConfig, PyGlobalState, VirtualMachine, setting::Settings, thread}; +use super::{ + Context, PyConfig, PyGlobalState, VirtualMachine, + runtime::{self, InterpreterWhence}, + setting::Settings, + thread, +}; use crate::{ PyResult, builtins, common::rc::PyRc, frozen::FrozenModule, getpath, py_freeze, stdlib::atexit, vm::PyBaseExceptionRef, @@ -36,18 +41,34 @@ pub struct InterpreterBuilder { init_hooks: Vec, } -/// Private helper to initialize a VM with settings, context, and custom initialization. -fn initialize_main_vm( +/// Options for constructing a main or sub-interpreter VM. +struct InitializeVmOpts<'a> { settings: Settings, ctx: PyRc, module_defs: Vec<&'static builtins::PyModuleDef>, frozen_modules: Vec<(&'static str, FrozenModule)>, init_hooks: Vec, - init: F, -) -> (VirtualMachine, PyRc) + is_main: bool, + whence: InterpreterWhence, + /// When `Some`, reuse parent module_defs/frozen/config seeds for a subinterpreter. + parent_state: Option<&'a PyGlobalState>, +} + +/// Shared constructor for main and sub-interpreters. +fn initialize_vm(opts: InitializeVmOpts<'_>, init: F) -> (VirtualMachine, PyRc) where F: FnOnce(&mut VirtualMachine), { + let InitializeVmOpts { + settings, + ctx, + module_defs, + frozen_modules, + init_hooks, + is_main, + whence, + parent_state, + } = opts; use crate::codecs::CodecsRegistry; use crate::common::hash::HashSecret; use crate::common::lock::PyMutex; @@ -55,55 +76,84 @@ where use core::sync::atomic::{AtomicBool, AtomicU64}; use crossbeam_utils::atomic::AtomicCell; - let paths = getpath::init_path_config(&settings); - let config = PyConfig::new(settings, paths); + let (config, all_module_defs, frozen, hash_secret, int_max_str_digits) = + if let Some(parent) = parent_state { + // Subinterpreter: clone config and module tables from parent, fresh runtime state. + let int_max_str_digits = AtomicCell::new(parent.int_max_str_digits.load()); + ( + parent.config.clone(), + parent.module_defs.clone(), + parent.frozen.clone(), + parent.hash_secret, + int_max_str_digits, + ) + } else { + let paths = getpath::init_path_config(&settings); + let config = PyConfig::new(settings, paths); - // Build module_defs map from builtin modules + additional modules - let mut all_module_defs: BTreeMap<&'static str, &'static builtins::PyModuleDef> = - crate::stdlib::builtin_module_defs(&ctx) - .into_iter() - .chain(module_defs) - .map(|def| (def.name.as_str(), def)) - .collect(); + // Build module_defs map from builtin modules + additional modules + let mut all_module_defs: BTreeMap<&'static str, &'static builtins::PyModuleDef> = + crate::stdlib::builtin_module_defs(&ctx) + .into_iter() + .chain(module_defs) + .map(|def| (def.name.as_str(), def)) + .collect(); - // Register sysconfigdata under platform-specific name as well - if let Some(&sysconfigdata_def) = all_module_defs.get("_sysconfigdata") { - use std::sync::OnceLock; - static SYSCONFIGDATA_NAME: OnceLock<&'static str> = OnceLock::new(); - let leaked_name = *SYSCONFIGDATA_NAME.get_or_init(|| { - let name = crate::stdlib::sys::sysconfigdata_name(); - Box::leak(name.into_boxed_str()) - }); - all_module_defs.insert(leaked_name, sysconfigdata_def); - } + // Register sysconfigdata under platform-specific name as well + if let Some(&sysconfigdata_def) = all_module_defs.get("_sysconfigdata") { + use std::sync::OnceLock; + static SYSCONFIGDATA_NAME: OnceLock<&'static str> = OnceLock::new(); + let leaked_name = *SYSCONFIGDATA_NAME.get_or_init(|| { + let name = crate::stdlib::sys::sysconfigdata_name(); + Box::leak(name.into_boxed_str()) + }); + all_module_defs.insert(leaked_name, sysconfigdata_def); + } - // Create hash secret - let seed = match config.settings.hash_seed { - Some(seed) => seed, - None => super::process_hash_secret_seed(), - }; - let hash_secret = HashSecret::new(seed); + let seed = match config.settings.hash_seed { + Some(seed) => seed, + None => super::process_hash_secret_seed(), + }; + let hash_secret = HashSecret::new(seed); - // Create codec registry and warnings state + let int_max_str_digits = AtomicCell::new(match config.settings.int_max_str_digits { + -1 => 4300, + other => other, + } as usize); + + let mut frozen: std::collections::HashMap< + &'static str, + FrozenModule, + rapidhash::quality::RandomState, + > = core_frozen_inits().collect(); + frozen.extend(frozen_modules); + + ( + config, + all_module_defs, + frozen, + hash_secret, + int_max_str_digits, + ) + }; + + // Per-interpreter ephemeral state (must not be shared across interpreters). let codec_registry = CodecsRegistry::new(&ctx); let warnings = WarningsState::init_state(&ctx); - // Create int_max_str_digits - let int_max_str_digits = AtomicCell::new(match config.settings.int_max_str_digits { - -1 => 4300, - other => other, - } as usize); - - // Initialize frozen modules (core + user-provided) - let mut frozen: std::collections::HashMap< - &'static str, - FrozenModule, - rapidhash::quality::RandomState, - > = core_frozen_inits().collect(); - frozen.extend(frozen_modules); - - // Create PyGlobalState + let interpreter_id = runtime::alloc_interpreter_id(); + + // Process main OS thread identity is process-global; subinterpreters inherit + // it from the parent so `is_main_thread()` stays correct when running on the + // main OS thread under a subinterpreter. + #[cfg(feature = "threading")] + let main_thread_ident = AtomicCell::new(parent_state.map_or(0, |p| p.main_thread_ident.load())); + + // Create PyGlobalState (≈ PyInterpreterState) let global_state = PyRc::new(PyGlobalState { + interpreter_id, + whence, + is_main, config, module_defs: all_module_defs, frozen, @@ -124,7 +174,7 @@ where global_profile_func: PyMutex::default(), type_mutex: PyMutex::default(), #[cfg(feature = "threading")] - main_thread_ident: AtomicCell::new(0), + main_thread_ident, #[cfg(feature = "threading")] thread_frames: parking_lot::Mutex::new(std::collections::HashMap::new()), #[cfg(feature = "threading")] @@ -159,6 +209,7 @@ where // Clone global_state for Interpreter after all initialization is done let global_state = vm.state.clone(); + runtime::register_interpreter(&global_state); (vm, global_state) } @@ -271,12 +322,17 @@ impl InterpreterBuilder { /// This consumes the configuration and returns a fully initialized Interpreter. #[must_use] pub fn build(self) -> Interpreter { - let (vm, global_state) = initialize_main_vm( - self.settings, - self.ctx, - self.module_defs, - self.frozen_modules, - self.init_hooks, + let (vm, global_state) = initialize_vm( + InitializeVmOpts { + settings: self.settings, + ctx: self.ctx, + module_defs: self.module_defs, + frozen_modules: self.frozen_modules, + init_hooks: self.init_hooks, + is_main: true, + whence: InterpreterWhence::Runtime, + parent_state: None, + }, |_| {}, // No additional init needed ); Interpreter { global_state, vm } @@ -295,7 +351,13 @@ impl Default for InterpreterBuilder { } } -/// The general interface for the VM +/// One isolated Python interpreter in the process (≈ CPython `PyInterpreterState` + main tstate). +/// +/// Historically RustPython exposed a single process-level `Interpreter`. For PEP 734 +/// (multiple interpreters / subinterpreters) this type is now the owned handle for +/// **one** interpreter. Use [`Interpreter::create_subinterpreter`] to create additional +/// isolated interpreters that share the process-wide type context but not modules or +/// `PyGlobalState`. /// /// # Examples /// Runs a simple embedded hello world program. @@ -318,6 +380,12 @@ pub struct Interpreter { vm: VirtualMachine, } +impl Drop for Interpreter { + fn drop(&mut self) { + runtime::unregister_interpreter(self.global_state.interpreter_id); + } +} + impl Interpreter { /// Create a new interpreter configuration builder. /// @@ -350,17 +418,110 @@ impl Interpreter { where F: FnOnce(&mut VirtualMachine), { - let (vm, global_state) = initialize_main_vm( - settings, - Context::genesis().clone(), - Vec::new(), // No module_defs - Vec::new(), // No frozen_modules - Vec::new(), // No init_hooks + let (vm, global_state) = initialize_vm( + InitializeVmOpts { + settings, + ctx: Context::genesis().clone(), + module_defs: Vec::new(), + frozen_modules: Vec::new(), + init_hooks: Vec::new(), + is_main: true, + whence: InterpreterWhence::Runtime, + parent_state: None, + }, init, ); Self { global_state, vm } } + /// Process-global interpreter id (main is [`MAIN_INTERPRETER_ID`]). + #[inline] + #[must_use] + pub fn id(&self) -> i64 { + self.global_state.interpreter_id + } + + /// Where this interpreter was created. + #[inline] + #[must_use] + pub fn whence(&self) -> InterpreterWhence { + self.global_state.whence + } + + /// Whether this is the process main interpreter. + #[inline] + #[must_use] + pub fn is_main(&self) -> bool { + self.global_state.is_main + } + + /// Whether this is the PEP 734 process main interpreter (`get_main()`). + /// + /// Unlike [`Interpreter::is_main`], which is set for every top-level + /// interpreter, this is true for only the single first-registered main. + #[inline] + #[must_use] + pub fn is_process_main(&self) -> bool { + runtime::main_interpreter_id() == Some(self.id()) + } + + /// Create a subinterpreter and hand ownership to the runtime, returning its + /// id. The runtime keeps it alive until [`runtime::take_owned_interpreter`]. + /// + /// This is the shape `_interpreters.create()` will use: Python receives an + /// id, not an owned handle. + #[cfg(feature = "threading")] + #[must_use] + pub fn create_owned_subinterpreter(&self) -> i64 { + runtime::store_owned_interpreter(self.create_subinterpreter()) + } + + /// Create an isolated subinterpreter sharing this interpreter's type context + /// (`Context`) and module definitions, but with its own `sys.modules`, + /// builtins module instance, thread registry, and stop-the-world state. + /// + /// This is the Rust-side foundation for PEP 734 / `_interpreters.create()`. + /// It does not yet expose a Python module API. + /// + /// May be called while the parent is entered (matching CPython, where + /// `_interpreters.create()` runs under the main interpreter). When the + /// calling thread is currently attached to a VM, that attachment is + /// temporarily saved so the subinterpreter can bootstrap as an outermost + /// enter (correct thread-slot / stop-the-world state). + #[must_use] + pub fn create_subinterpreter(&self) -> Self { + // Suspend the caller's current VM attachment (if any) for the duration + // of subinterpreter initialization. Nested bootstrap would otherwise + // swap `CURRENT_THREAD_SLOT` to the new interpreter while leaving the + // outer interpreter's attach state inconsistent. Always restore, even + // if initialization panics. + #[cfg(feature = "threading")] + let _restore_parent = { + let saved = thread::current_vm_is_set().then(thread::save_current_thread); + scopeguard::guard(saved, |saved| { + if let Some(saved) = saved { + thread::restore_current_thread(saved); + } + }) + }; + + let (vm, global_state) = initialize_vm( + InitializeVmOpts { + // settings unused when parent_state is Some + settings: Settings::default(), + ctx: self.vm.ctx.clone(), + module_defs: Vec::new(), + frozen_modules: Vec::new(), + init_hooks: Vec::new(), + is_main: false, + whence: InterpreterWhence::Stdlib, + parent_state: Some(&self.global_state), + }, + |_| {}, + ); + Self { global_state, vm } + } + /// Run a function with the main virtual machine and return a PyResult of the result. /// /// To enter vm context multiple times or to avoid buffer/exception management, this function is preferred. @@ -582,8 +743,9 @@ fn core_frozen_inits() -> impl Iterator { mod tests { use super::*; use crate::{ - PyObjectRef, + AsObject, PyObjectRef, builtins::{PyStr, int}, + vm::{MAIN_INTERPRETER_ID, runtime}, }; use malachite_bigint::ToBigInt; @@ -608,4 +770,469 @@ mod tests { assert_eq!(value.as_wtf8(), "Hello Hello Hello Hello ") }) } + + /// Main interpreter is marked main with Runtime whence and is registered. + #[test] + fn main_interpreter_identity() { + let main = Interpreter::without_stdlib(Default::default()); + assert!(main.is_main()); + assert_eq!(main.whence(), InterpreterWhence::Runtime); + assert!( + runtime::list_interpreters() + .iter() + .any(|info| info.id == main.id() && info.whence == InterpreterWhence::Runtime) + ); + // When this is the sole sequential main in a quiet process, id is 0; + // under parallel tests the id is still unique and registered. + assert!(main.id() >= MAIN_INTERPRETER_ID); + } + + /// Subinterpreters get distinct ids, Stdlib whence, and appear in the registry. + #[test] + fn create_subinterpreter_registers_distinct_ids() { + let main = Interpreter::without_stdlib(Default::default()); + let sub1 = main.create_subinterpreter(); + let sub2 = main.create_subinterpreter(); + + assert!(main.is_main()); + assert!(!sub1.is_main()); + assert!(!sub2.is_main()); + assert_eq!(sub1.whence(), InterpreterWhence::Stdlib); + assert_eq!(sub2.whence(), InterpreterWhence::Stdlib); + assert_ne!(main.id(), sub1.id()); + assert_ne!(main.id(), sub2.id()); + assert_ne!(sub1.id(), sub2.id()); + + let ids: Vec = runtime::list_interpreters() + .into_iter() + .map(|i| i.id) + .collect(); + assert!(ids.contains(&main.id())); + assert!(ids.contains(&sub1.id())); + assert!(ids.contains(&sub2.id())); + } + + /// Dropping a subinterpreter unregisters it; main remains. + #[test] + fn drop_subinterpreter_unregisters() { + let main = Interpreter::without_stdlib(Default::default()); + let sub_id = { + let sub = main.create_subinterpreter(); + let id = sub.id(); + assert!(runtime::lookup_interpreter(id).is_some()); + id + }; + assert!(runtime::lookup_interpreter(sub_id).is_none()); + assert!(runtime::lookup_interpreter(main.id()).is_some()); + } + + /// Each interpreter has its own `sys.modules` / builtins module instance. + #[test] + fn subinterpreters_isolate_modules() { + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + + let (main_sys_ptr, main_builtins_ptr, main_ctx_ptr, main_state_ptr) = main.enter(|vm| { + ( + vm.sys_module.as_object() as *const _, + vm.builtins.as_object() as *const _, + PyRc::as_ptr(&vm.ctx), + PyRc::as_ptr(&vm.state), + ) + }); + let (sub_sys_ptr, sub_builtins_ptr, sub_ctx_ptr, sub_state_ptr) = sub.enter(|vm| { + ( + vm.sys_module.as_object() as *const _, + vm.builtins.as_object() as *const _, + PyRc::as_ptr(&vm.ctx), + PyRc::as_ptr(&vm.state), + ) + }); + + assert_ne!(main_sys_ptr, sub_sys_ptr); + assert_ne!(main_builtins_ptr, sub_builtins_ptr); + // Distinct per-interpreter state. + assert_ne!(main_state_ptr, sub_state_ptr); + // Shared process-wide type context (immortal / builtin types). + assert_eq!(main_ctx_ptr, sub_ctx_ptr); + } + + /// Mutations to interpreter-owned modules must not leak between interpreters. + #[test] + fn subinterpreters_behaviorally_isolate_builtins_and_sys_modules() { + const PROBE: &str = "__rustpython_subinterpreter_isolation_probe__"; + + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + + main.enter(|vm| { + vm.builtins + .set_attr(PROBE, vm.ctx.new_int(11_i32), vm) + .unwrap(); + vm.sys_module + .get_attr("modules", vm) + .unwrap() + .set_item(PROBE, vm.ctx.new_int(12_i32).into(), vm) + .unwrap(); + }); + + sub.enter(|vm| { + assert!(vm.builtins.get_attr(PROBE, vm).is_err()); + let modules = vm.sys_module.get_attr("modules", vm).unwrap(); + assert!(modules.get_item(PROBE, vm).is_err()); + + vm.builtins + .set_attr(PROBE, vm.ctx.new_int(21_i32), vm) + .unwrap(); + modules + .set_item(PROBE, vm.ctx.new_int(22_i32).into(), vm) + .unwrap(); + }); + + main.enter(|vm| { + let builtin_probe = vm.builtins.get_attr(PROBE, vm).unwrap(); + assert_eq!(*int::get_value(&builtin_probe), 11_i32.to_bigint().unwrap()); + + let module_probe = vm + .sys_module + .get_attr("modules", vm) + .unwrap() + .get_item(PROBE, vm) + .unwrap(); + assert_eq!(*int::get_value(&module_probe), 12_i32.to_bigint().unwrap()); + }); + } + + /// Creating a subinterpreter while the parent is entered must not corrupt + /// the parent's current-VM / thread-slot state. + #[test] + fn create_subinterpreter_while_parent_entered() { + let main = Interpreter::without_stdlib(Default::default()); + main.enter(|vm| { + let before = vm.state.interpreter_id; + let sub = main.create_subinterpreter(); + assert_ne!(sub.id(), before); + // Still the parent after create returns. + assert_eq!(vm.state.interpreter_id, before); + // Can still use the parent VM. + let n: PyObjectRef = vm.ctx.new_int(7_i32).into(); + assert_eq!(int::get_value(&n), &7_i32.to_bigint().unwrap()); + // And the sub is independently usable after parent section. + drop(sub); + }); + } + + /// Sequential enter of main then sub on the same OS thread is safe. + #[test] + fn sequential_enter_main_and_sub() { + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + + main.enter(|vm| { + assert!(vm.state.is_main_interpreter()); + let a: PyObjectRef = vm.ctx.new_int(1_i32).into(); + let b: PyObjectRef = vm.ctx.new_int(2_i32).into(); + let res = vm._add(&a, &b).unwrap(); + assert_eq!(*int::get_value(&res), 3_i32.to_bigint().unwrap()); + }); + sub.enter(|vm| { + assert!(!vm.state.is_main_interpreter()); + let a: PyObjectRef = vm.ctx.new_int(10_i32).into(); + let b: PyObjectRef = vm.ctx.new_int(5_i32).into(); + let res = vm._mul(&a, &b).unwrap(); + assert_eq!(*int::get_value(&res), 50_i32.to_bigint().unwrap()); + }); + // Re-enter main after sub. + main.enter(|vm| { + assert!(vm.state.is_main_interpreter()); + }); + } + + /// Concurrent use of main + subinterpreter on different OS threads. + #[cfg(feature = "threading")] + #[test] + fn concurrent_main_and_subinterpreter_threads() { + use alloc::sync::Arc; + use core::sync::atomic::{AtomicUsize, Ordering}; + + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + let counter = Arc::new(AtomicUsize::new(0)); + + let c1 = Arc::clone(&counter); + let h_main = main.enter(|vm| { + let thread_vm = vm.new_thread(); + let c = Arc::clone(&c1); + std::thread::spawn(move || { + thread_vm.run(|vm| { + for _ in 0..100 { + let a: PyObjectRef = vm.ctx.new_int(1_i32).into(); + let b: PyObjectRef = vm.ctx.new_int(1_i32).into(); + let _ = vm._add(&a, &b).unwrap(); + c.fetch_add(1, Ordering::Relaxed); + } + assert!(vm.state.is_main_interpreter()); + }); + }) + }); + + let c2 = Arc::clone(&counter); + let h_sub = sub.enter(|vm| { + let thread_vm = vm.new_thread(); + let c = Arc::clone(&c2); + std::thread::spawn(move || { + thread_vm.run(|vm| { + for _ in 0..100 { + let a: PyObjectRef = vm.ctx.new_int(2_i32).into(); + let b: PyObjectRef = vm.ctx.new_int(3_i32).into(); + let _ = vm._mul(&a, &b).unwrap(); + c.fetch_add(1, Ordering::Relaxed); + } + assert!(!vm.state.is_main_interpreter()); + }); + }) + }); + + h_main.join().expect("main worker panicked"); + h_sub.join().expect("sub worker panicked"); + assert_eq!(counter.load(Ordering::Relaxed), 200); + } + + /// Entering one interpreter must not serialize entry into another interpreter. + #[cfg(feature = "threading")] + #[test] + fn main_and_subinterpreter_run_sections_overlap() { + use alloc::sync::Arc; + use core::time::Duration; + use std::{ + sync::{Condvar, Mutex}, + time::Instant, + }; + + #[derive(Default)] + struct OverlapState { + entered: usize, + release: bool, + } + + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + let state = Arc::new((Mutex::new(OverlapState::default()), Condvar::new())); + + let spawn_worker = |interpreter: &Interpreter| { + let state = Arc::clone(&state); + interpreter.enter(|vm| { + let thread_vm = vm.new_thread(); + std::thread::spawn(move || { + thread_vm.run(|vm| { + let a: PyObjectRef = vm.ctx.new_int(20_i32).into(); + let b: PyObjectRef = vm.ctx.new_int(22_i32).into(); + assert_eq!( + *int::get_value(&vm._add(&a, &b).unwrap()), + 42_i32.to_bigint().unwrap() + ); + + let (lock, ready) = &*state; + let mut state = lock.lock().unwrap(); + state.entered += 1; + ready.notify_all(); + while !state.release { + state = ready.wait(state).unwrap(); + } + }); + }) + }) + }; + + let main_worker = spawn_worker(&main); + let sub_worker = spawn_worker(&sub); + + let (lock, ready) = &*state; + let deadline = Instant::now() + Duration::from_secs(2); + let mut state_guard = lock.lock().unwrap(); + while state_guard.entered < 2 { + let now = Instant::now(); + if now >= deadline { + break; + } + let (next, _) = ready.wait_timeout(state_guard, deadline - now).unwrap(); + state_guard = next; + } + let overlapped = state_guard.entered == 2; + state_guard.release = true; + ready.notify_all(); + drop(state_guard); + + main_worker.join().expect("main worker panicked"); + sub_worker.join().expect("subinterpreter worker panicked"); + assert!( + overlapped, + "main and subinterpreter run sections were serialized" + ); + } + + /// A busy interpreter must not prevent another interpreter from making progress. + #[cfg(feature = "threading")] + #[test] + fn busy_main_interpreter_does_not_block_subinterpreter() { + use alloc::sync::Arc; + use core::{ + sync::atomic::{AtomicBool, Ordering}, + time::Duration, + }; + use std::time::Instant; + + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + let main_started = Arc::new(AtomicBool::new(false)); + let sub_finished = Arc::new(AtomicBool::new(false)); + + let main_started_worker = Arc::clone(&main_started); + let sub_finished_worker = Arc::clone(&sub_finished); + let main_worker = main.enter(|vm| { + let thread_vm = vm.new_thread(); + std::thread::spawn(move || { + thread_vm.run(|vm| { + main_started_worker.store(true, Ordering::Release); + let deadline = Instant::now() + Duration::from_secs(2); + let mut operations = 0; + while !sub_finished_worker.load(Ordering::Acquire) && Instant::now() < deadline + { + let a: PyObjectRef = vm.ctx.new_int(20_i32).into(); + let b: PyObjectRef = vm.ctx.new_int(22_i32).into(); + let result = vm._add(&a, &b).unwrap(); + assert_eq!(*int::get_value(&result), 42_i32.to_bigint().unwrap()); + operations += 1; + std::thread::yield_now(); + } + (sub_finished_worker.load(Ordering::Acquire), operations) + }) + }) + }); + + let main_started_worker = Arc::clone(&main_started); + let sub_finished_worker = Arc::clone(&sub_finished); + let sub_worker = sub.enter(|vm| { + let thread_vm = vm.new_thread(); + std::thread::spawn(move || { + while !main_started_worker.load(Ordering::Acquire) { + std::thread::yield_now(); + } + thread_vm.run(|vm| { + let a: PyObjectRef = vm.ctx.new_int(6_i32).into(); + let b: PyObjectRef = vm.ctx.new_int(7_i32).into(); + let result = vm._mul(&a, &b).unwrap(); + assert_eq!(*int::get_value(&result), 42_i32.to_bigint().unwrap()); + sub_finished_worker.store(true, Ordering::Release); + }); + }) + }); + + let (sub_progressed_while_main_was_busy, main_operations) = + main_worker.join().expect("main worker panicked"); + sub_worker.join().expect("subinterpreter worker panicked"); + + assert!(main_operations > 0); + assert!( + sub_progressed_while_main_was_busy, + "subinterpreter made no progress until the busy main interpreter exited" + ); + } + + /// `new_thread` on a subinterpreter shares that subinterpreter's state, not main's. + #[cfg(feature = "threading")] + #[test] + fn subinterpreter_new_thread_shares_sub_state() { + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + let sub_id = sub.id(); + + let handle = sub.enter(|vm| { + let thread_vm = vm.new_thread(); + std::thread::spawn(move || { + thread_vm.run(|vm| { + assert_eq!(vm.state.interpreter_id, sub_id); + assert!(!vm.state.is_main_interpreter()); + }); + }) + }); + handle.join().expect("thread panicked"); + } + + /// Multiple subinterpreters can each run bytecode via compile+exec. + #[cfg(feature = "rustpython-compiler")] + #[test] + fn subinterpreter_runs_python_code() { + use crate::compiler::Mode; + + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + + sub.enter(|vm| { + let scope = vm.new_scope_with_builtins(); + let source = "x = 40 + 2\n"; + let code = vm + .compile(source, Mode::Exec, "") + .map_err(|err| err.into_pyexception(vm, Some(source))) + .unwrap(); + vm.run_code_obj(code, scope.clone()).unwrap(); + let x = scope.globals.get_item("x", vm).unwrap(); + assert_eq!(*int::get_value(&x), 42_i32.to_bigint().unwrap()); + }); + } + + /// The runtime can own a subinterpreter by id and hand it back on destroy. + #[cfg(feature = "threading")] + #[test] + fn runtime_owned_interpreter_lifecycle() { + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + let id = sub.id(); + + let before = runtime::owned_interpreter_count(); + assert_eq!(runtime::store_owned_interpreter(sub), id); + assert!(runtime::is_owned_interpreter(id)); + assert!(runtime::lookup_interpreter(id).is_some()); + assert_eq!(runtime::owned_interpreter_count(), before + 1); + + // Reclaiming removes ownership but keeps the interpreter alive while the + // returned handle is held. + let reclaimed = runtime::take_owned_interpreter(id).expect("owned by runtime"); + assert_eq!(reclaimed.id(), id); + assert!(!runtime::is_owned_interpreter(id)); + assert!(runtime::lookup_interpreter(id).is_some()); + assert!(runtime::take_owned_interpreter(id).is_none()); + + // Dropping the reclaimed handle unregisters it. + drop(reclaimed); + assert!(runtime::lookup_interpreter(id).is_none()); + } + + /// `create_owned_subinterpreter` stores the sub and returns only its id. + #[cfg(feature = "threading")] + #[test] + fn create_owned_subinterpreter_returns_id() { + let main = Interpreter::without_stdlib(Default::default()); + let id = main.create_owned_subinterpreter(); + assert!(runtime::is_owned_interpreter(id)); + assert_ne!(id, main.id()); + + let sub = runtime::take_owned_interpreter(id).expect("owned by runtime"); + assert_eq!(sub.id(), id); + assert!(!sub.is_main()); + } + + /// The process main id is recorded once and is stable across later creates. + #[test] + fn process_main_id_recorded_and_stable() { + // At least one main exists by now (this one, if not an earlier test), so + // `get_main()` is populated. + let main = Interpreter::without_stdlib(Default::default()); + let recorded = runtime::main_interpreter_id().expect("a process main exists"); + + // Recording is once-only: further interpreters do not displace it. + let _sub = main.create_subinterpreter(); + let _main2 = Interpreter::without_stdlib(Default::default()); + assert_eq!(runtime::main_interpreter_id(), Some(recorded)); + } } diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index 7c7d017c1fd..df71227b3a8 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -13,6 +13,7 @@ mod interpreter; mod method; #[cfg(feature = "rustpython-compiler")] mod python_run; +pub mod runtime; mod setting; pub mod thread; mod vm_new; @@ -61,16 +62,22 @@ use std::{ pub use context::Context; pub use interpreter::{Interpreter, InterpreterBuilder}; pub(crate) use method::PyMethod; +pub use runtime::{InterpreterInfo, InterpreterWhence, MAIN_INTERPRETER_ID}; pub use setting::{CheckHashPycsMode, Paths, PyConfig, Settings}; pub const MAX_MEMORY_SIZE: usize = isize::MAX as usize; // Objects are live when they are on stack, or referenced by a name (for now) -/// Top level container of a python virtual machine. In theory you could -/// create more instances of this struct and have them operate fully isolated. +/// Per-thread execution context for a single interpreter (≈ CPython `PyThreadState`). /// -/// To construct this, please refer to the [`Interpreter`] +/// A `VirtualMachine` holds thread-local eval state (exceptions, recursion, frames, +/// datastack) plus shared references to interpreter-owned data (`state`, +/// `builtins`, `sys_module`, `ctx`). Multiple VMs may share the same +/// [`PyGlobalState`] via [`VirtualMachine::new_thread`]; distinct interpreters +/// each have their own `PyGlobalState` (see [`Interpreter::create_subinterpreter`]). +/// +/// To construct the main VM of an interpreter, use [`Interpreter`]. pub struct VirtualMachine { pub builtins: PyRef, pub sys_module: PyRef, @@ -732,7 +739,18 @@ pub(crate) struct CallableCache { pub builtin_any: Option, } +/// Per-interpreter shared state (≈ CPython `PyInterpreterState`). +/// +/// Not process-global: each [`Interpreter`] (main or subinterpreter) owns its own +/// `PyGlobalState`. Process-wide pieces live elsewhere (`Context::genesis`, +/// GC, the interpreter registry in [`runtime`]). pub struct PyGlobalState { + /// Unique process-global interpreter id (main is [`MAIN_INTERPRETER_ID`]). + pub interpreter_id: i64, + /// How this interpreter was created. + pub whence: runtime::InterpreterWhence, + /// True only for the process main interpreter. + pub is_main: bool, pub config: PyConfig, pub module_defs: BTreeMap<&'static str, &'static builtins::PyModuleDef>, pub frozen: HashMap<&'static str, FrozenModule, rapidhash::quality::RandomState>, @@ -779,6 +797,14 @@ pub struct PyGlobalState { pub stop_the_world: StopTheWorldState, } +impl PyGlobalState { + #[inline] + #[must_use] + pub fn is_main_interpreter(&self) -> bool { + self.is_main + } +} + pub fn process_hash_secret_seed() -> u32 { use std::sync::OnceLock; static SEED: OnceLock = OnceLock::new(); @@ -1083,9 +1109,12 @@ impl VirtualMachine { assert!(!self.initialized, "Double Initialize Error"); - // Initialize main thread ident before any threading operations + // Process main-thread identity is owned by the main interpreter only + // (used for signal handling / `_thread._is_main_interpreter` helpers). #[cfg(feature = "threading")] - stdlib::_thread::init_main_thread_ident(self); + if self.state.is_main_interpreter() { + stdlib::_thread::init_main_thread_ident(self); + } stdlib::builtins::init_module(self, &self.builtins); let callable_cache_init = self.init_callable_cache(); diff --git a/crates/vm/src/vm/runtime.rs b/crates/vm/src/vm/runtime.rs new file mode 100644 index 00000000000..180a1b9ce99 --- /dev/null +++ b/crates/vm/src/vm/runtime.rs @@ -0,0 +1,241 @@ +//! Process-global runtime support for multiple interpreters (PEP 734 preparation). +//! +//! CPython maps roughly as: +//! - this module ≈ `_PyRuntimeState.interpreters` + ID allocation +//! - [`crate::vm::PyGlobalState`] ≈ `PyInterpreterState` +//! - [`crate::VirtualMachine`] ≈ `PyThreadState` (plus shared refs to interpreter state) +//! +//! Multiple [`crate::Interpreter`] instances can coexist in one process. Each owns +//! an isolated `PyGlobalState` (modules, codecs, thread registry, stop-the-world, …) +//! while sharing the process-wide [`crate::Context`] (builtin types / immortals). + +use crate::common::rc::PyRc; +use crate::vm::PyGlobalState; +use core::sync::atomic::{AtomicI64, Ordering}; +use parking_lot::Mutex; +use std::collections::HashMap; +use std::sync::OnceLock; + +/// Where an interpreter state came from (mirrors CPython `_PyInterpreterState_GetWhence`). +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +#[repr(i32)] +pub enum InterpreterWhence { + /// Unknown / not recorded. + Unknown = 0, + /// Created as the process main interpreter at runtime init. + Runtime = 1, + /// Legacy C-API creation path (reserved for C-API parity). + LegacyCapi = 2, + /// Modern C-API creation path (reserved for C-API parity). + Capi = 3, + /// Cross-interpreter C-API (reserved). + Xi = 4, + /// Created via the stdlib / Rust subinterpreter API (PEP 734). + Stdlib = 5, +} + +impl InterpreterWhence { + #[must_use] + pub const fn as_i32(self) -> i32 { + self as i32 + } +} + +/// Snapshot of a registered interpreter for enumeration APIs. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct InterpreterInfo { + pub id: i64, + pub whence: InterpreterWhence, +} + +struct RegistryEntry { + whence: InterpreterWhence, + /// Weak handle so the registry does not keep interpreters alive. + /// Type matches `PyRc` (Arc when threading, Rc otherwise). + #[cfg(feature = "threading")] + state: alloc::sync::Weak, + #[cfg(not(feature = "threading"))] + state: alloc::rc::Weak, +} + +struct InterpreterRegistry { + next_id: AtomicI64, + /// id → entry. Main interpreter is always id 0 when created first. + entries: Mutex>, +} + +// Without the `threading` feature `RegistryEntry` holds `rc::Weak`, which is +// `!Send`/`!Sync`, so the process-global registry static would not type-check. +// SAFETY: non-threading builds are single-threaded by construction (`Rc`-based +// objects are already unsound to touch across threads), and this process-global +// registry is only ever reached from that one thread. The `parking_lot::Mutex` +// still guards the map contents against reentrancy. +#[cfg(not(feature = "threading"))] +unsafe impl Send for InterpreterRegistry {} +#[cfg(not(feature = "threading"))] +unsafe impl Sync for InterpreterRegistry {} + +impl InterpreterRegistry { + fn new() -> Self { + Self { + // Monotonic ids starting at 0. Concurrent Interpreter construction + // (e.g. cargo test threads) must never share an id. + next_id: AtomicI64::new(0), + entries: Mutex::new(HashMap::new()), + } + } +} + +fn registry() -> &'static InterpreterRegistry { + static REGISTRY: OnceLock = OnceLock::new(); + REGISTRY.get_or_init(InterpreterRegistry::new) +} + +/// Conventional id of the first process main interpreter when allocation is +/// sequential (CPython parity). Concurrent construction may assign other ids; +/// use [`PyGlobalState::is_main`] / [`crate::Interpreter::is_main`] to identify +/// a main interpreter, not this constant alone. +pub const MAIN_INTERPRETER_ID: i64 = 0; + +/// Backs `sys.implementation.supports_isolated_interpreters`. +/// +/// The Rust substrate already isolates interpreters (`PyGlobalState` per +/// interpreter, per-interpreter thread slots / stop-the-world). This stays +/// `false` until the Python-facing `_interpreters` module is wired up; flip it +/// in the commit that lands `_interpreters`. +pub const SUPPORTS_ISOLATED_INTERPRETERS: bool = false; + +/// Process main interpreter id for PEP 734 `_interpreters.get_main()`, recorded +/// once when the first `is_main` interpreter is registered. +static MAIN_INTERPRETER: OnceLock = OnceLock::new(); + +/// Id of the process main interpreter (PEP 734 `get_main()`), or `None` before +/// any interpreter has been created. +/// +/// This is distinct from [`PyGlobalState::is_main`]: every top-level (non-sub) +/// interpreter carries `is_main` for its own signal / main-thread bookkeeping, +/// but only the first one registered becomes *the* process main. +#[must_use] +pub fn main_interpreter_id() -> Option { + MAIN_INTERPRETER.get().copied() +} + +/// Allocate a unique process-global interpreter id. +/// +/// Ids are strictly monotonic and never reused for the lifetime of the process. +/// This is required for thread-safe concurrent `Interpreter` construction +/// (parallel unit tests, multi-threaded embedding). +pub(crate) fn alloc_interpreter_id() -> i64 { + registry().next_id.fetch_add(1, Ordering::Relaxed) +} + +/// Register an interpreter state in the process-global table. +pub(crate) fn register_interpreter(state: &PyRc) { + let id = state.interpreter_id; + let whence = state.whence; + if state.is_main { + // First `is_main` interpreter defines the process main for `get_main()`. + // Additional top-level Interpreters (embedding) keep their own `is_main` + // flag but do not displace the recorded process main. + let _ = MAIN_INTERPRETER.set(id); + } + let mut entries = registry().entries.lock(); + entries.insert( + id, + RegistryEntry { + whence, + state: PyRc::downgrade(state), + }, + ); +} + +/// Unregister an interpreter (called when its owning `Interpreter` is dropped). +pub(crate) fn unregister_interpreter(id: i64) { + registry().entries.lock().remove(&id); +} + +/// Look up a live interpreter state by id. +#[must_use] +pub fn lookup_interpreter(id: i64) -> Option> { + let entries = registry().entries.lock(); + entries.get(&id).and_then(|e| e.state.upgrade()) +} + +/// List all currently registered (still-alive) interpreters. +#[must_use] +pub fn list_interpreters() -> Vec { + let entries = registry().entries.lock(); + let mut out: Vec = entries + .iter() + .filter_map(|(&id, entry)| { + // Drop dead weak refs from the listing. + if entry.state.strong_count() == 0 { + return None; + } + Some(InterpreterInfo { + id, + whence: entry.whence, + }) + }) + .collect(); + out.sort_by_key(|info| info.id); + out +} + +/// Number of registered interpreters that are still alive. +#[must_use] +pub fn interpreter_count() -> usize { + list_interpreters().len() +} + +/// Runtime-owned interpreters (the ownership anchor for the Python +/// `_interpreters` API). +/// +/// A Rust [`crate::Interpreter`] handle is normally owned by its Rust caller. +/// For PEP 734, `_interpreters.create()` returns only an id and the runtime +/// must keep the interpreter alive until `_interpreters.destroy(id)`. These +/// functions hold that ownership, keyed by interpreter id, while the weak +/// [`registry`] above still drives enumeration and lookup. +/// +/// Only available with the `threading` feature: a runtime-owned interpreter is +/// reachable from other OS threads, which requires `Interpreter: Send` (true +/// only when `PyObjectRef` is `Arc`-backed). +#[cfg(feature = "threading")] +fn owned_interpreters() -> &'static Mutex> { + static OWNED: OnceLock>> = OnceLock::new(); + OWNED.get_or_init(|| Mutex::new(HashMap::new())) +} + +/// Transfer ownership of `interp` to the runtime, returning its id. +#[cfg(feature = "threading")] +pub fn store_owned_interpreter(interp: crate::Interpreter) -> i64 { + let id = interp.id(); + // Ids are strictly monotonic, so this never displaces (and drops) an + // existing entry under the lock. + owned_interpreters().lock().insert(id, interp); + id +} + +/// Reclaim a runtime-owned interpreter, removing it from the owner table. +/// +/// The returned handle is dropped by the caller *outside* the owner lock; its +/// `Drop` unregisters the interpreter from the weak [`registry`]. +#[cfg(feature = "threading")] +#[must_use] +pub fn take_owned_interpreter(id: i64) -> Option { + owned_interpreters().lock().remove(&id) +} + +/// Whether `id` refers to a runtime-owned interpreter. +#[cfg(feature = "threading")] +#[must_use] +pub fn is_owned_interpreter(id: i64) -> bool { + owned_interpreters().lock().contains_key(&id) +} + +/// Number of runtime-owned interpreters currently alive. +#[cfg(feature = "threading")] +#[must_use] +pub fn owned_interpreter_count() -> usize { + owned_interpreters().lock().len() +} diff --git a/crates/vm/src/vm/setting.rs b/crates/vm/src/vm/setting.rs index 7298c95ab08..3c42ca0b6fc 100644 --- a/crates/vm/src/vm/setting.rs +++ b/crates/vm/src/vm/setting.rs @@ -25,6 +25,7 @@ pub struct Paths { /// Combined configuration: user settings + computed paths /// CPython directly exposes every fields under both of them. /// We separate them to maintain better ownership discipline. +#[derive(Clone)] pub struct PyConfig { pub settings: Settings, pub paths: Paths, @@ -39,6 +40,7 @@ impl PyConfig { /// User-configurable settings for the python vm. #[non_exhaustive] +#[derive(Clone)] pub struct Settings { /// -I pub isolated: bool, diff --git a/crates/vm/src/vm/thread.rs b/crates/vm/src/vm/thread.rs index 1bab539a0a6..683a09257dc 100644 --- a/crates/vm/src/vm/thread.rs +++ b/crates/vm/src/vm/thread.rs @@ -22,6 +22,8 @@ use core::{ sync::atomic::{AtomicUsize, Ordering}, }; use itertools::Itertools; +#[cfg(feature = "threading")] +use std::collections::HashMap; use std::thread_local; // Thread states for stop-the-world support. @@ -90,7 +92,17 @@ thread_local! { pub(crate) static COROUTINE_ORIGIN_TRACKING_DEPTH: Cell = const { Cell::new(0) }; - /// Current thread's slot for sys._current_frames() and sys._current_exceptions() + /// Per-interpreter thread slots for this OS thread (PEP 734 multi-interpreter). + /// + /// CPython keeps a `PyThreadState` per (thread, interpreter) pair. RustPython + /// mirrors that: each interpreter's `PyGlobalState.thread_frames` gets its own + /// [`ThreadSlot`] for this OS thread. `CURRENT_THREAD_SLOT` always points at + /// the slot for the currently entered interpreter. + #[cfg(feature = "threading")] + static INTERP_THREAD_SLOTS: RefCell> = + RefCell::new(HashMap::new()); + + /// Current thread's slot for the currently entered interpreter. #[cfg(feature = "threading")] static CURRENT_THREAD_SLOT: RefCell> = const { RefCell::new(None) }; @@ -131,15 +143,35 @@ pub fn with_current_vm(f: impl FnOnce(&VirtualMachine) -> R) -> R { } fn set_current_vm(vm: &VirtualMachine, f: impl FnOnce() -> R) -> R { + // Ensure the thread slot matches this VM's interpreter before it becomes + // current (important when switching interpreters on one OS thread). + #[cfg(feature = "threading")] + init_thread_slot_if_needed(vm); + VM_STACK.with(|vms| { vms.borrow_mut().push(vm.into()); scopeguard::defer! { vms.borrow_mut().pop(); + // Restore the slot for the VM that is current after pop (if any). + #[cfg(feature = "threading")] + restore_thread_slot_from_stack(); } f() }) } +/// After popping the VM stack, point `CURRENT_THREAD_SLOT` at the new top VM's +/// interpreter slot so nested multi-interpreter enters unwind cleanly. +#[cfg(feature = "threading")] +fn restore_thread_slot_from_stack() { + let prev = VM_STACK.with(|vms| vms.borrow().last().copied()); + if let Some(vm_ptr) = prev { + // SAFETY: entries on VM_STACK are valid for the enter/set_current_vm scope. + let vm = unsafe { vm_ptr.as_ref() }; + init_thread_slot_if_needed(vm); + } +} + pub fn try_with_current_vm(f: impl FnOnce(&VirtualMachine) -> R) -> Option { VM_STACK.with(|vms| { let vm = vms.borrow().last().copied()?; @@ -225,6 +257,10 @@ impl Drop for VmBootstrapGuard { #[cfg(feature = "threading")] if self.was_outermost { detach_thread(); + } else { + // Nested bootstrap: restore the outer VM's interpreter thread slot. + #[cfg(feature = "threading")] + restore_thread_slot_from_stack(); } } } @@ -287,7 +323,13 @@ pub fn restore_current_thread(state: SavedThreadState) { // SAFETY: borrowed VMs remain alive for the dynamic save/restore scope, // while an owned GILState VM was restored above before this dereference. - attach_thread(unsafe { vm.as_ref() }); + let vm = unsafe { vm.as_ref() }; + // Point CURRENT_THREAD_SLOT at the restored interpreter before attach. + // After subinterpreter bootstrap, CURRENT may still refer to the temporary + // subinterpreter slot (DETACHED); attaching that would leave the parent + // slot detached and later confuse outermost detach. + init_thread_slot_if_needed(vm); + attach_thread(vm); VM_STACK.with(|vms| *vms.borrow_mut() = vm_stack); } @@ -340,40 +382,53 @@ pub fn release_current_thread(state: CurrentVmAttachState) { detach_thread(); } -/// Initialize thread slot for current thread if not already initialized. -/// Called automatically by enter_vm(). +/// Ensure this OS thread has a [`ThreadSlot`] registered with `vm`'s interpreter +/// and make it the current slot. +/// +/// Called automatically by `enter_vm()` / `VmBootstrapGuard` whenever a VM +/// becomes current. Switching between interpreters on the same OS thread swaps +/// `CURRENT_THREAD_SLOT` to that interpreter's slot (creating one if needed). #[cfg(feature = "threading")] fn init_thread_slot_if_needed(vm: &VirtualMachine) { - CURRENT_THREAD_SLOT.with(|slot| { - if slot.borrow().is_none() { - let thread_id = crate::stdlib::_thread::get_ident(); - let mut registry = vm.state.thread_frames.lock(); - let new_slot = Arc::new(ThreadSlot { - #[cfg(unix)] - top_frame: AtomicPtr::new(core::ptr::null_mut()), - top_iframe: AtomicUsize::new(0), - #[cfg(not(unix))] - frames: parking_lot::Mutex::new(Vec::new()), - exception: crate::PyAtomicRef::from(None::), - state: core::sync::atomic::AtomicI32::new( - if vm.state.stop_the_world.requested.load(Ordering::Acquire) { - // Match init_threadstate(): new thread-state starts - // suspended while stop-the-world is active. - THREAD_SUSPENDED - } else { - THREAD_DETACHED - }, - ), - stop_requested: core::sync::atomic::AtomicBool::new(false), - thread: std::thread::current(), - qsbr: crate::object::qsbr::QSBR.register(), - }); - registry.insert(thread_id, new_slot.clone()); - drop(registry); - #[cfg(all(unix, feature = "threading"))] - CURRENT_TOP_FRAME_SLOT.with(|c| c.set(&new_slot.top_frame)); - *slot.borrow_mut() = Some(new_slot); + let interp_id = vm.state.interpreter_id; + let slot = INTERP_THREAD_SLOTS.with(|slots| { + let mut slots = slots.borrow_mut(); + if let Some(existing) = slots.get(&interp_id) { + return existing.clone(); } + + let thread_id = crate::stdlib::_thread::get_ident(); + let mut registry = vm.state.thread_frames.lock(); + let new_slot = Arc::new(ThreadSlot { + #[cfg(unix)] + top_frame: AtomicPtr::new(core::ptr::null_mut()), + top_iframe: AtomicUsize::new(0), + #[cfg(not(unix))] + frames: parking_lot::Mutex::new(Vec::new()), + exception: crate::PyAtomicRef::from(None::), + state: core::sync::atomic::AtomicI32::new( + if vm.state.stop_the_world.requested.load(Ordering::Acquire) { + // Match init_threadstate(): new thread-state starts + // suspended while stop-the-world is active. + THREAD_SUSPENDED + } else { + THREAD_DETACHED + }, + ), + stop_requested: core::sync::atomic::AtomicBool::new(false), + thread: std::thread::current(), + qsbr: crate::object::qsbr::QSBR.register(), + }); + registry.insert(thread_id, new_slot.clone()); + drop(registry); + slots.insert(interp_id, new_slot.clone()); + new_slot + }); + + #[cfg(all(unix, feature = "threading"))] + CURRENT_TOP_FRAME_SLOT.with(|c| c.set(&slot.top_frame)); + CURRENT_THREAD_SLOT.with(|current| { + *current.borrow_mut() = Some(slot); }); } @@ -759,16 +814,21 @@ pub fn get_all_current_exceptions(vm: &VirtualMachine) -> Vec<(u64, Option registry.remove(&thread_id), @@ -789,7 +849,6 @@ pub fn cleanup_current_thread_frames(vm: &VirtualMachine) { None }; - #[cfg(feature = "threading")] if let Some(slot) = &_removed && vm.state.stop_the_world.requested.load(Ordering::Acquire) && thread_id != vm.state.stop_the_world.requester_ident() @@ -799,12 +858,19 @@ pub fn cleanup_current_thread_frames(vm: &VirtualMachine) { // Unblock requester countdown progress. vm.state.stop_the_world.notify_thread_gone(); } - // Clear the cached top-frame pointer before dropping the slot Arc so no - // later `set_current_frame` dereferences freed slot memory. - #[cfg(all(unix, feature = "threading"))] - CURRENT_TOP_FRAME_SLOT.with(|c| c.set(core::ptr::null())); + + // If CURRENT pointed at the cleaned slot, clear it (and top-frame cache). CURRENT_THREAD_SLOT.with(|s| { - *s.borrow_mut() = None; + let clear = match (s.borrow().as_ref(), slot_to_clean.as_ref()) { + (Some(cur), Some(cleaned)) => Arc::ptr_eq(cur, cleaned), + (Some(_), None) => false, + (None, _) => false, + }; + if clear { + *s.borrow_mut() = None; + #[cfg(all(unix, feature = "threading"))] + CURRENT_TOP_FRAME_SLOT.with(|c| c.set(core::ptr::null())); + } }); } @@ -873,7 +939,10 @@ pub fn reinit_frame_slot_after_fork(vm: &VirtualMachine) { drop(registry); CURRENT_THREAD_SLOT.with(|s| { - *s.borrow_mut() = Some(new_slot); + *s.borrow_mut() = Some(new_slot.clone()); + }); + INTERP_THREAD_SLOTS.with(|slots| { + slots.borrow_mut().insert(vm.state.interpreter_id, new_slot); }); } From 43f4b23314e703ace272352f26326f2db97414c0 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Thu, 13 Aug 2026 23:58:42 +0900 Subject: [PATCH 02/20] gc: stop every interpreter while collecting The generation lists are process-global, so a collection reads and frees objects owned by every interpreter. CollectStopTheWorld stopped only the collecting interpreter, leaving other interpreters' threads free to mutate the same object graph during the reference-subtraction, reachability and snapshot phases. Stop all live interpreters instead, in runtime id order, and restart them in reverse. The global `collecting` mutex serializes collectors process-wide, so no second collector takes these exclusions in another order; fork acquires a single interpreter's exclusion, so the orders cannot cycle. - runtime: add live_interpreter_states(), ordered by interpreter id. - StopTheWorldState methods take &PyGlobalState instead of &VirtualMachine, so an interpreter's world can be stopped without a VM for it; update the call sites in frame, _thread, posix, faulthandler and capi. - Document in gc_state() that the collector is process-wide: gc.disable(), thresholds, gc.garbage and gc.get_objects() observe process-wide state, and a per-interpreter collector additionally needs untrack_object (called from default_dealloc with no VM in scope) routed to the owning interpreter. Tests: stop_the_world_parks_threads_of_another_interpreter asserts a thread entered in one interpreter parks another interpreter's threads (it fails without this change), plus a collect-while-another-interpreter-churns test. Assisted-by: Claude Code:claude-opus-4-8 --- crates/capi/src/pystate.rs | 4 +- crates/stdlib/src/faulthandler.rs | 4 +- crates/vm/src/builtins/frame.rs | 4 +- crates/vm/src/gc_state.rs | 70 ++++++++++----- crates/vm/src/stdlib/_thread.rs | 8 +- crates/vm/src/stdlib/posix.rs | 4 +- crates/vm/src/vm/interpreter.rs | 140 ++++++++++++++++++++++++++++++ crates/vm/src/vm/mod.rs | 44 +++++----- crates/vm/src/vm/runtime.rs | 18 ++++ 9 files changed, 239 insertions(+), 57 deletions(-) diff --git a/crates/capi/src/pystate.rs b/crates/capi/src/pystate.rs index 865a116443b..cec3bc240b1 100644 --- a/crates/capi/src/pystate.rs +++ b/crates/capi/src/pystate.rs @@ -109,8 +109,8 @@ mod tests { current_vm_is_set(), "This thread did not have a vm attached" ); - vm.state.stop_the_world.stop_the_world(vm); - vm.state.stop_the_world.start_the_world(vm); + vm.state.stop_the_world.stop_the_world(&vm.state); + vm.state.stop_the_world.start_the_world(&vm.state); }); }); }); diff --git a/crates/stdlib/src/faulthandler.rs b/crates/stdlib/src/faulthandler.rs index 6edd023f1eb..3fbb8391bec 100644 --- a/crates/stdlib/src/faulthandler.rs +++ b/crates/stdlib/src/faulthandler.rs @@ -260,8 +260,8 @@ mod decl { use core::sync::atomic::Ordering; let current_tid = rustpython_vm::stdlib::_thread::get_ident(); { - vm.state.stop_the_world.stop_the_world(vm); - scopeguard::defer! { vm.state.stop_the_world.start_the_world(vm); } + vm.state.stop_the_world.stop_the_world(&vm.state); + scopeguard::defer! { vm.state.stop_the_world.start_the_world(&vm.state); } let registry = vm.state.thread_frames.lock(); #[expect( clippy::iter_over_hash_type, diff --git a/crates/vm/src/builtins/frame.rs b/crates/vm/src/builtins/frame.rs index 94ec827b7a6..d5d2b608553 100644 --- a/crates/vm/src/builtins/frame.rs +++ b/crates/vm/src/builtins/frame.rs @@ -897,8 +897,8 @@ impl Py { { // Enter STW before dereferencing `prev` — the owning thread may // return and free the stack-allocated iframe at any time. - vm.state.stop_the_world.stop_the_world(vm); - scopeguard::defer! { vm.state.stop_the_world.start_the_world(vm); } + vm.state.stop_the_world.stop_the_world(&vm.state); + scopeguard::defer! { vm.state.stop_the_world.start_the_world(&vm.state); } let prev_ref = unsafe { &*prev }; // Fast path: already materialized. if let Some(fo) = prev_ref.frame_obj() { diff --git a/crates/vm/src/gc_state.rs b/crates/vm/src/gc_state.rs index 9744d4ae992..6bde02586b1 100644 --- a/crates/vm/src/gc_state.rs +++ b/crates/vm/src/gc_state.rs @@ -146,42 +146,58 @@ struct GcPtr(NonNull); /// well-defined while all other threads are parked at a safepoint. Restarting /// happens explicitly once the snapshot has pinned every object; `Drop` is a /// backstop that also restarts on the early-return paths. +/// +/// The generation lists are process-global, so a collection walks objects +/// owned by *every* interpreter. Stopping only the collecting interpreter +/// would leave another interpreter's threads mutating the same object graph, +/// so every live interpreter is stopped. Stopping in `runtime` id order keeps +/// exclusion acquisition ordered; the `collecting` mutex additionally +/// serializes collections process-wide, so no second collector can take these +/// exclusions in another order. #[cfg(feature = "threading")] struct CollectStopTheWorld { - vm: *const crate::VirtualMachine, - stopped: bool, + /// Stopped interpreter states, in stop order. Held as strong references so + /// an interpreter cannot be dropped between stop and restart. + stopped: Vec>, } #[cfg(feature = "threading")] impl CollectStopTheWorld { - /// Request stop-the-world when the current thread has an attached VM. - /// Falls back to no barrier when no VM is attached (the tracked-object - /// reads then run without other threads only if the caller guarantees it). + /// Request stop-the-world on every live interpreter when the current thread + /// has an attached VM. Falls back to no barrier when no VM is attached (the + /// tracked-object reads then run without other threads only if the caller + /// guarantees it). fn new() -> Self { - let vm = crate::vm::thread::try_with_current_vm(|vm| { - vm.state.stop_the_world.stop_the_world(vm); - vm as *const crate::VirtualMachine - }); - match vm { - Some(vm) => Self { vm, stopped: true }, - None => Self { - vm: core::ptr::null(), - stopped: false, - }, + // No attached VM means no interpreter is running Python on this thread; + // keep the historical no-barrier fallback. + if !crate::vm::thread::current_vm_is_set() { + return Self { + stopped: Vec::new(), + }; + } + + let states = crate::vm::runtime::live_interpreter_states(); + let mut stopped = Vec::with_capacity(states.len()); + for state in states { + state.stop_the_world.stop_the_world(&state); + stopped.push(state); } + Self { stopped } } /// Restart the world. Idempotent. fn restart(&mut self) { - if self.stopped { - // SAFETY: the current thread stays attached to this VM for the - // whole collection — the VM is never popped from the thread's VM - // stack while collecting — so the pointer is valid here. - let vm = unsafe { &*self.vm }; - vm.state.stop_the_world.start_the_world(vm); - self.stopped = false; + // Reverse of the stop order. + for state in self.stopped.drain(..).rev() { + state.stop_the_world.start_the_world(&state); } } + + /// Whether this collection actually stopped the world. + #[cfg(all(unix, debug_assertions))] + fn is_stopped(&self) -> bool { + !self.stopped.is_empty() + } } #[cfg(feature = "threading")] @@ -638,7 +654,7 @@ impl GcState { // because stack-allocated frames update only CURRENT_FRAME (via // set_current_frame_nosave), not top_frame. #[cfg(all(unix, feature = "threading", debug_assertions))] - if stw.stopped { + if stw.is_stopped() { let unreachable_set: HashSet = unreachable.iter().copied().collect(); let mut cur = crate::vm::thread::get_current_frame(); while !cur.is_null() { @@ -1091,6 +1107,14 @@ impl GcState { /// In threading mode this is a true global (OnceLock). /// In non-threading mode this is thread-local, because PyRwLock/PyMutex /// use Cell-based locks that are not Sync. +/// +/// The collector is process-wide rather than per-interpreter: every +/// interpreter's tracked objects live in these generation lists, so a +/// collection stops and collects across all of them, and `gc.disable()`, +/// thresholds, `gc.garbage` and `gc.get_objects()` observe process-wide state. +/// Making the collector per-interpreter additionally requires routing +/// `untrack_object` (called from `default_dealloc`, where no VM is in scope) to +/// the owning interpreter's lists. pub fn gc_state() -> &'static GcState { rustpython_common::static_cell! { static GC_STATE: GcState; diff --git a/crates/vm/src/stdlib/_thread.rs b/crates/vm/src/stdlib/_thread.rs index fe753cd78b7..09866acd224 100644 --- a/crates/vm/src/stdlib/_thread.rs +++ b/crates/vm/src/stdlib/_thread.rs @@ -1193,8 +1193,8 @@ pub(crate) mod _thread { { use core::sync::atomic::Ordering; let current_ident = get_ident(); - vm.state.stop_the_world.stop_the_world(vm); - scopeguard::defer! { vm.state.stop_the_world.start_the_world(vm); } + vm.state.stop_the_world.stop_the_world(&vm.state); + scopeguard::defer! { vm.state.stop_the_world.start_the_world(&vm.state); } let registry = vm.state.thread_frames.lock(); registry .iter() @@ -1249,8 +1249,8 @@ pub(crate) mod _thread { { use core::sync::atomic::Ordering; let current_ident = get_ident(); - vm.state.stop_the_world.stop_the_world(vm); - scopeguard::defer! { vm.state.stop_the_world.start_the_world(vm); } + vm.state.stop_the_world.stop_the_world(&vm.state); + scopeguard::defer! { vm.state.stop_the_world.start_the_world(&vm.state); } let registry = vm.state.thread_frames.lock(); registry .iter() diff --git a/crates/vm/src/stdlib/posix.rs b/crates/vm/src/stdlib/posix.rs index 6b91950e907..e78f1ec6e40 100644 --- a/crates/vm/src/stdlib/posix.rs +++ b/crates/vm/src/stdlib/posix.rs @@ -626,7 +626,7 @@ pub mod module { crate::stdlib::_imp::acquire_imp_lock_for_fork(vm); #[cfg(feature = "threading")] - vm.state.stop_the_world.stop_the_world(vm); + vm.state.stop_the_world.stop_the_world(&vm.state); } fn py_os_after_fork_child(vm: &VirtualMachine) { @@ -729,7 +729,7 @@ pub mod module { fn py_os_after_fork_parent(vm: &VirtualMachine) { #[cfg(feature = "threading")] - vm.state.stop_the_world.start_the_world(vm); + vm.state.stop_the_world.start_the_world(&vm.state); #[cfg(feature = "threading")] crate::stdlib::_imp::release_imp_lock_after_fork_parent(); diff --git a/crates/vm/src/vm/interpreter.rs b/crates/vm/src/vm/interpreter.rs index 5d76b57cb0b..4a1e5e4b9d0 100644 --- a/crates/vm/src/vm/interpreter.rs +++ b/crates/vm/src/vm/interpreter.rs @@ -1222,6 +1222,146 @@ mod tests { assert!(!sub.is_main()); } + /// A collection must stop every interpreter, not just the collecting one: + /// the generation lists are process-global, so the reachability walk reads + /// objects owned by other interpreters while their threads would otherwise + /// still be mutating them. + #[cfg(all(feature = "threading", feature = "rustpython-compiler"))] + #[test] + fn gc_collect_is_safe_while_another_interpreter_runs() { + use crate::compiler::Mode; + use alloc::sync::Arc; + use core::{ + sync::atomic::{AtomicBool, Ordering}, + time::Duration, + }; + use std::time::Instant; + + // Each interpreter churns reference cycles so both contribute tracked + // objects to the shared generation lists. + const CHURN: &str = "\ +for _ in range(40): + a = {} + b = {'peer': a} + a['peer'] = b +"; + + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + let stop = Arc::new(AtomicBool::new(false)); + + let run_source = |vm: &VirtualMachine, source: &str| { + let scope = vm.new_scope_with_builtins(); + let code = vm + .compile(source, Mode::Exec, "") + .map_err(|err| err.into_pyexception(vm, Some(source))) + .unwrap(); + vm.run_code_obj(code, scope).unwrap(); + }; + + // Subinterpreter thread: allocate cycles continuously. + let stop_worker = Arc::clone(&stop); + let churner = sub.enter(|vm| { + let thread_vm = vm.new_thread(); + std::thread::spawn(move || { + thread_vm.run(|vm| { + while !stop_worker.load(Ordering::Acquire) { + run_source(vm, CHURN); + } + }); + }) + }); + + // Main interpreter: force collections while the sub keeps mutating. + main.enter(|vm| { + run_source(vm, CHURN); + let deadline = Instant::now() + Duration::from_secs(2); + let mut collections = 0; + while Instant::now() < deadline && collections < 20 { + crate::gc_state::gc_state().collect_force(2); + collections += 1; + } + assert!(collections > 0); + }); + + stop.store(true, Ordering::Release); + churner.join().expect("churn worker panicked"); + } + + /// A thread entered in one interpreter can park another interpreter's + /// threads. This is what makes a collection safe: the generation lists are + /// process-global, so the collector must be able to stop every interpreter, + /// not only its own. + #[cfg(all(feature = "threading", feature = "rustpython-compiler"))] + #[test] + fn stop_the_world_parks_threads_of_another_interpreter() { + use crate::compiler::Mode; + use alloc::sync::Arc; + use core::{ + sync::atomic::{AtomicBool, AtomicU64, Ordering}, + time::Duration, + }; + + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + let sub_state = sub.enter(|vm| vm.state.clone()); + + let progress = Arc::new(AtomicU64::new(0)); + let stop = Arc::new(AtomicBool::new(false)); + + // Sub-interpreter worker: runs bytecode (so it reaches safepoints) and + // reports progress every iteration. + let progress_worker = Arc::clone(&progress); + let stop_worker = Arc::clone(&stop); + let worker = sub.enter(|vm| { + let thread_vm = vm.new_thread(); + std::thread::spawn(move || { + thread_vm.run(|vm| { + let source = "x = 1 + 1\n"; + let code = vm + .compile(source, Mode::Exec, "") + .map_err(|err| err.into_pyexception(vm, Some(source))) + .unwrap(); + while !stop_worker.load(Ordering::Acquire) { + let scope = vm.new_scope_with_builtins(); + vm.run_code_obj(code.clone(), scope).unwrap(); + progress_worker.fetch_add(1, Ordering::Release); + } + }); + }) + }); + + // Wait until the worker is actually running. + while progress.load(Ordering::Acquire) == 0 { + std::thread::yield_now(); + } + + main.enter(|_vm| { + // Stop the *subinterpreter* from a thread whose current interpreter + // is main — the cross-interpreter stop a collection performs. + sub_state.stop_the_world.stop_the_world(&sub_state); + + let parked_at = progress.load(Ordering::Acquire); + std::thread::sleep(Duration::from_millis(50)); + assert_eq!( + progress.load(Ordering::Acquire), + parked_at, + "subinterpreter thread kept running while its world was stopped" + ); + + sub_state.stop_the_world.start_the_world(&sub_state); + }); + + // After restart the worker makes progress again. + let resumed_from = progress.load(Ordering::Acquire); + while progress.load(Ordering::Acquire) == resumed_from { + std::thread::yield_now(); + } + + stop.store(true, Ordering::Release); + worker.join().expect("worker panicked"); + } + /// The process main id is recorded once and is stable across later creates. #[test] fn process_main_id_recorded_and_stable() { diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index df71227b3a8..84a1efb4568 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -264,9 +264,9 @@ impl StopTheWorldState { } #[inline] - fn init_thread_countdown(&self, vm: &VirtualMachine) -> i64 { + fn init_thread_countdown(&self, state: &PyGlobalState) -> i64 { let requester = self.requester.load(Ordering::Relaxed); - let registry = vm.state.thread_frames.lock(); + let registry = state.thread_frames.lock(); // Keep requested/count initialization serialized with thread-slot // registration (which also takes this lock), matching the // HEAD_LOCK-guarded stop-the-world bookkeeping. @@ -295,10 +295,10 @@ impl StopTheWorldState { /// Try to CAS detached threads directly to SUSPENDED and check whether /// stop countdown reached zero after parking detached threads. - fn park_detached_threads(&self, vm: &VirtualMachine) -> bool { + fn park_detached_threads(&self, state: &PyGlobalState) -> bool { use thread::{THREAD_ATTACHED, THREAD_DETACHED, THREAD_SUSPENDED}; let requester = self.requester.load(Ordering::Relaxed); - let registry = vm.state.thread_frames.lock(); + let registry = state.thread_frames.lock(); let mut attached_seen = 0u64; let mut forced_parks = 0u64; @@ -420,23 +420,23 @@ impl StopTheWorldState { /// Takes the shared exclusion first so at most one requester (fork or GC) /// drives the stop→start span at a time; it is released by /// `start_the_world`/`reset_after_fork`. - pub fn stop_the_world(&self, vm: &VirtualMachine) { + pub fn stop_the_world(&self, state: &PyGlobalState) { self.acquire_exclusion(); let start = std::time::Instant::now(); let requester_ident = crate::stdlib::_thread::get_ident(); self.requester.store(requester_ident, Ordering::Relaxed); self.stats_stop_calls.fetch_add(1, Ordering::Relaxed); - let initial_countdown = self.init_thread_countdown(vm); + let initial_countdown = self.init_thread_countdown(state); stw_trace(format_args!("stop begin requester={requester_ident}")); // Park detached threads and set stop bits, then confirm every other // thread is SUSPENDED. The completion condition is level-triggered // (`all_non_requester_suspended`) so an already-suspended thread that // was counted but will not notify again cannot stall the stop. - self.park_detached_threads(vm); - if initial_countdown == 0 || self.all_non_requester_suspended(vm) { + self.park_detached_threads(state); + if initial_countdown == 0 || self.all_non_requester_suspended(state) { self.world_stopped.store(true, Ordering::Release); #[cfg(debug_assertions)] - self.debug_assert_all_non_requester_suspended(vm); + self.debug_assert_all_non_requester_suspended(state); stw_trace(format_args!( "stop end requester={requester_ident} wait_ns=0 polls=0" )); @@ -445,8 +445,8 @@ impl StopTheWorldState { let mut polls = 0u64; loop { - self.park_detached_threads(vm); - if self.all_non_requester_suspended(vm) { + self.park_detached_threads(state); + if self.all_non_requester_suspended(state) { break; } polls = polls.saturating_add(1); @@ -454,7 +454,7 @@ impl StopTheWorldState { // Re-check under the wait mutex first to avoid a lost-wake race: // a thread may have suspended and notified right before we enter wait. let guard = self.notify_mutex.lock().unwrap(); - if self.all_non_requester_suspended(vm) { + if self.all_non_requester_suspended(state) { drop(guard); break; } @@ -483,18 +483,18 @@ impl StopTheWorldState { } self.world_stopped.store(true, Ordering::Release); #[cfg(debug_assertions)] - self.debug_assert_all_non_requester_suspended(vm); + self.debug_assert_all_non_requester_suspended(state); stw_trace(format_args!( "stop end requester={requester_ident} wait_ns={wait_ns} polls={polls}" )); } /// Resume all suspended threads (`start_the_world`). - pub fn start_the_world(&self, vm: &VirtualMachine) { + pub fn start_the_world(&self, state: &PyGlobalState) { use thread::{THREAD_DETACHED, THREAD_SUSPENDED}; let requester = self.requester.load(Ordering::Relaxed); stw_trace(format_args!("start begin requester={requester}")); - let registry = vm.state.thread_frames.lock(); + let registry = state.thread_frames.lock(); // Clear the request flag BEFORE waking threads. Otherwise a thread // returning from allow_threads → attach_thread could observe // `requested == true`, re-suspend itself, and stay parked forever. @@ -528,7 +528,7 @@ impl StopTheWorldState { self.thread_countdown.store(0, Ordering::Release); self.requester.store(0, Ordering::Relaxed); #[cfg(debug_assertions)] - self.debug_assert_all_non_requester_detached(vm); + self.debug_assert_all_non_requester_detached(state); // Release the exclusion last, ending the stop→start span so the next // requester (fork or GC) can proceed. self.release_exclusion(); @@ -611,10 +611,10 @@ impl StopTheWorldState { /// lost-decrement race under rapid back-to-back stops: a thread that is /// already SUSPENDED when a new stop counts it neither notifies nor is /// force-parked again, so an edge-based countdown could never reach zero. - fn all_non_requester_suspended(&self, vm: &VirtualMachine) -> bool { + fn all_non_requester_suspended(&self, state: &PyGlobalState) -> bool { use thread::THREAD_SUSPENDED; let requester = self.requester.load(Ordering::Relaxed); - let registry = vm.state.thread_frames.lock(); + let registry = state.thread_frames.lock(); #[expect( clippy::iter_over_hash_type, @@ -632,10 +632,10 @@ impl StopTheWorldState { } #[cfg(debug_assertions)] - fn debug_assert_all_non_requester_suspended(&self, vm: &VirtualMachine) { + fn debug_assert_all_non_requester_suspended(&self, state: &PyGlobalState) { use thread::THREAD_SUSPENDED; let requester = self.requester.load(Ordering::Relaxed); - let registry = vm.state.thread_frames.lock(); + let registry = state.thread_frames.lock(); #[expect( clippy::iter_over_hash_type, @@ -655,10 +655,10 @@ impl StopTheWorldState { } #[cfg(debug_assertions)] - fn debug_assert_all_non_requester_detached(&self, vm: &VirtualMachine) { + fn debug_assert_all_non_requester_detached(&self, state: &PyGlobalState) { use thread::THREAD_SUSPENDED; let requester = self.requester.load(Ordering::Relaxed); - let registry = vm.state.thread_frames.lock(); + let registry = state.thread_frames.lock(); #[expect( clippy::iter_over_hash_type, diff --git a/crates/vm/src/vm/runtime.rs b/crates/vm/src/vm/runtime.rs index 180a1b9ce99..5072f21574c 100644 --- a/crates/vm/src/vm/runtime.rs +++ b/crates/vm/src/vm/runtime.rs @@ -188,6 +188,24 @@ pub fn interpreter_count() -> usize { list_interpreters().len() } +/// All live interpreter states, ordered by id. +/// +/// Used by the cyclic collector, which must stop every interpreter's threads +/// (not just the collecting one) because GC-tracked objects from all +/// interpreters share one object graph. Ordering is deterministic so that +/// multiple stop-the-world requesters always take exclusions in the same order. +#[must_use] +pub fn live_interpreter_states() -> Vec> { + let entries = registry().entries.lock(); + let mut states: Vec<(i64, PyRc)> = entries + .iter() + .filter_map(|(&id, entry)| entry.state.upgrade().map(|state| (id, state))) + .collect(); + drop(entries); + states.sort_by_key(|(id, _)| *id); + states.into_iter().map(|(_, state)| state).collect() +} + /// Runtime-owned interpreters (the ownership anchor for the Python /// `_interpreters` API). /// From ffd3d1e4255ba91704188198e603eadce25708b4 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 14 Aug 2026 03:18:37 +0900 Subject: [PATCH 03/20] vm: scope the interpreter registry per thread without threading The registry held `rc::Weak` in a process-global `OnceLock` and covered the resulting `!Send`/`!Sync` with an `unsafe impl` justifying it as "non-threading builds are single-threaded". That is not this codebase's model: `static_cell!` is thread-local without the `threading` feature precisely so each OS thread can own its own `Context::genesis()` and `GcState`, so two threads could reach the same `Rc` counts through the registry. Use `static_cell!` for the registry as well, matching `gc_state()`, and drop the `unsafe impl`. Ids are consequently unique per registry rather than per process in non-threading builds, which is documented on `alloc_interpreter_id`. Move the recorded main interpreter id into the registry so it follows the same scoping instead of living in a separate global `OnceLock`. Also make `CollectStopTheWorld::new` accumulate into a live guard: it built a bare `Vec` and only moved it into the restarting `Drop` type after stopping every interpreter, so an unwind partway through the loop left the already stopped interpreters parked forever with their exclusion held. Assisted-by: Claude Code:claude-opus-4-8 --- crates/vm/src/gc_state.rs | 15 +++++--- crates/vm/src/vm/runtime.rs | 69 +++++++++++++++++++++---------------- 2 files changed, 50 insertions(+), 34 deletions(-) diff --git a/crates/vm/src/gc_state.rs b/crates/vm/src/gc_state.rs index 6bde02586b1..beb854063fd 100644 --- a/crates/vm/src/gc_state.rs +++ b/crates/vm/src/gc_state.rs @@ -176,13 +176,18 @@ impl CollectStopTheWorld { }; } - let states = crate::vm::runtime::live_interpreter_states(); - let mut stopped = Vec::with_capacity(states.len()); - for state in states { + // Accumulate into a live `Self` rather than a bare Vec: if a later + // `stop_the_world` unwinds, dropping this guard restarts the + // interpreters already stopped, instead of leaving their threads parked + // and their exclusion held forever. + let mut guard = Self { + stopped: Vec::new(), + }; + for state in crate::vm::runtime::live_interpreter_states() { state.stop_the_world.stop_the_world(&state); - stopped.push(state); + guard.stopped.push(state); } - Self { stopped } + guard } /// Restart the world. Idempotent. diff --git a/crates/vm/src/vm/runtime.rs b/crates/vm/src/vm/runtime.rs index 5072f21574c..17245fffca3 100644 --- a/crates/vm/src/vm/runtime.rs +++ b/crates/vm/src/vm/runtime.rs @@ -14,7 +14,6 @@ use crate::vm::PyGlobalState; use core::sync::atomic::{AtomicI64, Ordering}; use parking_lot::Mutex; use std::collections::HashMap; -use std::sync::OnceLock; /// Where an interpreter state came from (mirrors CPython `_PyInterpreterState_GetWhence`). #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] @@ -58,36 +57,41 @@ struct RegistryEntry { state: alloc::rc::Weak, } +/// `main_id` value before any main interpreter has been registered. +const NO_MAIN_INTERPRETER: i64 = -1; + struct InterpreterRegistry { next_id: AtomicI64, + /// Id of the first registered `is_main` interpreter (PEP 734 `get_main()`), + /// or [`NO_MAIN_INTERPRETER`]. + main_id: AtomicI64, /// id → entry. Main interpreter is always id 0 when created first. entries: Mutex>, } -// Without the `threading` feature `RegistryEntry` holds `rc::Weak`, which is -// `!Send`/`!Sync`, so the process-global registry static would not type-check. -// SAFETY: non-threading builds are single-threaded by construction (`Rc`-based -// objects are already unsound to touch across threads), and this process-global -// registry is only ever reached from that one thread. The `parking_lot::Mutex` -// still guards the map contents against reentrancy. -#[cfg(not(feature = "threading"))] -unsafe impl Send for InterpreterRegistry {} -#[cfg(not(feature = "threading"))] -unsafe impl Sync for InterpreterRegistry {} - impl InterpreterRegistry { fn new() -> Self { Self { // Monotonic ids starting at 0. Concurrent Interpreter construction // (e.g. cargo test threads) must never share an id. next_id: AtomicI64::new(0), + main_id: AtomicI64::new(NO_MAIN_INTERPRETER), entries: Mutex::new(HashMap::new()), } } } +/// The interpreter registry. +/// +/// With `threading` this is one process-global table. Without it, `PyRc` is +/// `Rc` and each OS thread owns an independent `Context::genesis()` and +/// `GcState`, so the registry is thread-local for the same reason `gc_state()` +/// is: an `Rc` handle must never be reachable from another thread. +/// `static_cell!` provides exactly that split. fn registry() -> &'static InterpreterRegistry { - static REGISTRY: OnceLock = OnceLock::new(); + rustpython_common::static_cell! { + static REGISTRY: InterpreterRegistry; + } REGISTRY.get_or_init(InterpreterRegistry::new) } @@ -105,39 +109,45 @@ pub const MAIN_INTERPRETER_ID: i64 = 0; /// in the commit that lands `_interpreters`. pub const SUPPORTS_ISOLATED_INTERPRETERS: bool = false; -/// Process main interpreter id for PEP 734 `_interpreters.get_main()`, recorded -/// once when the first `is_main` interpreter is registered. -static MAIN_INTERPRETER: OnceLock = OnceLock::new(); - -/// Id of the process main interpreter (PEP 734 `get_main()`), or `None` before -/// any interpreter has been created. +/// Id of the main interpreter (PEP 734 `get_main()`), or `None` before any +/// interpreter has been created. /// /// This is distinct from [`PyGlobalState::is_main`]: every top-level (non-sub) /// interpreter carries `is_main` for its own signal / main-thread bookkeeping, -/// but only the first one registered becomes *the* process main. +/// but only the first one registered becomes *the* main. #[must_use] pub fn main_interpreter_id() -> Option { - MAIN_INTERPRETER.get().copied() + match registry().main_id.load(Ordering::Acquire) { + NO_MAIN_INTERPRETER => None, + id => Some(id), + } } -/// Allocate a unique process-global interpreter id. +/// Allocate a unique interpreter id. /// -/// Ids are strictly monotonic and never reused for the lifetime of the process. -/// This is required for thread-safe concurrent `Interpreter` construction -/// (parallel unit tests, multi-threaded embedding). +/// Ids are strictly monotonic and never reused for the lifetime of the +/// registry, so concurrent `Interpreter` construction (parallel unit tests, +/// multi-threaded embedding) never shares an id. Without `threading` the +/// registry — like `Context::genesis()` and the GC state — is per OS thread, so +/// ids are unique within a thread rather than across the process. pub(crate) fn alloc_interpreter_id() -> i64 { registry().next_id.fetch_add(1, Ordering::Relaxed) } -/// Register an interpreter state in the process-global table. +/// Register an interpreter state in the registry. pub(crate) fn register_interpreter(state: &PyRc) { let id = state.interpreter_id; let whence = state.whence; if state.is_main { - // First `is_main` interpreter defines the process main for `get_main()`. + // First `is_main` interpreter defines the main for `get_main()`. // Additional top-level Interpreters (embedding) keep their own `is_main` - // flag but do not displace the recorded process main. - let _ = MAIN_INTERPRETER.set(id); + // flag but do not displace the recorded main. + let _ = registry().main_id.compare_exchange( + NO_MAIN_INTERPRETER, + id, + Ordering::AcqRel, + Ordering::Relaxed, + ); } let mut entries = registry().entries.lock(); entries.insert( @@ -220,6 +230,7 @@ pub fn live_interpreter_states() -> Vec> { /// only when `PyObjectRef` is `Arc`-backed). #[cfg(feature = "threading")] fn owned_interpreters() -> &'static Mutex> { + use std::sync::OnceLock; static OWNED: OnceLock>> = OnceLock::new(); OWNED.get_or_init(|| Mutex::new(HashMap::new())) } From 007999421df736597e834a757d617ddb1bcb00be Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 14 Aug 2026 03:49:27 +0900 Subject: [PATCH 04/20] vm: keep the interpreter registry usable across bootstrap, drop and fork MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three gaps where the registry did not describe the interpreters that actually exist, each of which hides an interpreter from the collector's stop-the-world. Register before `initialize()`. Registration ran as the last step of `initialize_vm`, so the whole bootstrap — which executes Python bytecode and allocates GC-tracked objects — was invisible to `live_interpreter_states()`. It still cannot run any earlier than this: the init hooks take `PyRc::get_mut` on the state, which fails as soon as the registry holds a weak reference to it. Stop unregistering in `Interpreter::drop`. The handle does not decide the interpreter's lifetime — every `ThreadedVirtualMachine` from `new_thread()` holds its own `PyRc` — so an interpreter with running workers disappeared from the registry while its threads kept mutating the object graph. The entries are weak, so lifetime is already correct without the removal; dead entries are now reaped when registering instead. Interpreters are consequently released rather than unregistered at a fixed point, so the two tests asserting disappearance now wait for it: a collection in progress legitimately holds a reference to every live interpreter. Repair other interpreters after fork. `py_os_after_fork_child` only fixed the forking interpreter, leaving every other one with slots for threads that did not survive (still ATTACHED if they were running bytecode) plus locks and stop-the-world flags held by them. Since a collection stops all interpreters, the child's first collection would wait for threads that no longer exist. Reset their locks, stop-the-world state and thread tables, drop this thread's cached slots for them, and reinit the registry's own locks first, since enumerating interpreters now takes them. Tests: test_gc, test_threading and test_fork1 pass, as do the vm tests in both the threading and default configurations. Assisted-by: Claude Code:claude-opus-4-8 --- crates/vm/src/stdlib/posix.rs | 60 +++++++++++++++++++++++++++++++++ crates/vm/src/vm/interpreter.rs | 42 +++++++++++++++++------ crates/vm/src/vm/runtime.rs | 27 ++++++++++++--- crates/vm/src/vm/thread.rs | 13 +++++++ 4 files changed, 126 insertions(+), 16 deletions(-) diff --git a/crates/vm/src/stdlib/posix.rs b/crates/vm/src/stdlib/posix.rs index e78f1ec6e40..aaea11acca2 100644 --- a/crates/vm/src/stdlib/posix.rs +++ b/crates/vm/src/stdlib/posix.rs @@ -630,6 +630,13 @@ pub mod module { } fn py_os_after_fork_child(vm: &VirtualMachine) { + // The interpreter registry is reachable from every thread, so repair it + // before anything enumerates interpreters. + #[cfg(all(unix, feature = "threading"))] + unsafe { + crate::vm::runtime::reinit_after_fork() + }; + #[cfg(feature = "threading")] vm.state.stop_the_world.reset_after_fork(); @@ -639,6 +646,12 @@ pub mod module { #[cfg(feature = "threading")] reinit_locks_after_fork(vm); + // The collector stops every interpreter, so interpreters other than the + // forking one must be repaired too; otherwise the child's first + // collection waits for threads that did not survive the fork. + #[cfg(all(unix, feature = "threading"))] + reinit_other_interpreters_after_fork(vm); + // Reinit per-object IO buffer locks on std streams. // BufferedReader/Writer/TextIOWrapper use PyThreadMutex which can be // held by dead parent threads, causing deadlocks on any IO in the child. @@ -727,6 +740,53 @@ pub mod module { } } + /// Repair every live interpreter other than the forking one after `fork()`. + /// + /// Only the forking thread survives, so each other interpreter is left with + /// slots for threads that no longer exist (still ATTACHED if they were + /// running bytecode) and possibly locks or stop-the-world flags held by + /// them. Since a collection stops all interpreters, that state would hang + /// the child's first collection. + /// + /// # Safety + /// Must only be called after `fork()` in the child, when no other threads exist. + #[cfg(all(unix, feature = "threading"))] + fn reinit_other_interpreters_after_fork(vm: &VirtualMachine) { + use rustpython_common::lock::reinit_mutex_after_fork; + + for state in crate::vm::runtime::live_interpreter_states() { + if state.interpreter_id == vm.state.interpreter_id { + continue; + } + + unsafe { + reinit_mutex_after_fork(&state.before_forkers); + reinit_mutex_after_fork(&state.after_forkers_child); + reinit_mutex_after_fork(&state.after_forkers_parent); + reinit_mutex_after_fork(&state.atexit_funcs); + reinit_mutex_after_fork(&state.global_trace_func); + reinit_mutex_after_fork(&state.global_profile_func); + reinit_mutex_after_fork(&state.type_mutex); + reinit_mutex_after_fork(&state.monitoring); + reinit_mutex_after_fork(&state.thread_frames); + reinit_mutex_after_fork(&state.thread_handles); + reinit_mutex_after_fork(&state.shutdown_handles); + + state.codec_registry.reinit_after_fork(); + } + + state.stop_the_world.reset_after_fork(); + + // Every thread registered here belongs to the parent, including any + // slot the forking thread itself registered before the fork. + state.thread_frames.lock().clear(); + state.thread_handles.lock().clear(); + state.shutdown_handles.lock().clear(); + } + + crate::vm::thread::purge_other_interpreter_slots_after_fork(vm.state.interpreter_id); + } + fn py_os_after_fork_parent(vm: &VirtualMachine) { #[cfg(feature = "threading")] vm.state.stop_the_world.start_the_world(&vm.state); diff --git a/crates/vm/src/vm/interpreter.rs b/crates/vm/src/vm/interpreter.rs index 4a1e5e4b9d0..0ad9b89d364 100644 --- a/crates/vm/src/vm/interpreter.rs +++ b/crates/vm/src/vm/interpreter.rs @@ -200,6 +200,13 @@ where // Call custom init function (can mutate vm.state) init(&mut vm); + // Register before `initialize()` runs any Python: it allocates GC-tracked + // objects, so a collection on another thread has to be able to stop this + // interpreter while that happens. It cannot be registered earlier — the + // hooks above take `PyRc::get_mut` on the state, which fails once the + // registry holds a weak reference to it. + runtime::register_interpreter(&vm.state); + // `initialize()` runs Python bytecode directly (e.g. importing `codecs` // and `encodings`) before any `enter_vm` scope exists, so attach this // thread for the duration so type cache reads see it as ATTACHED. @@ -209,7 +216,6 @@ where // Clone global_state for Interpreter after all initialization is done let global_state = vm.state.clone(); - runtime::register_interpreter(&global_state); (vm, global_state) } @@ -380,12 +386,6 @@ pub struct Interpreter { vm: VirtualMachine, } -impl Drop for Interpreter { - fn drop(&mut self) { - runtime::unregister_interpreter(self.global_state.interpreter_id); - } -} - impl Interpreter { /// Create a new interpreter configuration builder. /// @@ -812,7 +812,27 @@ mod tests { assert!(ids.contains(&sub2.id())); } - /// Dropping a subinterpreter unregisters it; main remains. + /// An interpreter stays looked-up-able until nothing holds its state. + /// + /// Dropping the handle is not the end of its life: `new_thread()` workers + /// hold their own reference, and a collection in progress holds one for + /// every live interpreter while the world is stopped. So the registry entry + /// goes away eventually rather than at the drop. + fn wait_until_unregistered(id: i64) { + use core::time::Duration; + use std::time::Instant; + + let deadline = Instant::now() + Duration::from_secs(5); + while runtime::lookup_interpreter(id).is_some() { + assert!( + Instant::now() < deadline, + "interpreter {id} still registered long after its last reference" + ); + std::thread::yield_now(); + } + } + + /// Dropping a subinterpreter releases it; main remains. #[test] fn drop_subinterpreter_unregisters() { let main = Interpreter::without_stdlib(Default::default()); @@ -822,7 +842,7 @@ mod tests { assert!(runtime::lookup_interpreter(id).is_some()); id }; - assert!(runtime::lookup_interpreter(sub_id).is_none()); + wait_until_unregistered(sub_id); assert!(runtime::lookup_interpreter(main.id()).is_some()); } @@ -1203,9 +1223,9 @@ mod tests { assert!(runtime::lookup_interpreter(id).is_some()); assert!(runtime::take_owned_interpreter(id).is_none()); - // Dropping the reclaimed handle unregisters it. + // Dropping the reclaimed handle releases it. drop(reclaimed); - assert!(runtime::lookup_interpreter(id).is_none()); + wait_until_unregistered(id); } /// `create_owned_subinterpreter` stores the sub and returns only its id. diff --git a/crates/vm/src/vm/runtime.rs b/crates/vm/src/vm/runtime.rs index 17245fffca3..13005b616bd 100644 --- a/crates/vm/src/vm/runtime.rs +++ b/crates/vm/src/vm/runtime.rs @@ -150,6 +150,11 @@ pub(crate) fn register_interpreter(state: &PyRc) { ); } let mut entries = registry().entries.lock(); + // Entries are weak and an interpreter's lifetime is decided by its last + // `PyRc` — which outlives the `Interpreter` handle whenever + // `new_thread()` workers are still running — so nothing removes them at a + // fixed point. Reap the dead ones here to bound the table instead. + entries.retain(|_, entry| entry.state.strong_count() > 0); entries.insert( id, RegistryEntry { @@ -159,11 +164,6 @@ pub(crate) fn register_interpreter(state: &PyRc) { ); } -/// Unregister an interpreter (called when its owning `Interpreter` is dropped). -pub(crate) fn unregister_interpreter(id: i64) { - registry().entries.lock().remove(&id); -} - /// Look up a live interpreter state by id. #[must_use] pub fn lookup_interpreter(id: i64) -> Option> { @@ -198,6 +198,23 @@ pub fn interpreter_count() -> usize { list_interpreters().len() } +/// Reset the registry's locks after `fork()`. +/// +/// The tables are reachable from every thread, so a thread that died in the +/// fork may have left one locked; the child would then deadlock the first time +/// it enumerates interpreters (which the collector now does on every stop). +/// +/// # Safety +/// Must only be called after `fork()` in the child process, when no other +/// threads exist and the calling thread holds neither lock. +#[cfg(all(unix, feature = "threading"))] +pub unsafe fn reinit_after_fork() { + unsafe { + crate::common::lock::reinit_mutex_after_fork(®istry().entries); + crate::common::lock::reinit_mutex_after_fork(owned_interpreters()); + } +} + /// All live interpreter states, ordered by id. /// /// Used by the cyclic collector, which must stop every interpreter's threads diff --git a/crates/vm/src/vm/thread.rs b/crates/vm/src/vm/thread.rs index 683a09257dc..6d005b1c427 100644 --- a/crates/vm/src/vm/thread.rs +++ b/crates/vm/src/vm/thread.rs @@ -946,6 +946,19 @@ pub fn reinit_frame_slot_after_fork(vm: &VirtualMachine) { }); } +/// Drop this thread's cached slots for every interpreter except `keep_id`. +/// +/// After `fork()` only the calling thread survives, and the other +/// interpreters' registries are cleared; a cached slot would otherwise stay +/// current for an interpreter that no longer lists it, hiding the thread from +/// that interpreter's stop-the-world. The next enter builds a fresh slot. +#[cfg(feature = "threading")] +pub fn purge_other_interpreter_slots_after_fork(keep_id: i64) { + INTERP_THREAD_SLOTS.with(|slots| { + slots.borrow_mut().retain(|&id, _| id == keep_id); + }); +} + pub fn with_vm(obj: &PyObject, f: F) -> Option where F: Fn(&VirtualMachine) -> R, From 8f108ff1931c3b01a335d03c682845e28a882926 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 14 Aug 2026 03:28:10 +0900 Subject: [PATCH 05/20] vm: attach and detach thread slots when switching interpreters MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `enter_vm` decided whether to attach from `was_outermost` (an empty VM_STACK), which held while a thread could only ever be in one interpreter. With a slot per (thread, interpreter) pair, entering interpreter B from a thread already inside interpreter A's section switched CURRENT_THREAD_SLOT to B's slot but attached nothing: the thread then ran B's bytecode with B's slot DETACHED while A's slot stayed ATTACHED. A collector stopping B force-parks the DETACHED slot and concludes B is stopped, and then walks the object graph this thread is still mutating. Pair the attach/detach with the slot switch instead (≈ `_PyThreadState_Swap`): `begin_interpreter_section` detaches the enclosing interpreter's slot, makes the target slot current and attaches it, and `end_interpreter_section` undoes that and re-attaches the enclosing interpreter. Both live in `set_current_vm`, which every path making a VM current already goes through, so `enter_vm` and `VmBootstrapGuard` no longer track outermost-ness themselves. `nested_enter_of_subinterpreter_is_stoppable` covers this: it runs a subinterpreter nested inside the parent's section and asserts the sub's threads park when the sub's world is stopped. It fails with the previous attach-at-outermost-only behavior. Assisted-by: Claude Code:claude-opus-4-8 --- crates/vm/src/vm/interpreter.rs | 67 +++++++++++++++ crates/vm/src/vm/thread.rs | 147 ++++++++++++++++++-------------- 2 files changed, 152 insertions(+), 62 deletions(-) diff --git a/crates/vm/src/vm/interpreter.rs b/crates/vm/src/vm/interpreter.rs index 0ad9b89d364..c33e2d7d3e4 100644 --- a/crates/vm/src/vm/interpreter.rs +++ b/crates/vm/src/vm/interpreter.rs @@ -1382,6 +1382,73 @@ for _ in range(40): worker.join().expect("worker panicked"); } + /// Entering a subinterpreter from inside the parent's `enter` must attach + /// the subinterpreter's thread slot (and detach the parent's). Otherwise the + /// thread runs the sub's bytecode with a DETACHED slot, and a collector + /// stopping that interpreter force-parks the slot and wrongly concludes the + /// world is stopped while this thread keeps mutating objects. + #[cfg(all(feature = "threading", feature = "rustpython-compiler"))] + #[test] + fn nested_enter_of_subinterpreter_is_stoppable() { + use crate::compiler::Mode; + use alloc::sync::Arc; + use core::{ + sync::atomic::{AtomicBool, AtomicU64, Ordering}, + time::Duration, + }; + + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + let sub_state = sub.enter(|vm| vm.state.clone()); + + let progress = Arc::new(AtomicU64::new(0)); + let stop = Arc::new(AtomicBool::new(false)); + + // Worker runs the SUB nested inside an active MAIN section. + let progress_worker = Arc::clone(&progress); + let stop_worker = Arc::clone(&stop); + let main_vm = main.enter(|vm| vm.new_thread()); + let sub_vm = sub.enter(|vm| vm.new_thread()); + let worker = std::thread::spawn(move || { + main_vm.run(|_main| { + sub_vm.run(|vm| { + let source = "x = 1 + 1\n"; + let code = vm + .compile(source, Mode::Exec, "") + .map_err(|err| err.into_pyexception(vm, Some(source))) + .unwrap(); + while !stop_worker.load(Ordering::Acquire) { + let scope = vm.new_scope_with_builtins(); + vm.run_code_obj(code.clone(), scope).unwrap(); + progress_worker.fetch_add(1, Ordering::Release); + } + }); + }); + }); + + while progress.load(Ordering::Acquire) == 0 { + std::thread::yield_now(); + } + + sub_state.stop_the_world.stop_the_world(&sub_state); + let parked_at = progress.load(Ordering::Acquire); + std::thread::sleep(Duration::from_millis(50)); + assert_eq!( + progress.load(Ordering::Acquire), + parked_at, + "nested subinterpreter thread kept running while the sub's world was stopped" + ); + sub_state.stop_the_world.start_the_world(&sub_state); + + let resumed_from = progress.load(Ordering::Acquire); + while progress.load(Ordering::Acquire) == resumed_from { + std::thread::yield_now(); + } + + stop.store(true, Ordering::Release); + worker.join().expect("nested worker panicked"); + } + /// The process main id is recorded once and is stable across later creates. #[test] fn process_main_id_recorded_and_stable() { diff --git a/crates/vm/src/vm/thread.rs b/crates/vm/src/vm/thread.rs index 6d005b1c427..5fb40601cff 100644 --- a/crates/vm/src/vm/thread.rs +++ b/crates/vm/src/vm/thread.rs @@ -143,35 +143,22 @@ pub fn with_current_vm(f: impl FnOnce(&VirtualMachine) -> R) -> R { } fn set_current_vm(vm: &VirtualMachine, f: impl FnOnce() -> R) -> R { - // Ensure the thread slot matches this VM's interpreter before it becomes - // current (important when switching interpreters on one OS thread). + // Attach to this VM's interpreter, detaching the enclosing one if this is a + // switch between interpreters on the same OS thread. #[cfg(feature = "threading")] - init_thread_slot_if_needed(vm); + let switched = begin_interpreter_section(vm); VM_STACK.with(|vms| { vms.borrow_mut().push(vm.into()); scopeguard::defer! { vms.borrow_mut().pop(); - // Restore the slot for the VM that is current after pop (if any). #[cfg(feature = "threading")] - restore_thread_slot_from_stack(); + end_interpreter_section(switched); } f() }) } -/// After popping the VM stack, point `CURRENT_THREAD_SLOT` at the new top VM's -/// interpreter slot so nested multi-interpreter enters unwind cleanly. -#[cfg(feature = "threading")] -fn restore_thread_slot_from_stack() { - let prev = VM_STACK.with(|vms| vms.borrow().last().copied()); - if let Some(vm_ptr) = prev { - // SAFETY: entries on VM_STACK are valid for the enter/set_current_vm scope. - let vm = unsafe { vm_ptr.as_ref() }; - init_thread_slot_if_needed(vm); - } -} - pub fn try_with_current_vm(f: impl FnOnce(&VirtualMachine) -> R) -> Option { VM_STACK.with(|vms| { let vm = vms.borrow().last().copied()?; @@ -182,27 +169,8 @@ pub fn try_with_current_vm(f: impl FnOnce(&VirtualMachine) -> R) -> Option } pub fn enter_vm(vm: &VirtualMachine, f: impl FnOnce() -> R) -> R { - // Outermost enter_vm: transition DETACHED → ATTACHED - #[cfg(feature = "threading")] - let was_outermost = !current_vm_is_set(); - - // Initialize thread slot for this thread if not already done - #[cfg(feature = "threading")] - init_thread_slot_if_needed(vm); - - #[cfg(feature = "threading")] - if was_outermost { - attach_thread(vm); - } - - scopeguard::defer! { - // Outermost exit: transition ATTACHED → DETACHED - #[cfg(feature = "threading")] - if was_outermost { - detach_thread(); - } - } - + // Attach/detach is handled by `set_current_vm`, which pairs it with the + // VM_STACK push so that switching interpreters mid-stack stays consistent. set_current_vm(vm, f) } @@ -220,29 +188,19 @@ pub fn enter_vm(vm: &VirtualMachine, f: impl FnOnce() -> R) -> R { #[must_use] pub(crate) struct VmBootstrapGuard { #[cfg(feature = "threading")] - was_outermost: bool, + switched: bool, } impl VmBootstrapGuard { pub(crate) fn new(vm: &VirtualMachine) -> Self { - // Outermost: transition DETACHED → ATTACHED - #[cfg(feature = "threading")] - let was_outermost = !current_vm_is_set(); - - // Initialize thread slot for this thread if not already done #[cfg(feature = "threading")] - init_thread_slot_if_needed(vm); - - #[cfg(feature = "threading")] - if was_outermost { - attach_thread(vm); - } + let switched = begin_interpreter_section(vm); VM_STACK.with(|vms| vms.borrow_mut().push(vm.into())); Self { #[cfg(feature = "threading")] - was_outermost, + switched, } } } @@ -253,15 +211,8 @@ impl Drop for VmBootstrapGuard { vms.borrow_mut().pop(); }); - // Outermost exit: transition ATTACHED → DETACHED #[cfg(feature = "threading")] - if self.was_outermost { - detach_thread(); - } else { - // Nested bootstrap: restore the outer VM's interpreter thread slot. - #[cfg(feature = "threading")] - restore_thread_slot_from_stack(); - } + end_interpreter_section(self.switched); } } @@ -390,8 +341,16 @@ pub fn release_current_thread(state: CurrentVmAttachState) { /// `CURRENT_THREAD_SLOT` to that interpreter's slot (creating one if needed). #[cfg(feature = "threading")] fn init_thread_slot_if_needed(vm: &VirtualMachine) { + let slot = ensure_thread_slot(vm); + set_current_thread_slot(slot); +} + +/// Look up (creating if needed) this thread's [`ThreadSlot`] for `vm`'s +/// interpreter, without making it the current slot. +#[cfg(feature = "threading")] +fn ensure_thread_slot(vm: &VirtualMachine) -> CurrentFrameSlot { let interp_id = vm.state.interpreter_id; - let slot = INTERP_THREAD_SLOTS.with(|slots| { + INTERP_THREAD_SLOTS.with(|slots| { let mut slots = slots.borrow_mut(); if let Some(existing) = slots.get(&interp_id) { return existing.clone(); @@ -423,15 +382,79 @@ fn init_thread_slot_if_needed(vm: &VirtualMachine) { drop(registry); slots.insert(interp_id, new_slot.clone()); new_slot - }); + }) +} - #[cfg(all(unix, feature = "threading"))] +/// Make `slot` the current thread slot (and the cached top-frame pointer). +#[cfg(feature = "threading")] +fn set_current_thread_slot(slot: CurrentFrameSlot) { + #[cfg(unix)] CURRENT_TOP_FRAME_SLOT.with(|c| c.set(&slot.top_frame)); CURRENT_THREAD_SLOT.with(|current| { *current.borrow_mut() = Some(slot); }); } +/// Whether the current thread slot is ATTACHED. +#[cfg(feature = "threading")] +fn current_slot_is_attached() -> bool { + CURRENT_THREAD_SLOT.with(|slot| { + slot.borrow() + .as_ref() + .is_some_and(|s| s.state.load(Ordering::Acquire) == THREAD_ATTACHED) + }) +} + +/// Attach this thread to `vm`'s interpreter for the duration of a section, +/// detaching whichever interpreter it was attached to (≈ `_PyThreadState_Swap`). +/// +/// A thread must never be ATTACHED to two interpreters at once: stop-the-world +/// treats an ATTACHED slot as "running this interpreter's bytecode" and a +/// DETACHED slot as parkable without cooperation, so running interpreter B's +/// code while B's slot is DETACHED would let a collector conclude B is stopped +/// while this thread keeps mutating the (process-global) object graph. +/// +/// Returns whether the attachment changed, i.e. whether the matching +/// [`end_interpreter_section`] must undo it. +#[cfg(feature = "threading")] +fn begin_interpreter_section(vm: &VirtualMachine) -> bool { + let target = ensure_thread_slot(vm); + let already_current = CURRENT_THREAD_SLOT.with(|slot| { + slot.borrow() + .as_ref() + .is_some_and(|s| Arc::ptr_eq(s, &target)) + }); + if already_current && current_slot_is_attached() { + // Nested section in the same interpreter: already attached. + return false; + } + if !already_current && current_slot_is_attached() { + detach_thread(); + } + set_current_thread_slot(target); + attach_thread(vm); + true +} + +/// Undo [`begin_interpreter_section`]: detach this interpreter and re-attach the +/// enclosing one, if any. Call after the VM has been popped from `VM_STACK`. +#[cfg(feature = "threading")] +fn end_interpreter_section(switched: bool) { + if !switched { + return; + } + if current_slot_is_attached() { + detach_thread(); + } + // The enclosing section, if any, is the VM now on top of the stack. + if let Some(vm_ptr) = VM_STACK.with(|vms| vms.borrow().last().copied()) { + // SAFETY: entries on VM_STACK are valid for their enter/set_current_vm scope. + let vm = unsafe { vm_ptr.as_ref() }; + set_current_thread_slot(ensure_thread_slot(vm)); + attach_thread(vm); + } +} + /// Transition DETACHED → ATTACHED. Blocks if the thread was SUSPENDED by /// a stop-the-world request (like `_PyThreadState_Attach` + `tstate_wait_attach`). #[cfg(feature = "threading")] From 696a8c1011eec373ef6bb926d0f0a4b53635e45f Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 14 Aug 2026 12:50:36 +0900 Subject: [PATCH 06/20] vm: list only the current interpreter's subclasses MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Interpreters share the context, so `class Foo(int)` in one of them pushes onto the same `int.subclasses` every other one reads, and `int.__subclasses__()` returned types no other interpreter can reach. Record the creating interpreter on `HeapTypeExt` and filter `__subclasses__` by it, the way `lookup_tp_subclasses` reads `tp_subclasses` out of per-interpreter state for static builtin types. Types built before any interpreter exists — the ones the shared context creates, including the exception hierarchy — carry no id and stay visible to every interpreter, which is what `_PyStaticType_InitBuiltin` produces by registering the builtin subclass links once per interpreter. The other walks over `subclasses` (version-tag invalidation, abc flag propagation, mro updates, slot propagation) are left as they are: each starts from a type being mutated, so from a heap type, whose subclasses all live in the interpreter that created it. `subinterpreter_subclasses_are_scoped_to_their_interpreter` covers this and fails without the filter. Assisted-by: Claude Code:claude-opus-5 --- crates/vm/src/builtins/type.rs | 37 +++++++++++++++++++-- crates/vm/src/vm/interpreter.rs | 58 +++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/crates/vm/src/builtins/type.rs b/crates/vm/src/builtins/type.rs index 1776270751e..5a7169983af 100644 --- a/crates/vm/src/builtins/type.rs +++ b/crates/vm/src/builtins/type.rs @@ -294,6 +294,16 @@ pub struct HeapTypeExt { pub slots: Option>>, pub type_data: PyRwLock>, pub specialization_cache: TypeSpecializationCache, + /// The interpreter this type was created in, or `None` for the types the + /// shared context builds before any interpreter exists. + pub interpreter_id: Option, +} + +impl HeapTypeExt { + /// The interpreter a type created right now belongs to. + fn creating_interpreter_id() -> Option { + crate::vm::thread::try_with_current_vm(|vm| vm.state.interpreter_id) + } } pub struct TypeSpecializationCache { @@ -553,6 +563,22 @@ impl PyType { self.modified_inner(); } + /// Whether the interpreter with `interpreter_id` can see this type. + /// + /// Interpreters share the context, so a subclass of a shared type is + /// recorded on an object every interpreter reaches. Only the interpreter + /// that created it can name it, so only that one lists it. + pub fn is_visible_to_interpreter(&self, interpreter_id: i64) -> bool { + match self + .heaptype_ext + .as_ref() + .and_then(|ext| ext.interpreter_id) + { + Some(owner) => owner == interpreter_id, + None => true, + } + } + pub fn new_simple_heap( name: &str, base: &Py, @@ -589,6 +615,7 @@ impl PyType { slots: None, type_data: PyRwLock::new(None), specialization_cache: TypeSpecializationCache::new(), + interpreter_id: HeapTypeExt::creating_interpreter_id(), }; let base = bases[0].clone(); @@ -1948,13 +1975,18 @@ impl PyType { } #[pymethod] - fn __subclasses__(&self) -> PyList { + fn __subclasses__(&self, vm: &VirtualMachine) -> PyList { let mut subclasses = self.subclasses.write(); subclasses.retain(|x| x.upgrade().is_some()); + let interpreter_id = vm.state.interpreter_id; PyList::from( subclasses .iter() - .map(|x| x.upgrade().unwrap()) + .filter_map(|x| x.upgrade()) + .filter(|obj| { + obj.downcast_ref::() + .is_none_or(|typ| typ.is_visible_to_interpreter(interpreter_id)) + }) .collect::>(), ) } @@ -2368,6 +2400,7 @@ impl Constructor for PyType { slots: heaptype_slots.clone(), type_data: PyRwLock::new(None), specialization_cache: TypeSpecializationCache::new(), + interpreter_id: HeapTypeExt::creating_interpreter_id(), }; (slots, heaptype_ext) }; diff --git a/crates/vm/src/vm/interpreter.rs b/crates/vm/src/vm/interpreter.rs index c33e2d7d3e4..2bb08e80bc0 100644 --- a/crates/vm/src/vm/interpreter.rs +++ b/crates/vm/src/vm/interpreter.rs @@ -1201,6 +1201,64 @@ mod tests { }); } + /// Subclassing a shared type records the subclass on an object every + /// interpreter reaches, but only the interpreter that created it lists it. + #[test] + fn subinterpreter_subclasses_are_scoped_to_their_interpreter() { + use crate::compiler::Mode; + use crate::scope::Scope; + + fn run(vm: &VirtualMachine, scope: &Scope, source: &str) { + let code = vm + .compile(source, Mode::Exec, "") + .map_err(|err| err.into_pyexception(vm, Some(source))) + .unwrap(); + vm.run_code_obj(code, scope.clone()).unwrap(); + } + + fn lists_subclass(vm: &VirtualMachine, scope: &Scope, name: &str) -> bool { + run( + vm, + scope, + &format!("found = any(c.__name__ == {name:?} for c in int.__subclasses__())\n"), + ); + let found = scope.globals.get_item("found", vm).unwrap(); + found.try_to_bool(vm).unwrap() + } + + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + + // The scopes are what keep the classes alive; a subclass list holds + // only weak references, so both must outlive every assertion below. + let main_scope = main.enter(|vm| { + let scope = vm.new_scope_with_builtins(); + run(vm, &scope, "class MainOnly(int): pass\n"); + scope + }); + let sub_scope = sub.enter(|vm| { + let scope = vm.new_scope_with_builtins(); + run(vm, &scope, "class SubOnly(int): pass\n"); + scope + }); + + main.enter(|vm| { + assert!(lists_subclass(vm, &main_scope, "MainOnly")); + assert!(!lists_subclass(vm, &main_scope, "SubOnly")); + // A subclass built before either interpreter existed belongs to the + // shared context, so it stays visible to both. + assert!(lists_subclass(vm, &main_scope, "bool")); + }); + sub.enter(|vm| { + assert!(lists_subclass(vm, &sub_scope, "SubOnly")); + assert!(!lists_subclass(vm, &sub_scope, "MainOnly")); + assert!(lists_subclass(vm, &sub_scope, "bool")); + }); + + main.enter(|_| drop(main_scope)); + sub.enter(|_| drop(sub_scope)); + } + /// The runtime can own a subinterpreter by id and hand it back on destroy. #[cfg(feature = "threading")] #[test] From 562b6579912c33cbf19fdf9dc43063a1a865d76f Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 14 Aug 2026 14:25:51 +0900 Subject: [PATCH 07/20] gc: give each interpreter its own collector state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The generation lists stay process-wide, because an object is untracked from `default_dealloc`, where no interpreter is in scope to route to. What changes is that a collection no longer acts on every interpreter's objects, and the gc module no longer reports one interpreter's state to another. `track_object` stamps the running interpreter into a new `gc_owner` word on the object header, and a collection takes as candidates only the objects carrying its own tag plus the ones carrying none. `gc_owner` fits in the padding the header's alignment already forces, so objects do not grow; an assertion on the header size keeps it that way. Objects allocated with no interpreter running — everything the shared context builds — carry no owner and stay candidates for every interpreter, which is where they were before. Objects that outlive the interpreter that tracked them are adopted the same way by the next full collection, rather than being left to a collector that will never come. `enabled`, the thresholds, the debug flags, the statistics, `gc.garbage` and `gc.callbacks` move onto `PyGlobalState` — the last two off the shared `Context` — so `gc.disable()`, `gc.set_threshold()`, `gc.get_stats()`, `gc.get_objects()` and `gc.garbage` describe the interpreter that asks. The occupancy counts behind `gc.get_count()` and `gc.get_freeze_count()` stay process-wide: they measure how full the shared lists are. Their decrements are saturating now, since a collection zeroes the generations it emptied while another interpreter's objects are still sitting in them. Stop-the-world still stops every interpreter. Unowned objects are candidates and any interpreter can incref one, so the refcounts a collection reads are only stable while all of them are parked. This also fixes a deadlock it exposed: `CollectStopTheWorld` dropped its references to the stopped interpreters while the collection still held the generation read locks, so releasing the last reference to one — which frees its objects, and so untracks them — waited for a write lock behind that read lock. The references are now held until the guard itself drops. `collections_only_reach_the_collecting_interpreter` and `get_objects_only_reports_the_calling_interpreter` cover this and both fail without the owner check. Assisted-by: Claude Code:claude-opus-5 --- crates/capi/src/objimpl.rs | 28 +- crates/vm/src/builtins/function.rs | 5 +- crates/vm/src/frame.rs | 5 +- crates/vm/src/gc_state.rs | 566 ++++++++++++++++++++--------- crates/vm/src/object/core.rs | 38 +- crates/vm/src/object/mod.rs | 2 +- crates/vm/src/stdlib/gc.rs | 60 +-- crates/vm/src/stdlib/posix.rs | 5 +- crates/vm/src/vm/context.rs | 8 - crates/vm/src/vm/interpreter.rs | 113 +++++- crates/vm/src/vm/mod.rs | 14 +- crates/vm/src/vm/thread.rs | 20 + 12 files changed, 620 insertions(+), 244 deletions(-) diff --git a/crates/capi/src/objimpl.rs b/crates/capi/src/objimpl.rs index 99b0be7cc68..aa74bb69379 100644 --- a/crates/capi/src/objimpl.rs +++ b/crates/capi/src/objimpl.rs @@ -9,7 +9,7 @@ pub unsafe extern "C" fn PyObject_GC_Track(op: *mut PyObject) { with_vm(|_vm| { let obj = unsafe { &*op }; if !obj.is_gc_tracked() { - unsafe { gc_state::gc_state().track_object(obj.into()) }; + unsafe { gc_state::gc_state().track_object(obj.into(), gc_state::current_owner()) }; } }) } @@ -36,29 +36,33 @@ pub unsafe extern "C" fn PyObject_GC_IsFinalized(op: *mut PyObject) -> c_int { #[unsafe(no_mangle)] pub extern "C" fn PyGC_Collect() -> isize { - let result = gc_state::gc_state().collect(2); - (result.collected + result.uncollectable) as isize + with_vm(|vm| { + let result = vm.state.gc.collect(2); + (result.collected + result.uncollectable) as isize + }) } #[unsafe(no_mangle)] pub extern "C" fn PyGC_Enable() -> c_int { - let gc = gc_state::gc_state(); - let was_enabled = gc.is_enabled(); - gc.enable(); - was_enabled.into() + with_vm(|vm| { + let was_enabled: c_int = vm.state.gc.is_enabled().into(); + vm.state.gc.enable(); + was_enabled + }) } #[unsafe(no_mangle)] pub extern "C" fn PyGC_Disable() -> c_int { - let gc = gc_state::gc_state(); - let was_enabled = gc.is_enabled(); - gc.disable(); - was_enabled.into() + with_vm(|vm| { + let was_enabled: c_int = vm.state.gc.is_enabled().into(); + vm.state.gc.disable(); + was_enabled + }) } #[unsafe(no_mangle)] pub extern "C" fn PyGC_IsEnabled() -> c_int { - gc_state::gc_state().is_enabled().into() + with_vm(|vm| -> c_int { vm.state.gc.is_enabled().into() }) } #[unsafe(no_mangle)] diff --git a/crates/vm/src/builtins/function.rs b/crates/vm/src/builtins/function.rs index 90315bcb194..ac35a19c013 100644 --- a/crates/vm/src/builtins/function.rs +++ b/crates/vm/src/builtins/function.rs @@ -669,7 +669,10 @@ impl Py { ); // SAFETY: the frame is alive (held by `frame`) and untracked. unsafe { - crate::gc_state::gc_state().track_object(core::ptr::NonNull::from(frame.as_object())); + crate::gc_state::gc_state().track_object( + core::ptr::NonNull::from(frame.as_object()), + crate::gc_state::current_owner(), + ); } frame.set_generator(&obj); obj diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index a1e7a98d545..3e31b29cc00 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -2611,7 +2611,10 @@ pub(crate) fn release_datastack_frame(frame: &Py, vm: &VirtualMachi ); // SAFETY: the frame is alive (held by `frame` and the escaped reference) // and untracked. - unsafe { crate::gc_state::gc_state().track_object(NonNull::from(frame_obj)) }; + unsafe { + crate::gc_state::gc_state() + .track_object(NonNull::from(frame_obj), crate::gc_state::current_owner()) + }; } type BinaryOpExtendGuard = fn(&PyObject, &PyObject, &VirtualMachine) -> bool; diff --git a/crates/vm/src/gc_state.rs b/crates/vm/src/gc_state.rs index beb854063fd..de249e67968 100644 --- a/crates/vm/src/gc_state.rs +++ b/crates/vm/src/gc_state.rs @@ -4,7 +4,7 @@ use crate::common::linked_list::LinkedList; use crate::common::lock::{PyMutex, PyRwLock}; -use crate::object::{GC_PERMANENT, GC_UNTRACKED, GcLink}; +use crate::object::{GC_NO_OWNER, GC_PERMANENT, GC_UNTRACKED, GcLink}; use crate::{AsObject, PyObject, PyObjectRef}; use core::ptr::NonNull; use core::sync::atomic::{AtomicBool, AtomicU32, AtomicUsize, Ordering}; @@ -56,10 +56,12 @@ pub struct GcStats { pub duration: f64, } -/// A single GC generation with intrusive linked list +/// One generation's collection policy and statistics, per interpreter. +/// +/// The objects themselves live in the process-wide lists on [`GcState`], so the +/// occupancy count sits there; what an interpreter owns is when to collect and +/// what its own collections have done. pub struct GcGeneration { - /// Number of objects in this generation - count: AtomicUsize, /// Threshold for triggering collection threshold: AtomicU32, /// Collection statistics @@ -70,7 +72,6 @@ impl GcGeneration { #[must_use] pub const fn new(threshold: u32) -> Self { Self { - count: AtomicUsize::new(0), threshold: AtomicU32::new(threshold), stats: PyMutex::new(GcStats { collections: 0, @@ -82,10 +83,6 @@ impl GcGeneration { } } - pub fn count(&self) -> usize { - self.count.load(Ordering::SeqCst) - } - pub fn threshold(&self) -> u32 { self.threshold.load(Ordering::SeqCst) } @@ -131,6 +128,26 @@ impl GcGeneration { } } +/// Drop one from a generation's occupancy. +/// +/// A collection resets the counts of the generations it emptied, but it only +/// empties its own interpreter's objects; another interpreter's stay behind with +/// the count already zeroed, and untracking one of those must not wrap. +fn release_count(count: &AtomicUsize) { + let _ = count.fetch_update(Ordering::SeqCst, Ordering::SeqCst, |count| { + Some(count.saturating_sub(1)) + }); +} + +/// Whether `owner`'s collections act on `obj`. +/// +/// Objects with no owner — everything the shared context allocates, and anything +/// allocated with no interpreter current — belong to all of them. +fn is_owned_by(obj: &PyObject, owner: u32) -> bool { + let obj_owner = obj.gc_owner(); + obj_owner == owner || obj_owner == GC_NO_OWNER +} + /// Wrapper for NonNull to impl Hash/Eq for use in temporary collection sets. /// Only used within collect_inner, never shared across threads. #[derive(Clone, Copy, PartialEq, Eq, Hash)] @@ -147,18 +164,23 @@ struct GcPtr(NonNull); /// happens explicitly once the snapshot has pinned every object; `Drop` is a /// backstop that also restarts on the early-return paths. /// -/// The generation lists are process-global, so a collection walks objects -/// owned by *every* interpreter. Stopping only the collecting interpreter -/// would leave another interpreter's threads mutating the same object graph, -/// so every live interpreter is stopped. Stopping in `runtime` id order keeps -/// exclusion acquisition ordered; the `collecting` mutex additionally +/// A collection acts on one interpreter's objects, but its candidates include +/// the ones no interpreter owns, which every interpreter can reference and so +/// incref. Reading a refcount that another interpreter is changing is what +/// makes an object look unreachable when it is not, so every live interpreter +/// is stopped, not just the collecting one. Stopping in `runtime` id order +/// keeps exclusion acquisition ordered; the `collecting` mutex additionally /// serializes collections process-wide, so no second collector can take these /// exclusions in another order. #[cfg(feature = "threading")] struct CollectStopTheWorld { /// Stopped interpreter states, in stop order. Held as strong references so - /// an interpreter cannot be dropped between stop and restart. + /// an interpreter cannot be dropped between stop and restart, and kept past + /// the restart so that releasing the last one — which frees that + /// interpreter's objects, and so removes them from these lists — happens + /// after the collection has let go of the generation locks. stopped: Vec>, + restarted: bool, } #[cfg(feature = "threading")] @@ -173,6 +195,7 @@ impl CollectStopTheWorld { if !crate::vm::thread::current_vm_is_set() { return Self { stopped: Vec::new(), + restarted: true, }; } @@ -182,6 +205,7 @@ impl CollectStopTheWorld { // and their exclusion held forever. let mut guard = Self { stopped: Vec::new(), + restarted: false, }; for state in crate::vm::runtime::live_interpreter_states() { state.stop_the_world.stop_the_world(&state); @@ -192,9 +216,14 @@ impl CollectStopTheWorld { /// Restart the world. Idempotent. fn restart(&mut self) { - // Reverse of the stop order. - for state in self.stopped.drain(..).rev() { - state.stop_the_world.start_the_world(&state); + if self.restarted { + return; + } + self.restarted = true; + // Reverse of the stop order. The references stay until this guard is + // dropped; see the field comment. + for state in self.stopped.iter().rev() { + state.stop_the_world.start_the_world(state); } } @@ -212,29 +241,32 @@ impl Drop for CollectStopTheWorld { } } -/// Global GC state +/// The process-wide object lists every interpreter's collections walk. +/// +/// Interpreter-owned policy and results live in [`GcInterpreterState`]; what is +/// here is shared because the lists are: an object is untracked from +/// `default_dealloc`, where no interpreter is in scope, so it has to be findable +/// without one. pub struct GcState { - /// 3 generations (0 = youngest, 2 = oldest) - pub generations: [GcGeneration; 3], - /// Permanent generation (frozen objects) - pub permanent: GcGeneration, - /// GC enabled flag - pub enabled: AtomicBool, /// Per-generation intrusive linked lists for object tracking. /// Objects start in gen0, survivors are promoted to gen1, then gen2. generation_lists: [PyRwLock>; 3], /// Frozen/permanent objects (excluded from normal GC) permanent_list: PyRwLock>, - /// Debug flags - pub debug: AtomicU32, - /// gc.garbage list (uncollectable objects with __del__) - pub garbage: PyMutex>, - /// gc.callbacks list - pub callbacks: PyMutex>, + /// Number of tracked objects per generation, across all interpreters. + counts: [AtomicUsize; 3], + /// Number of frozen objects. + permanent_count: AtomicUsize, /// Mutex for collection (prevents concurrent collections) collecting: PyMutex<()>, /// Allocation counter for gen0 alloc_count: AtomicUsize, + /// Next `gc_owner` tag to hand to an interpreter. + next_owner: AtomicU32, + /// Tags of interpreters that are gone. Their objects outlived them, so a + /// collection adopts them — tags them `GC_NO_OWNER` again — as it walks, + /// rather than leaving them for a collector that will never come. + retired: PyMutex>, } // SAFETY: All fields are either inherently Send/Sync (atomics, RwLock, Mutex) or protected by PyMutex. @@ -254,102 +286,71 @@ impl GcState { #[must_use] pub const fn new() -> Self { Self { - generations: [ - GcGeneration::new(2000), // young - GcGeneration::new(10), // old[0] - GcGeneration::new(0), // old[1] - ], - permanent: GcGeneration::new(0), - enabled: AtomicBool::new(true), generation_lists: [ PyRwLock::new(LinkedList::new()), PyRwLock::new(LinkedList::new()), PyRwLock::new(LinkedList::new()), ], permanent_list: PyRwLock::new(LinkedList::new()), - debug: AtomicU32::new(0), - garbage: PyMutex::new(Vec::new()), - callbacks: PyMutex::new(Vec::new()), + counts: [ + AtomicUsize::new(0), + AtomicUsize::new(0), + AtomicUsize::new(0), + ], + permanent_count: AtomicUsize::new(0), collecting: PyMutex::new(()), alloc_count: AtomicUsize::new(0), + next_owner: AtomicU32::new(GC_NO_OWNER + 1), + retired: PyMutex::new(Vec::new()), } } - /// Check if GC is enabled - pub fn is_enabled(&self) -> bool { - self.enabled.load(Ordering::SeqCst) - } - - /// Enable GC - pub fn enable(&self) { - self.enabled.store(true, Ordering::SeqCst); - } - - /// Disable GC - pub fn disable(&self) { - self.enabled.store(false, Ordering::SeqCst); - } - - /// Get debug flags - pub fn get_debug(&self) -> GcDebugFlags { - GcDebugFlags::from_bits_truncate(self.debug.load(Ordering::SeqCst)) - } - - /// Set debug flags - pub fn set_debug(&self, flags: GcDebugFlags) { - self.debug.store(flags.bits(), Ordering::SeqCst); - } - - /// Get thresholds for all generations - pub fn get_threshold(&self) -> (u32, u32, u32) { - ( - self.generations[0].threshold(), - self.generations[1].threshold(), - self.generations[2].threshold(), - ) + /// Reserve a tag for a new interpreter. Tags are never reused; exhausting + /// the 32-bit space falls back to `GC_NO_OWNER`, which costs isolation but + /// stays correct, rather than aliasing a live interpreter. + fn alloc_owner(&self) -> u32 { + self.next_owner + .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |next| { + next.checked_add(1) + }) + .unwrap_or(GC_NO_OWNER) } - /// Set thresholds - pub fn set_threshold(&self, t0: u32, t1: Option, t2: Option) { - self.generations[0].set_threshold(t0); - if let Some(t1) = t1 { - self.generations[1].set_threshold(t1); - } - if let Some(t2) = t2 { - self.generations[2].set_threshold(t2); + /// Record that `owner`'s interpreter is gone, so the next collection adopts + /// whatever it left behind. Retagging the objects here would mean walking + /// every list under an interpreter drop, which happens while a collection + /// holds the collecting lock. + fn retire_owner(&self, owner: u32) { + if owner == GC_NO_OWNER { + return; } + self.retired.lock().push(owner); } - /// Get counts for all generations + /// Get counts for all generations. Tracked objects are shared, so these are + /// process-wide even though the thresholds they are compared against are + /// per interpreter. pub fn get_count(&self) -> (usize, usize, usize) { ( - self.generations[0].count(), - self.generations[1].count(), - self.generations[2].count(), + self.counts[0].load(Ordering::SeqCst), + self.counts[1].load(Ordering::SeqCst), + self.counts[2].load(Ordering::SeqCst), ) } - /// Get statistics for all generations - pub fn get_stats(&self) -> [GcStats; 3] { - [ - self.generations[0].stats(), - self.generations[1].stats(), - self.generations[2].stats(), - ] - } - - /// Track a new object (add to gen0). + /// Track a new object (add to gen0) as owned by `owner`. /// O(1) — intrusive linked list push_front, no hashing. /// /// # Safety /// obj must be a valid pointer to a PyObject - pub unsafe fn track_object(&self, obj: NonNull) { + pub unsafe fn track_object(&self, obj: NonNull, owner: u32) { let obj_ref = unsafe { obj.as_ref() }; obj_ref.set_gc_tracked(); obj_ref.set_gc_generation(0); + obj_ref.set_gc_owner(owner); self.generation_lists[0].write().push_front(obj); - self.generations[0].count.fetch_add(1, Ordering::SeqCst); + self.counts[0].fetch_add(1, Ordering::SeqCst); self.alloc_count.fetch_add(1, Ordering::SeqCst); } @@ -369,10 +370,10 @@ impl GcState { ( &self.generation_lists[obj_gen as usize] as &PyRwLock>, - &self.generations[obj_gen as usize].count, + &self.counts[obj_gen as usize], ) } else if obj_gen == GC_PERMANENT { - (&self.permanent_list, &self.permanent.count) + (&self.permanent_list, &self.permanent_count) } else { return; // GC_UNTRACKED or unknown — already untracked }; @@ -384,7 +385,7 @@ impl GcState { continue; // Retry with the updated generation } if unsafe { list.remove(obj) }.is_some() { - count.fetch_sub(1, Ordering::SeqCst); + release_count(count); obj_ref.clear_gc_tracked(); obj_ref.set_gc_generation(GC_UNTRACKED); } else { @@ -402,14 +403,18 @@ impl GcState { } } - /// Get tracked objects (for gc.get_objects) - /// If generation is None, returns all tracked objects. - /// If generation is Some(n), returns objects in generation n only. - pub fn get_objects(&self, generation: Option) -> Vec { + /// Get the objects `owner` tracks (for gc.get_objects), plus the ones no + /// interpreter owns. + /// If generation is None, returns all such objects. + /// If generation is Some(n), returns those in generation n only. + pub fn get_objects(&self, generation: Option, owner: u32) -> Vec { fn collect_from_list( list: &LinkedList, + owner: u32, ) -> impl Iterator + '_ { - list.iter().filter_map(|obj| obj.try_to_owned()) + list.iter() + .filter(move |obj| is_owned_by(obj, owner)) + .filter_map(|obj| obj.try_to_owned()) } match generation { @@ -417,14 +422,14 @@ impl GcState { // Return all tracked objects from all generations + permanent let mut result = Vec::new(); for gen_list in &self.generation_lists { - result.extend(collect_from_list(&gen_list.read())); + result.extend(collect_from_list(&gen_list.read(), owner)); } - result.extend(collect_from_list(&self.permanent_list.read())); + result.extend(collect_from_list(&self.permanent_list.read(), owner)); result } Some(g) if (0..=2).contains(&g) => { let guard = self.generation_lists[g as usize].read(); - collect_from_list(&guard).collect() + collect_from_list(&guard, owner).collect() } _ => Vec::new(), } @@ -433,14 +438,14 @@ impl GcState { /// Check if automatic GC should run and run it if needed. /// Called after object allocation. /// Returns true if GC was run, false otherwise. - pub fn maybe_collect(&self) -> bool { - if !self.is_enabled() { + fn maybe_collect(&self, gc: &GcInterpreterState) -> bool { + if !gc.is_enabled() { return false; } // Check gen0 threshold - let count0 = self.generations[0].count.load(Ordering::SeqCst) as u32; - let threshold0 = self.generations[0].threshold(); + let count0 = self.counts[0].load(Ordering::SeqCst) as u32; + let threshold0 = gc.generations[0].threshold(); if threshold0 > 0 && count0 >= threshold0 { #[cfg(feature = "threading")] { @@ -456,7 +461,7 @@ impl GcState { // thread whose frames could be read mid-mutation, so collect inline. #[cfg(not(feature = "threading"))] { - self.collect(0); + self.collect_inner(gc, 0, false); return true; } } @@ -464,18 +469,13 @@ impl GcState { false } - /// Perform garbage collection on the given generation - pub fn collect(&self, generation: usize) -> CollectResult { - self.collect_inner(generation, false) - } - - /// Force collection even if GC is disabled (for manual gc.collect() calls) - pub fn collect_force(&self, generation: usize) -> CollectResult { - self.collect_inner(generation, true) - } - - fn collect_inner(&self, generation: usize, force: bool) -> CollectResult { - if !force && !self.is_enabled() { + fn collect_inner( + &self, + gc: &GcInterpreterState, + generation: usize, + force: bool, + ) -> CollectResult { + if !force && !gc.is_enabled() { return CollectResult::default(); } @@ -494,7 +494,7 @@ impl GcState { core::sync::atomic::fence(Ordering::SeqCst); let generation = generation.min(2); - let debug = self.get_debug(); + let debug = gc.get_debug(); // Clear the method cache to release strong references that // might prevent cycle collection (_PyType_ClearCache). @@ -532,26 +532,45 @@ impl GcState { .map(|i| self.generation_lists[i].read()) .collect(); + // Only this interpreter's objects, plus the ones no interpreter owns. + // Another interpreter's objects stay out of the candidate set, so they + // act as external roots: anything they reference survives this pass. + let owner = gc.owner; + let retired = self.retired.lock().clone(); let mut collecting: HashSet = HashSet::new(); for gen_list in &gen_locks { for obj in gen_list.iter() { - if obj.strong_count() > 0 { + if retired.contains(&obj.gc_owner()) { + obj.set_gc_owner(GC_NO_OWNER); + } + if obj.strong_count() > 0 && is_owned_by(obj, owner) { collecting.insert(GcPtr(NonNull::from(obj))); } } } + // A full collection is the only one that sees every generation, so it + // is where adoption finishes and the tags stop being tracked. + if generation == 2 && !retired.is_empty() { + for obj in self.permanent_list.read().iter() { + if retired.contains(&obj.gc_owner()) { + obj.set_gc_owner(GC_NO_OWNER); + } + } + self.retired.lock().retain(|tag| !retired.contains(tag)); + } + if collecting.is_empty() { // Reset counts for generations whose objects were promoted away. // For gen2 (oldest), survivors stay in-place so don't reset gen2 count. let reset_end = if generation >= 2 { 2 } else { generation + 1 }; for i in 0..reset_end { - self.generations[i].count.store(0, Ordering::SeqCst); + self.counts[i].store(0, Ordering::SeqCst); } let duration = elapsed_secs(start_time); - self.generations[generation].update_stats(0, 0, 0, duration); + gc.generations[generation].update_stats(0, 0, 0, duration); return CollectResult { collected: 0, uncollectable: 0, @@ -722,12 +741,12 @@ impl GcState { self.promote_survivors(generation, &survivor_refs); let reset_end = if generation >= 2 { 2 } else { generation + 1 }; for i in 0..reset_end { - self.generations[i].count.store(0, Ordering::SeqCst); + self.counts[i].store(0, Ordering::SeqCst); } let duration = elapsed_secs(start_time); - self.generations[generation].update_stats(0, 0, candidates, duration); + gc.generations[generation].update_stats(0, 0, candidates, duration); return CollectResult { collected: 0, uncollectable: 0, @@ -745,12 +764,12 @@ impl GcState { self.promote_survivors(generation, &survivor_refs); let reset_end = if generation >= 2 { 2 } else { generation + 1 }; for i in 0..reset_end { - self.generations[i].count.store(0, Ordering::SeqCst); + self.counts[i].store(0, Ordering::SeqCst); } let duration = elapsed_secs(start_time); - self.generations[generation].update_stats(0, 0, candidates, duration); + gc.generations[generation].update_stats(0, 0, candidates, duration); return CollectResult { collected: 0, uncollectable: 0, @@ -870,7 +889,7 @@ impl GcState { } if debug.contains(GcDebugFlags::SAVEALL) { - let mut garbage_guard = self.garbage.lock(); + let mut garbage_guard = gc.garbage.lock(); for obj_ref in &truly_dead { garbage_guard.push(obj_ref.clone()); } @@ -947,7 +966,10 @@ impl GcState { reason = "Iteration order doesn't matter here" )] for &ptr in &late_resurrected { - unsafe { self.track_object(ptr.0) }; + // Re-tracking a resurrected object: it keeps the owner it + // was allocated under. + let owner = unsafe { ptr.0.as_ref() }.gc_owner(); + unsafe { self.track_object(ptr.0, owner) }; } } rustpython_common::refcount::with_deferred_drops(|| { @@ -971,12 +993,12 @@ impl GcState { // For gen2 (oldest), survivors stay in-place so don't reset gen2 count. let reset_end = if generation >= 2 { 2 } else { generation + 1 }; for i in 0..reset_end { - self.generations[i].count.store(0, Ordering::SeqCst); + self.counts[i].store(0, Ordering::SeqCst); } let duration = elapsed_secs(start_time); - self.generations[generation].update_stats(collected, 0, candidates, duration); + gc.generations[generation].update_stats(collected, 0, candidates, duration); CollectResult { collected, @@ -1019,14 +1041,10 @@ impl GcState { } if unsafe { src.remove(ptr) }.is_some() { - self.generations[src_gen] - .count - .fetch_sub(1, Ordering::SeqCst); + release_count(&self.counts[src_gen]); dst.push_front(ptr); - self.generations[next_gen] - .count - .fetch_add(1, Ordering::SeqCst); + self.counts[next_gen].fetch_add(1, Ordering::SeqCst); obj.set_gc_generation(next_gen as u8); } @@ -1036,45 +1054,66 @@ impl GcState { /// Get count of frozen objects pub fn get_freeze_count(&self) -> usize { - self.permanent.count() + self.permanent_count.load(Ordering::SeqCst) } - /// Freeze all tracked objects (move to permanent generation). + /// Freeze the objects `owner` could collect (move them to the permanent + /// generation). /// Lock order: generation_lists[i] → permanent_list (consistent with unfreeze). - pub fn freeze(&self) { + fn freeze(&self, owner: u32) { let mut count = 0usize; for (gen_idx, gen_list) in self.generation_lists.iter().enumerate() { let mut list = gen_list.write(); let mut perm = self.permanent_list.write(); - while let Some(ptr) = list.pop_front() { + let moving: Vec<_> = list + .iter() + .filter(|obj| is_owned_by(obj, owner)) + .map(NonNull::from) + .collect(); + for ptr in moving { + if unsafe { list.remove(ptr) }.is_none() { + continue; + } perm.push_front(ptr); unsafe { ptr.as_ref().set_gc_generation(GC_PERMANENT) }; count += 1; + release_count(&self.counts[gen_idx]); } - self.generations[gen_idx].count.store(0, Ordering::SeqCst); } - self.permanent.count.fetch_add(count, Ordering::SeqCst); + self.permanent_count.fetch_add(count, Ordering::SeqCst); } - /// Unfreeze all objects (move from permanent to gen2). + /// Unfreeze the objects `owner` froze (move them from permanent to gen2). /// Lock order: generation_lists[2] → permanent_list (consistent with freeze). - pub fn unfreeze(&self) { + fn unfreeze(&self, owner: u32) { let mut count = 0usize; { let mut gen2 = self.generation_lists[2].write(); let mut perm_list = self.permanent_list.write(); - while let Some(ptr) = perm_list.pop_front() { + let moving: Vec<_> = perm_list + .iter() + .filter(|obj| is_owned_by(obj, owner)) + .map(NonNull::from) + .collect(); + for ptr in moving { + if unsafe { perm_list.remove(ptr) }.is_none() { + continue; + } gen2.push_front(ptr); unsafe { ptr.as_ref().set_gc_generation(2) }; count += 1; } - self.permanent.count.store(0, Ordering::SeqCst); + let _ = self.permanent_count.fetch_update( + Ordering::SeqCst, + Ordering::SeqCst, + |permanent| Some(permanent.saturating_sub(count)), + ); } - self.generations[2].count.fetch_add(count, Ordering::SeqCst); + self.counts[2].fetch_add(count, Ordering::SeqCst); } /// Reset all locks to unlocked state after fork(). @@ -1091,13 +1130,7 @@ impl GcState { unsafe { reinit_mutex_after_fork(&self.collecting); - reinit_mutex_after_fork(&self.garbage); - reinit_mutex_after_fork(&self.callbacks); - - for generation in &self.generations { - generation.reinit_stats_after_fork(); - } - self.permanent.reinit_stats_after_fork(); + reinit_mutex_after_fork(&self.retired); for rw in &self.generation_lists { reinit_rwlock_after_fork(rw); @@ -1107,19 +1140,192 @@ impl GcState { } } +/// Per-interpreter garbage collector state (≈ `PyInterpreterState.gc`). +/// +/// The generation lists are process-wide (see [`GcState`]); what an interpreter +/// owns is the policy applied to them and the results — which objects its +/// collections consider, whether they run automatically, and where uncollectable +/// objects end up. +pub struct GcInterpreterState { + /// Tag written into every object this interpreter tracks. + owner: u32, + /// Per-generation thresholds and statistics. + pub generations: [GcGeneration; 3], + /// GC enabled flag + enabled: AtomicBool, + /// Debug flags + debug: AtomicU32, + /// Uncollectable objects saved by this interpreter's collections, drained + /// into `py_garbage` by `gc.collect()`. + pub garbage: PyMutex>, + /// `gc.garbage` + pub py_garbage: crate::builtins::PyListRef, + /// `gc.callbacks` + pub py_callbacks: crate::builtins::PyListRef, +} + +impl GcInterpreterState { + pub fn new(ctx: &crate::vm::Context) -> Self { + Self { + owner: gc_state().alloc_owner(), + generations: [ + GcGeneration::new(2000), // young + GcGeneration::new(10), // old[0] + GcGeneration::new(0), // old[1] + ], + enabled: AtomicBool::new(true), + debug: AtomicU32::new(0), + garbage: PyMutex::new(Vec::new()), + py_garbage: ctx.new_list(Vec::new()), + py_callbacks: ctx.new_list(Vec::new()), + } + } + + /// Check if GC is enabled + pub fn is_enabled(&self) -> bool { + self.enabled.load(Ordering::SeqCst) + } + + /// Enable GC + pub fn enable(&self) { + self.enabled.store(true, Ordering::SeqCst); + } + + /// Disable GC + pub fn disable(&self) { + self.enabled.store(false, Ordering::SeqCst); + } + + /// Get debug flags + pub fn get_debug(&self) -> GcDebugFlags { + GcDebugFlags::from_bits_truncate(self.debug.load(Ordering::SeqCst)) + } + + /// Set debug flags + pub fn set_debug(&self, flags: GcDebugFlags) { + self.debug.store(flags.bits(), Ordering::SeqCst); + } + + /// Get thresholds for all generations + pub fn get_threshold(&self) -> (u32, u32, u32) { + ( + self.generations[0].threshold(), + self.generations[1].threshold(), + self.generations[2].threshold(), + ) + } + + /// Set thresholds + pub fn set_threshold(&self, t0: u32, t1: Option, t2: Option) { + self.generations[0].set_threshold(t0); + if let Some(t1) = t1 { + self.generations[1].set_threshold(t1); + } + if let Some(t2) = t2 { + self.generations[2].set_threshold(t2); + } + } + + /// Get statistics for all generations + pub fn get_stats(&self) -> [GcStats; 3] { + [ + self.generations[0].stats(), + self.generations[1].stats(), + self.generations[2].stats(), + ] + } + + /// Perform garbage collection on the given generation + pub fn collect(&self, generation: usize) -> CollectResult { + gc_state().collect_inner(self, generation, false) + } + + /// Force collection even if GC is disabled (for manual gc.collect() calls) + pub fn collect_force(&self, generation: usize) -> CollectResult { + gc_state().collect_inner(self, generation, true) + } + + /// The tracked objects this interpreter can reach (for gc.get_objects). + pub fn get_objects(&self, generation: Option) -> Vec { + gc_state().get_objects(generation, self.owner) + } + + /// Move the objects this interpreter could collect into the permanent + /// generation. + pub fn freeze(&self) { + gc_state().freeze(self.owner); + } + + /// Move them back out of it. + pub fn unfreeze(&self) { + gc_state().unfreeze(self.owner); + } + + /// Reset this interpreter's GC locks to unlocked state after fork(). + /// + /// # Safety + /// Must only be called after fork() in the child process when no other + /// threads exist. The calling thread must NOT hold any of these locks. + #[cfg(all(unix, feature = "threading"))] + pub unsafe fn reinit_after_fork(&self) { + unsafe { + crate::common::lock::reinit_mutex_after_fork(&self.garbage); + for generation in &self.generations { + generation.reinit_stats_after_fork(); + } + } + } +} + +impl Drop for GcInterpreterState { + fn drop(&mut self) { + // Objects this interpreter tracked can outlive it (another interpreter + // may still hold one). Clearing the tag hands them to every collection + // instead of stranding them, and frees the tag for reuse. + gc_state().retire_owner(self.owner); + } +} + +/// The tag `track_object` should write for the interpreter running now. +#[must_use] +pub fn current_owner() -> u32 { + // SAFETY: the pointee is owned by the `PyGlobalState` of the VM on top of + // this thread's VM stack, which outlives the section this call runs in. + crate::vm::thread::current_gc_state().map_or(GC_NO_OWNER, |gc| unsafe { gc.as_ref() }.owner) +} + +/// Track a freshly allocated object under the interpreter running now, and let +/// it collect if the allocation pushed gen0 past its threshold. +/// +/// # Safety +/// obj must be a valid pointer to a PyObject that is not already tracked. +pub(crate) unsafe fn track_new_object(obj: NonNull) { + let state = gc_state(); + let Some(gc) = crate::vm::thread::current_gc_state() else { + // No interpreter is running: the shared context builds its own objects + // this way. They are left unowned, so every interpreter collects them. + unsafe { state.track_object(obj, GC_NO_OWNER) }; + return; + }; + // SAFETY: as in `current_owner`. + let gc = unsafe { gc.as_ref() }; + unsafe { state.track_object(obj, gc.owner) }; + state.maybe_collect(gc); +} + /// Get a reference to the GC state. /// /// In threading mode this is a true global (OnceLock). /// In non-threading mode this is thread-local, because PyRwLock/PyMutex /// use Cell-based locks that are not Sync. /// -/// The collector is process-wide rather than per-interpreter: every -/// interpreter's tracked objects live in these generation lists, so a -/// collection stops and collects across all of them, and `gc.disable()`, -/// thresholds, `gc.garbage` and `gc.get_objects()` observe process-wide state. -/// Making the collector per-interpreter additionally requires routing -/// `untrack_object` (called from `default_dealloc`, where no VM is in scope) to -/// the owning interpreter's lists. +/// Every interpreter's tracked objects live in these lists, because untracking +/// happens in `default_dealloc`, where no interpreter is in scope to route to. +/// What a collection *acts on* is still one interpreter's own objects, selected +/// by the `gc_owner` tag; [`GcInterpreterState`] holds the rest of the state +/// that goes with that. The counts here, and so `gc.get_count()` and +/// `gc.get_freeze_count()`, stay process-wide: they measure how full these +/// lists are. pub fn gc_state() -> &'static GcState { rustpython_common::static_cell! { static GC_STATE: GcState; @@ -1131,18 +1337,21 @@ pub fn gc_state() -> &'static GcState { mod tests { use super::*; + fn interpreter_state() -> GcInterpreterState { + GcInterpreterState::new(crate::vm::Context::genesis()) + } + #[test] fn gc_state_default() { - let state = GcState::new(); + let state = interpreter_state(); assert!(state.is_enabled()); assert_eq!(state.get_debug(), GcDebugFlags::empty()); assert_eq!(state.get_threshold(), (2000, 10, 0)); - assert_eq!(state.get_count(), (0, 0, 0)); } #[test] fn gc_enable_disable() { - let state = GcState::new(); + let state = interpreter_state(); assert!(state.is_enabled()); state.disable(); assert!(!state.is_enabled()); @@ -1152,18 +1361,29 @@ mod tests { #[test] fn gc_threshold() { - let state = GcState::new(); + let state = interpreter_state(); state.set_threshold(100, Some(20), Some(30)); assert_eq!(state.get_threshold(), (100, 20, 30)); } #[test] fn gc_debug_flags() { - let state = GcState::new(); + let state = interpreter_state(); state.set_debug(GcDebugFlags::STATS | GcDebugFlags::COLLECTABLE); assert_eq!( state.get_debug(), GcDebugFlags::STATS | GcDebugFlags::COLLECTABLE ); } + + /// Live interpreters never share an owner tag, or their collections would + /// reach each other's objects. + #[test] + fn gc_owner_tags_are_distinct_while_live() { + let first = interpreter_state(); + let second = interpreter_state(); + assert_ne!(first.owner, second.owner); + assert_ne!(first.owner, GC_NO_OWNER); + assert_ne!(second.owner, GC_NO_OWNER); + } } diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index 0ee7a062ee7..c81e636c3ea 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -303,6 +303,10 @@ bitflags::bitflags! { /// GC generation constants pub(crate) const GC_UNTRACKED: u8 = 0xFF; pub(crate) const GC_PERMANENT: u8 = 3; +/// `gc_owner` of an object that belongs to no single interpreter: everything +/// the shared context allocates, and anything allocated with no interpreter +/// current. Every interpreter collects these. +pub(crate) const GC_NO_OWNER: u32 = 0; /// Link implementation for GC intrusive linked list tracking pub(crate) struct GcLink; @@ -389,6 +393,10 @@ pub(super) struct PyInner { /// GC generation index (0-2=gen, GC_PERMANENT=permanent, GC_UNTRACKED=not tracked). /// Uses PyAtomic for interior mutability (writes happen through &self under list locks). pub(super) gc_generation: PyAtomic, + /// Interpreter that tracked this object, or `GC_NO_OWNER`. Written by + /// `track_object`; read to scope a collection to one interpreter. + /// Sits in what would otherwise be padding, so it costs no space. + pub(super) gc_owner: PyAtomic, /// Intrusive linked list pointers for GC generational tracking pub(super) gc_pointers: Pointers, @@ -398,6 +406,11 @@ pub(super) struct PyInner { } pub(crate) const SIZEOF_PYOBJECT_HEAD: usize = core::mem::size_of::>(); +// ref_count, vtable, gc_pointers (two) and typ are one word each; the gc bits, +// generation and owner share the word of padding their alignment forces. Adding +// to that group is free only while this holds. +const _: () = assert!(SIZEOF_PYOBJECT_HEAD == 6 * core::mem::size_of::()); + impl PyInner { /// Read type flags and member_count via raw pointers to avoid Stacked Borrows /// violations during bootstrap, where type objects have self-referential typ pointers. @@ -1216,6 +1229,7 @@ impl PyInner { vtable: PyObjVTable::of::(), gc_bits: Radium::new(0), gc_generation: Radium::new(GC_UNTRACKED), + gc_owner: Radium::new(GC_NO_OWNER), gc_pointers: Pointers::new(), typ: PyAtomicRef::from(typ), payload, @@ -1228,6 +1242,7 @@ impl PyInner { vtable: PyObjVTable::of::(), gc_bits: Radium::new(0), gc_generation: Radium::new(GC_UNTRACKED), + gc_owner: Radium::new(GC_NO_OWNER), gc_pointers: Pointers::new(), typ: PyAtomicRef::from(typ), payload, @@ -1734,6 +1749,20 @@ impl PyObject { self.0.gc_generation.store(generation, Ordering::Relaxed); } + /// The interpreter whose collections consider this object. + #[inline] + pub(crate) fn gc_owner(&self) -> u32 { + self.0.gc_owner.load(Ordering::Relaxed) + } + + /// Set the owning interpreter. Written by `track_object` before the object + /// enters a generation list, and reset to `GC_NO_OWNER` when the owning + /// interpreter goes away. + #[inline] + pub(crate) fn set_gc_owner(&self, owner: u32) { + self.0.gc_owner.store(owner, Ordering::Relaxed); + } + /// _PyObject_GC_TRACK #[inline] pub(crate) fn set_gc_tracked(&self) { @@ -2392,12 +2421,11 @@ impl PyRef { if (::HAS_TRAVERSE || has_dict || is_heaptype) && !T::NEW_REF_UNTRACKED { - let gc = crate::gc_state::gc_state(); + // Tracks under the interpreter running now and collects if this + // allocation pushed gen0 past its threshold. unsafe { - gc.track_object(ptr.cast()); + crate::gc_state::track_new_object(ptr.cast()); } - // Check if automatic GC should run - gc.maybe_collect(); } Self { ptr } @@ -2645,6 +2673,7 @@ pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef, PyTypeRef) { vtable: PyObjVTable::of::(), gc_bits: Radium::new(0), gc_generation: Radium::new(GC_UNTRACKED), + gc_owner: Radium::new(GC_NO_OWNER), gc_pointers: Pointers::new(), payload: type_payload, }, @@ -2660,6 +2689,7 @@ pub(crate) fn init_type_hierarchy() -> (PyTypeRef, PyTypeRef, PyTypeRef) { vtable: PyObjVTable::of::(), gc_bits: Radium::new(0), gc_generation: Radium::new(GC_UNTRACKED), + gc_owner: Radium::new(GC_NO_OWNER), gc_pointers: Pointers::new(), payload: object_payload, }, diff --git a/crates/vm/src/object/mod.rs b/crates/vm/src/object/mod.rs index b06957e1bc6..0ba4af17df1 100644 --- a/crates/vm/src/object/mod.rs +++ b/crates/vm/src/object/mod.rs @@ -9,5 +9,5 @@ pub use self::core::*; pub use self::ext::*; pub use self::payload::*; pub(crate) use core::SIZEOF_PYOBJECT_HEAD; -pub(crate) use core::{GC_PERMANENT, GC_UNTRACKED, GcLink}; +pub(crate) use core::{GC_NO_OWNER, GC_PERMANENT, GC_UNTRACKED, GcLink}; pub use traverse::{MaybeTraverse, Traverse, TraverseFn}; diff --git a/crates/vm/src/stdlib/gc.rs b/crates/vm/src/stdlib/gc.rs index b0007b4c867..af861862edb 100644 --- a/crates/vm/src/stdlib/gc.rs +++ b/crates/vm/src/stdlib/gc.rs @@ -23,20 +23,20 @@ mod gc { /// Enable automatic garbage collection. #[pyfunction] - fn enable() { - gc_state::gc_state().enable(); + fn enable(vm: &VirtualMachine) { + vm.state.gc.enable(); } /// Disable automatic garbage collection. #[pyfunction] - fn disable() { - gc_state::gc_state().disable(); + fn disable(vm: &VirtualMachine) { + vm.state.gc.disable(); } /// Return true if automatic gc is enabled. #[pyfunction] - fn isenabled() -> bool { - gc_state::gc_state().is_enabled() + fn isenabled(vm: &VirtualMachine) -> bool { + vm.state.gc.is_enabled() } /// Run a garbage collection. Returns the number of unreachable objects found. @@ -58,15 +58,14 @@ mod gc { invoke_callbacks(vm, "start", generation_num as usize, &Default::default()); // Manual gc.collect() should run even if GC is disabled - let gc = gc_state::gc_state(); + let gc = &vm.state.gc; let result = gc.collect_force(generation_num as usize); - // Move objects from gc_state.garbage to vm.ctx.gc_garbage (for DEBUG_SAVEALL) + // Publish what the collection saved as gc.garbage (for DEBUG_SAVEALL) { let mut state_garbage = gc.garbage.lock(); if !state_garbage.is_empty() { - let py_garbage = &vm.ctx.gc_garbage; - let mut garbage_vec = py_garbage.borrow_vec_mut(); + let mut garbage_vec = gc.py_garbage.borrow_vec_mut(); for obj in state_garbage.drain(..) { garbage_vec.push(obj); } @@ -82,7 +81,7 @@ mod gc { /// Return the current collection thresholds as a tuple. #[pyfunction] fn get_threshold(vm: &VirtualMachine) -> PyObjectRef { - let (t0, t1, t2) = gc_state::gc_state().get_threshold(); + let (t0, t1, t2) = vm.state.gc.get_threshold(); vm.ctx .new_tuple(vec![ vm.ctx.new_int(t0).into(), @@ -94,8 +93,13 @@ mod gc { /// Set the collection thresholds. #[pyfunction] - fn set_threshold(threshold0: u32, threshold1: OptionalArg, threshold2: OptionalArg) { - gc_state::gc_state().set_threshold( + fn set_threshold( + threshold0: u32, + threshold1: OptionalArg, + threshold2: OptionalArg, + vm: &VirtualMachine, + ) { + vm.state.gc.set_threshold( threshold0, threshold1.into_option(), threshold2.into_option(), @@ -117,20 +121,22 @@ mod gc { /// Return the current debugging flags. #[pyfunction] - fn get_debug() -> u32 { - gc_state::gc_state().get_debug().bits() + fn get_debug(vm: &VirtualMachine) -> u32 { + vm.state.gc.get_debug().bits() } /// Set the debugging flags. #[pyfunction] - fn set_debug(flags: u32) { - gc_state::gc_state().set_debug(gc_state::GcDebugFlags::from_bits_truncate(flags)); + fn set_debug(flags: u32, vm: &VirtualMachine) { + vm.state + .gc + .set_debug(gc_state::GcDebugFlags::from_bits_truncate(flags)); } /// Return a list of per-generation gc stats. #[pyfunction] fn get_stats(vm: &VirtualMachine) -> PyResult { - let stats = gc_state::gc_state().get_stats(); + let stats = vm.state.gc.get_stats(); let mut result = Vec::with_capacity(3); for stat in &stats { @@ -165,7 +171,7 @@ mod gc { { return Err(vm.new_value_error(format!("generation must be in range(0, 3), not {g}"))); } - let objects = gc_state::gc_state().get_objects(generation_opt); + let objects = vm.state.gc.get_objects(generation_opt); Ok(vm.ctx.new_list(objects)) } @@ -208,7 +214,7 @@ mod gc { let mut result = Vec::new(); // Scan all tracked objects across all generations - let all_objects = gc_state::gc_state().get_objects(None); + let all_objects = vm.state.gc.get_objects(None); for obj in all_objects { let obj_ptr = obj.as_ref() as *const crate::PyObject as usize; if stack_frames.contains(&obj_ptr) { @@ -241,14 +247,14 @@ mod gc { /// Freeze all objects tracked by gc. #[pyfunction] - fn freeze() { - gc_state::gc_state().freeze(); + fn freeze(vm: &VirtualMachine) { + vm.state.gc.freeze(); } /// Unfreeze all objects in the permanent generation. #[pyfunction] - fn unfreeze() { - gc_state::gc_state().unfreeze(); + fn unfreeze(vm: &VirtualMachine) { + vm.state.gc.unfreeze(); } /// Return the number of objects in the permanent generation. @@ -260,13 +266,13 @@ mod gc { /// gc.garbage - list of uncollectable objects #[pyattr] fn garbage(vm: &VirtualMachine) -> PyListRef { - vm.ctx.gc_garbage.clone() + vm.state.gc.py_garbage.clone() } /// gc.callbacks - list of callbacks to be invoked #[pyattr] fn callbacks(vm: &VirtualMachine) -> PyListRef { - vm.ctx.gc_callbacks.clone() + vm.state.gc.py_callbacks.clone() } /// Helper function to invoke GC callbacks @@ -276,7 +282,7 @@ mod gc { generation: usize, result: &gc_state::CollectResult, ) { - let callbacks_list = &vm.ctx.gc_callbacks; + let callbacks_list = &vm.state.gc.py_callbacks; let callbacks: Vec = callbacks_list.borrow_vec().to_vec(); if callbacks.is_empty() { return; diff --git a/crates/vm/src/stdlib/posix.rs b/crates/vm/src/stdlib/posix.rs index aaea11acca2..f4d5a70db34 100644 --- a/crates/vm/src/stdlib/posix.rs +++ b/crates/vm/src/stdlib/posix.rs @@ -732,8 +732,10 @@ pub mod module { // Codec registry RwLock vm.state.codec_registry.reinit_after_fork(); - // GC state (multiple Mutex + RwLock) + // GC state (multiple Mutex + RwLock), shared lists and this + // interpreter's own policy state. crate::gc_state::gc_state().reinit_after_fork(); + vm.state.gc.reinit_after_fork(); // Import lock (RawReentrantMutex) crate::stdlib::_imp::reinit_imp_lock_after_fork(); @@ -773,6 +775,7 @@ pub mod module { reinit_mutex_after_fork(&state.shutdown_handles); state.codec_registry.reinit_after_fork(); + state.gc.reinit_after_fork(); } state.stop_the_world.reset_after_fork(); diff --git a/crates/vm/src/vm/context.rs b/crates/vm/src/vm/context.rs index 9a545663576..08efde1b7b4 100644 --- a/crates/vm/src/vm/context.rs +++ b/crates/vm/src/vm/context.rs @@ -54,10 +54,7 @@ pub struct Context { pub(crate) string_pool: StringPool, pub(crate) slot_new_wrapper: PyMethodDef, pub names: ConstName, - // GC module state (callbacks and garbage lists) - pub gc_callbacks: PyListRef, - pub gc_garbage: PyListRef, } macro_rules! declare_const_name { @@ -363,8 +360,6 @@ impl Context { let empty_bytes = create_object(PyBytes::from(Vec::new()), types.bytes_type); // GC callbacks and garbage lists - let gc_callbacks = PyRef::new_ref(PyList::default(), types.list_type.to_owned(), None); - let gc_garbage = PyRef::new_ref(PyList::default(), types.list_type.to_owned(), None); Self { true_value, @@ -387,9 +382,6 @@ impl Context { string_pool, slot_new_wrapper, names, - - gc_callbacks, - gc_garbage, } } diff --git a/crates/vm/src/vm/interpreter.rs b/crates/vm/src/vm/interpreter.rs index 2bb08e80bc0..20ab4681051 100644 --- a/crates/vm/src/vm/interpreter.rs +++ b/crates/vm/src/vm/interpreter.rs @@ -151,6 +151,7 @@ where // Create PyGlobalState (≈ PyInterpreterState) let global_state = PyRc::new(PyGlobalState { + gc: crate::gc_state::GcInterpreterState::new(&ctx), interpreter_id, whence, is_main, @@ -617,7 +618,7 @@ impl Interpreter { vm.state.finalizing.store(true, Ordering::Release); // GC pass - collect cycles before module cleanup - crate::gc_state::gc_state().collect_force(2); + vm.state.gc.collect_force(2); // Module finalization: remove modules from sys.modules, GC collect // (while builtins is still available for __del__), then clear module dicts. @@ -1203,19 +1204,18 @@ mod tests { /// Subclassing a shared type records the subclass on an object every /// interpreter reaches, but only the interpreter that created it lists it. + fn run(vm: &VirtualMachine, scope: &crate::scope::Scope, source: &str) { + let code = vm + .compile(source, crate::compiler::Mode::Exec, "") + .map_err(|err| err.into_pyexception(vm, Some(source))) + .unwrap(); + vm.run_code_obj(code, scope.clone()).unwrap(); + } + #[test] fn subinterpreter_subclasses_are_scoped_to_their_interpreter() { - use crate::compiler::Mode; use crate::scope::Scope; - fn run(vm: &VirtualMachine, scope: &Scope, source: &str) { - let code = vm - .compile(source, Mode::Exec, "") - .map_err(|err| err.into_pyexception(vm, Some(source))) - .unwrap(); - vm.run_code_obj(code, scope.clone()).unwrap(); - } - fn lists_subclass(vm: &VirtualMachine, scope: &Scope, name: &str) -> bool { run( vm, @@ -1259,6 +1259,97 @@ mod tests { sub.enter(|_| drop(sub_scope)); } + /// A cycle allocated in one interpreter is not the parent's to collect. + #[test] + fn collections_only_reach_the_collecting_interpreter() { + use core::time::Duration; + use std::time::Instant; + + const CYCLE: &str = "class Node:\n pass\n\ + a = Node()\n\ + b = Node()\n\ + a.other = b\n\ + b.other = a\n\ + del a\n\ + del b\n"; + + fn live_nodes(vm: &VirtualMachine) -> usize { + vm.state + .gc + .get_objects(None) + .iter() + .filter(|obj| &*obj.class().name() == "Node") + .count() + } + + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + + let sub_scope = sub.enter(|vm| { + let scope = vm.new_scope_with_builtins(); + run(vm, &scope, CYCLE); + assert_eq!(live_nodes(vm), 2); + scope + }); + + // A collection in the parent walks its own tracked objects and leaves + // the sub's cycle where it is. Collections are serialized process-wide + // by a `try_lock`, so one running elsewhere in the suite makes + // `collect_force` a no-op; retry until this one gets to run. Each retry + // waits outside `enter`, since a thread that is entered but not running + // bytecode never reaches a safepoint, and the collection this is + // waiting for cannot stop it. + let deadline = Instant::now() + Duration::from_secs(30); + while !main.enter(|vm| vm.state.gc.collect_force(2).candidates > 0) { + assert!( + Instant::now() < deadline, + "no collection ran in the parent interpreter" + ); + std::thread::sleep(Duration::from_millis(5)); + } + sub.enter(|vm| assert_eq!(live_nodes(vm), 2)); + + sub.enter(|_| drop(sub_scope)); + } + + /// And it is not the parent's to enumerate either. + #[test] + fn get_objects_only_reports_the_calling_interpreter() { + fn tracks_class(vm: &VirtualMachine, name: &str) -> bool { + vm.state + .gc + .get_objects(None) + .iter() + .any(|obj| &*obj.class().name() == name) + } + + let main = Interpreter::without_stdlib(Default::default()); + let sub = main.create_subinterpreter(); + + let main_scope = main.enter(|vm| { + let scope = vm.new_scope_with_builtins(); + run(vm, &scope, "class MainNode:\n pass\nkeep = MainNode()\n"); + scope + }); + let sub_scope = sub.enter(|vm| { + let scope = vm.new_scope_with_builtins(); + run(vm, &scope, "class SubNode:\n pass\nkeep = SubNode()\n"); + scope + }); + + main.enter(|vm| { + assert!(tracks_class(vm, "MainNode")); + assert!(!tracks_class(vm, "SubNode")); + }); + sub.enter(|vm| { + assert!(tracks_class(vm, "SubNode")); + assert!(!tracks_class(vm, "MainNode")); + }); + + main.enter(|_| drop(main_scope)); + sub.enter(|_| drop(sub_scope)); + } + /// The runtime can own a subinterpreter by id and hand it back on destroy. #[cfg(feature = "threading")] #[test] @@ -1356,7 +1447,7 @@ for _ in range(40): let deadline = Instant::now() + Duration::from_secs(2); let mut collections = 0; while Instant::now() < deadline && collections < 20 { - crate::gc_state::gc_state().collect_force(2); + vm.state.gc.collect_force(2); collections += 1; } assert!(collections > 0); diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index 84a1efb4568..13d402a1702 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -795,6 +795,8 @@ pub struct PyGlobalState { /// Stop-the-world state for pre-fork thread suspension #[cfg(feature = "threading")] pub stop_the_world: StopTheWorldState, + /// This interpreter's garbage collector policy and results. + pub gc: crate::gc_state::GcInterpreterState, } impl PyGlobalState { @@ -1812,14 +1814,14 @@ impl VirtualMachine { // Phase 4: GC collect — modules removed from sys.modules are freed, // exposing cycles (e.g., dict ↔ function.__globals__). GC collects // these and calls __del__ while module dicts are still intact. - crate::gc_state::gc_state().collect_force(2); + self.state.gc.collect_force(2); // Phase 5: Clear module dicts in reverse import order using 2-pass algorithm. // Skip builtins and sys — those are cleared last. self.finalize_clear_module_dicts(&module_weakrefs); // Phase 6: GC collect — pick up anything freed by dict clearing. - crate::gc_state::gc_state().collect_force(2); + self.state.gc.collect_force(2); // Phase 7: Clear sys and builtins dicts last self.finalize_clear_sys_builtins_dict(); @@ -2329,8 +2331,10 @@ impl VirtualMachine { if mat_ptr != 0 { let fo = unsafe { &*(mat_ptr as *const crate::Py) }; unsafe { - crate::gc_state::gc_state() - .track_object(core::ptr::NonNull::from(fo.as_object())); + crate::gc_state::gc_state().track_object( + core::ptr::NonNull::from(fo.as_object()), + crate::gc_state::current_owner(), + ); let live_iframe = &*iframe_ptr; live_iframe.cold().temporary_refs.lock().clear(); } @@ -2809,7 +2813,7 @@ impl VirtualMachine { #[cfg(feature = "threading")] pub(crate) fn run_scheduled_gc(&self) { if crate::signal::take_gc_scheduled() { - crate::gc_state::gc_state().collect(0); + self.state.gc.collect(0); } } diff --git a/crates/vm/src/vm/thread.rs b/crates/vm/src/vm/thread.rs index 5fb40601cff..b69a244f639 100644 --- a/crates/vm/src/vm/thread.rs +++ b/crates/vm/src/vm/thread.rs @@ -159,6 +159,26 @@ fn set_current_vm(vm: &VirtualMachine, f: impl FnOnce() -> R) -> R { }) } +/// Pointer to the GC state of the interpreter running on this thread. +/// +/// The pointee belongs to the `PyGlobalState` of the VM on top of `VM_STACK`, +/// which is borrowed for the whole `set_current_vm` scope — so the pointer stays +/// valid as long as the caller remains inside that scope. +pub(crate) fn current_gc_state() -> Option> { + // Reached from every tracked allocation, including ones a thread-local + // destructor makes while the VM stack is being torn down, so neither a + // destroyed key nor an outstanding borrow may panic here. + VM_STACK + .try_with(|vms| { + let vm = vms.try_borrow().ok()?.last().copied()?; + // SAFETY: entries in VM_STACK either borrow a VM for the dynamic + // scope of a set_current_vm()/enter_vm() call or point at GILSTATE_VM. + Some(NonNull::from(&unsafe { vm.as_ref() }.state.gc)) + }) + .ok() + .flatten() +} + pub fn try_with_current_vm(f: impl FnOnce(&VirtualMachine) -> R) -> Option { VM_STACK.with(|vms| { let vm = vms.borrow().last().copied()?; From 30e202019e2ba90e33ccffdbdded9c8fece154c2 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 14 Aug 2026 19:03:06 +0900 Subject: [PATCH 08/20] _queue, _thread: detach before taking locks held across waits `_queue.Semaphore` holds its mutex across the `allow_threads` condvar wait, and `join_internal` holds a thread handle's completion mutex the same way. Stop-the-world can stop a thread while it holds either one. The remaining acquisitions ran attached, so a thread blocking on such a mutex had no safepoint left to reach: the stop never completed, and the holder was never resumed to release it. Route those acquisitions through helpers that detach first. The fork-child reinit paths keep their direct locks. Assisted-by: Claude --- crates/stdlib/src/_queue.rs | 33 ++++++++++++++++++++++++--------- crates/vm/src/stdlib/_thread.rs | 27 ++++++++++++++++++++------- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/crates/stdlib/src/_queue.rs b/crates/stdlib/src/_queue.rs index 1c8a4b0b21b..96e77a34a0b 100644 --- a/crates/stdlib/src/_queue.rs +++ b/crates/stdlib/src/_queue.rs @@ -74,9 +74,20 @@ mod _queue { } } - fn release(&self) { + /// Take `mutex`, detaching first so that blocking on it cannot stall a + /// stop-the-world request. + /// + /// A waiter holds this mutex across its `allow_threads` wait, so it can + /// still hold it when it is stopped. An attached thread blocking on it + /// would then never reach a safepoint, the stop would never complete, + /// and the holder would never be resumed to release it. + fn lock_count(&self, vm: &VirtualMachine) -> parking_lot::MutexGuard<'_, usize> { + vm.allow_threads(|| self.mutex.lock()) + } + + fn release(&self, vm: &VirtualMachine) { { - let mut count = self.mutex.lock(); + let mut count = self.lock_count(vm); *count += 1; } // lock dropped. now we can notify a waiting thread @@ -95,7 +106,7 @@ mod _queue { // Guard must be dropped before check_signals() below, since a // signal handler may call back into this same queue. { - let mut count = self.mutex.lock(); + let mut count = self.lock_count(vm); if *count > 0 { *count -= 1; @@ -151,11 +162,15 @@ mod _queue { } impl PySimpleQueue { - fn push(&self, item: PyObjectRef) { + #[cfg_attr( + not(feature = "threading"), + expect(unused_variables, reason = "only the semaphore needs the vm") + )] + fn push(&self, item: PyObjectRef, vm: &VirtualMachine) { self.buf.lock().push_back(item); #[cfg(feature = "threading")] - self.sem.release(); + self.sem.release(vm); } /// Returns a strong reference from the head of the buffer. @@ -221,14 +236,14 @@ mod _queue { } #[pymethod] - fn put(&self, args: PutArgs) { + fn put(&self, args: PutArgs, vm: &VirtualMachine) { let PutArgs { item, .. } = args; - self.push(item); + self.push(item, vm); } #[pymethod] - fn put_nowait(&self, item: PyObjectRef) { - self.push(item); + fn put_nowait(&self, item: PyObjectRef, vm: &VirtualMachine) { + self.push(item, vm); } #[pymethod] diff --git a/crates/vm/src/stdlib/_thread.rs b/crates/vm/src/stdlib/_thread.rs index 09866acd224..2c588a6597f 100644 --- a/crates/vm/src/stdlib/_thread.rs +++ b/crates/vm/src/stdlib/_thread.rs @@ -1385,6 +1385,19 @@ pub(crate) mod _thread { } } + /// Take a thread handle's completion mutex, detaching first. + /// + /// A joiner holds this mutex across its `allow_threads` wait, so it can + /// still hold it when stop-the-world stops it. An attached thread that + /// blocked on it would never reach a safepoint, so the stop could never + /// complete and the holder would never be resumed to release it. + fn lock_done<'a>( + lock: &'a parking_lot::Mutex, + vm: &VirtualMachine, + ) -> parking_lot::MutexGuard<'a, bool> { + vm.allow_threads(|| lock.lock()) + } + /// Reset a parking_lot::Mutex to unlocked state after fork. #[cfg(all(unix, feature = "host_env"))] fn reinit_parking_lot_mutex(mutex: &parking_lot::Mutex) { @@ -1467,7 +1480,7 @@ pub(crate) mod _thread { // Wait for thread completion using Condvar (supports timeout) // Loop to handle spurious wakeups let (lock, cvar) = &**done_event; - let mut done = lock.lock(); + let mut done = lock_done(lock, vm); // ThreadHandle_join semantics: self-join/finalizing checks // apply only while target thread has not reported it is exiting yet. @@ -1527,7 +1540,7 @@ pub(crate) mod _thread { drop(inner_guard); // Wait on done_event let (lock, cvar) = &**done_event; - let mut done = lock.lock(); + let mut done = lock_done(lock, vm); while !*done { vm.allow_threads(|| cvar.wait(&mut done)); } @@ -1589,7 +1602,7 @@ pub(crate) mod _thread { remove_from_shutdown_handles(vm, inner, done_event); let (lock, cvar) = &**done_event; - *lock.lock() = true; + *lock_done(lock, vm) = true; cvar.notify_all(); Ok(()) } @@ -1659,7 +1672,7 @@ pub(crate) mod _thread { // before returning True. let done = { let (lock, _) = &*self.done_event; - *lock.lock() + *lock_done(lock, vm) }; if !done { return Ok(false); @@ -1855,7 +1868,7 @@ pub(crate) mod _thread { // Starting a handle always resets the completion event. { let (done_lock, _) = &*handle.done_event; - *done_lock.lock() = false; + *lock_done(done_lock, vm) = false; } // Add non-daemon threads to shutdown registry so _shutdown() will wait for them @@ -1937,7 +1950,7 @@ pub(crate) mod _thread { // This must be LAST to ensure all cleanup is complete before join() returns { let (lock, cvar) = &*done_event_for_cleanup; - *lock.lock() = true; + *lock_done(lock, vm) = true; cvar.notify_all(); } } @@ -1972,7 +1985,7 @@ pub(crate) mod _thread { } { let (done_lock, done_cvar) = &*handle.done_event; - *done_lock.lock() = true; + *lock_done(done_lock, vm) = true; done_cvar.notify_all(); } if !daemon { From 5ce062e18758fa9c3afe165ccb60cb88c1379d11 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Fri, 14 Aug 2026 23:58:23 +0900 Subject: [PATCH 09/20] _io, _winapi: detach on the remaining stopped-holdable lock takes `TextIOWrapper.__repr__` took `data` directly while every other method takes it through `lock_opt`, which detaches. `Overlapped` holds `inner` across the `allow_threads` in `GetOverlappedResult`, and all four of its takes were direct. A thread stopped by stop-the-world can be holding either mutex, so taking one while attached left the blocked thread with no safepoint to reach. Assisted-by: Claude --- crates/vm/src/stdlib/_io.rs | 5 ++++- crates/vm/src/stdlib/_winapi.rs | 21 +++++++++++++++------ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/crates/vm/src/stdlib/_io.rs b/crates/vm/src/stdlib/_io.rs index ab1be4297ec..80085a82290 100644 --- a/crates/vm/src/stdlib/_io.rs +++ b/crates/vm/src/stdlib/_io.rs @@ -4078,7 +4078,10 @@ mod _io { vm.new_runtime_error(format!("reentrant call inside {type_name}.__repr__")) ); }; - let Some(data) = zelf.data.lock() else { + // Detach while blocked, like `lock_opt`: another thread can be + // stopped holding this mutex, and blocking on it while attached + // would leave no safepoint for that stop to complete at. + let Some(data) = zelf.data.lock_wrapped(|do_lock| vm.allow_threads(do_lock)) else { // Reentrant call return Ok(vm.ctx.new_str(Wtf8Buf::from(format!("<{type_name}>")))); }; diff --git a/crates/vm/src/stdlib/_winapi.rs b/crates/vm/src/stdlib/_winapi.rs index 0d54530d4b2..34e7b897e12 100644 --- a/crates/vm/src/stdlib/_winapi.rs +++ b/crates/vm/src/stdlib/_winapi.rs @@ -8,7 +8,7 @@ mod _winapi { use crate::{ Py, PyObjectRef, PyPayload, PyResult, TryFromObject, VirtualMachine, builtins::PyStrRef, - common::lock::PyMutex, + common::lock::{PyMutex, PyMutexGuard}, convert::ToPyException, function::{ArgMapping, ArgSequence, OptionalArg}, types::Constructor, @@ -566,9 +566,18 @@ mod _winapi { .map_err(|e| e.to_pyexception(vm)) } + /// Take `inner`, detaching while blocked. + /// + /// `GetOverlappedResult` holds this mutex across its `allow_threads` + /// wait, so a stopped thread can still be holding it. Blocking on it + /// while attached would leave no safepoint for that stop to complete at. + fn lock_inner(&self, vm: &VirtualMachine) -> PyMutexGuard<'_, host_overlapped::Operation> { + vm.allow_threads(|| self.inner.lock()) + } + #[pymethod] fn GetOverlappedResult(&self, wait: bool, vm: &VirtualMachine) -> PyResult<(u32, u32)> { - let mut inner = self.inner.lock(); + let mut inner = self.lock_inner(vm); vm.allow_threads(|| inner.get_result(wait)) .map(|result| (result.transferred, result.error)) .map_err(|e| e.to_pyexception(vm)) @@ -576,7 +585,7 @@ mod _winapi { #[pymethod] fn getbuffer(&self, vm: &VirtualMachine) -> PyResult> { - let inner = self.inner.lock(); + let inner = self.lock_inner(vm); if !inner.is_completed() { return Err(vm.new_value_error( "can't get read buffer before GetOverlappedResult() signals the operation completed", @@ -589,13 +598,13 @@ mod _winapi { #[pymethod] fn cancel(&self, vm: &VirtualMachine) -> PyResult<()> { - let mut inner = self.inner.lock(); + let mut inner = self.lock_inner(vm); inner.cancel().map_err(|e| e.to_pyexception(vm)) } #[pygetset] - fn event(&self) -> isize { - let inner = self.inner.lock(); + fn event(&self, vm: &VirtualMachine) -> isize { + let inner = self.lock_inner(vm); inner.event() as isize } } From 87b6c6852f0419e958fff8707c3da4d04d6d7f15 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sat, 15 Aug 2026 10:41:21 +0900 Subject: [PATCH 10/20] gc: size the interpreter owner tag to the header padding `gc_owner` was a u32. With 4-byte pointers its alignment pushed it out of the padding that follows the gc bits and generation, growing every object by a word and tripping the `SIZEOF_PYOBJECT_HEAD` assertion on 32-bit targets. Introduce `GcOwner = u16`, which the `repr(C)` layout places at offset 10 on 32-bit and 18 on 64-bit, leaving the header at 6 words on both. Tags now run out after 65535 interpreters; `alloc_owner` already falls back to `GC_NO_OWNER`, so an interpreter past that collects as it did before tagging. Also widen three test deadlines that measure liveness, not speed. Assisted-by: Claude --- crates/vm/src/gc_state.rs | 30 +++++++++++++++--------------- crates/vm/src/object/core.rs | 16 ++++++++++++---- crates/vm/src/object/mod.rs | 2 +- crates/vm/src/vm/interpreter.rs | 6 +++--- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/crates/vm/src/gc_state.rs b/crates/vm/src/gc_state.rs index de249e67968..a7e39d697fd 100644 --- a/crates/vm/src/gc_state.rs +++ b/crates/vm/src/gc_state.rs @@ -4,10 +4,10 @@ use crate::common::linked_list::LinkedList; use crate::common::lock::{PyMutex, PyRwLock}; -use crate::object::{GC_NO_OWNER, GC_PERMANENT, GC_UNTRACKED, GcLink}; +use crate::object::{GC_NO_OWNER, GC_PERMANENT, GC_UNTRACKED, GcLink, GcOwner}; use crate::{AsObject, PyObject, PyObjectRef}; use core::ptr::NonNull; -use core::sync::atomic::{AtomicBool, AtomicU32, AtomicUsize, Ordering}; +use core::sync::atomic::{AtomicBool, AtomicU16, AtomicU32, AtomicUsize, Ordering}; use std::collections::HashSet; fn elapsed_secs( @@ -143,7 +143,7 @@ fn release_count(count: &AtomicUsize) { /// /// Objects with no owner — everything the shared context allocates, and anything /// allocated with no interpreter current — belong to all of them. -fn is_owned_by(obj: &PyObject, owner: u32) -> bool { +fn is_owned_by(obj: &PyObject, owner: GcOwner) -> bool { let obj_owner = obj.gc_owner(); obj_owner == owner || obj_owner == GC_NO_OWNER } @@ -262,11 +262,11 @@ pub struct GcState { /// Allocation counter for gen0 alloc_count: AtomicUsize, /// Next `gc_owner` tag to hand to an interpreter. - next_owner: AtomicU32, + next_owner: AtomicU16, /// Tags of interpreters that are gone. Their objects outlived them, so a /// collection adopts them — tags them `GC_NO_OWNER` again — as it walks, /// rather than leaving them for a collector that will never come. - retired: PyMutex>, + retired: PyMutex>, } // SAFETY: All fields are either inherently Send/Sync (atomics, RwLock, Mutex) or protected by PyMutex. @@ -300,7 +300,7 @@ impl GcState { permanent_count: AtomicUsize::new(0), collecting: PyMutex::new(()), alloc_count: AtomicUsize::new(0), - next_owner: AtomicU32::new(GC_NO_OWNER + 1), + next_owner: AtomicU16::new(GC_NO_OWNER + 1), retired: PyMutex::new(Vec::new()), } } @@ -308,7 +308,7 @@ impl GcState { /// Reserve a tag for a new interpreter. Tags are never reused; exhausting /// the 32-bit space falls back to `GC_NO_OWNER`, which costs isolation but /// stays correct, rather than aliasing a live interpreter. - fn alloc_owner(&self) -> u32 { + fn alloc_owner(&self) -> GcOwner { self.next_owner .fetch_update(Ordering::Relaxed, Ordering::Relaxed, |next| { next.checked_add(1) @@ -320,7 +320,7 @@ impl GcState { /// whatever it left behind. Retagging the objects here would mean walking /// every list under an interpreter drop, which happens while a collection /// holds the collecting lock. - fn retire_owner(&self, owner: u32) { + fn retire_owner(&self, owner: GcOwner) { if owner == GC_NO_OWNER { return; } @@ -343,7 +343,7 @@ impl GcState { /// /// # Safety /// obj must be a valid pointer to a PyObject - pub unsafe fn track_object(&self, obj: NonNull, owner: u32) { + pub unsafe fn track_object(&self, obj: NonNull, owner: GcOwner) { let obj_ref = unsafe { obj.as_ref() }; obj_ref.set_gc_tracked(); obj_ref.set_gc_generation(0); @@ -407,10 +407,10 @@ impl GcState { /// interpreter owns. /// If generation is None, returns all such objects. /// If generation is Some(n), returns those in generation n only. - pub fn get_objects(&self, generation: Option, owner: u32) -> Vec { + pub fn get_objects(&self, generation: Option, owner: GcOwner) -> Vec { fn collect_from_list( list: &LinkedList, - owner: u32, + owner: GcOwner, ) -> impl Iterator + '_ { list.iter() .filter(move |obj| is_owned_by(obj, owner)) @@ -1060,7 +1060,7 @@ impl GcState { /// Freeze the objects `owner` could collect (move them to the permanent /// generation). /// Lock order: generation_lists[i] → permanent_list (consistent with unfreeze). - fn freeze(&self, owner: u32) { + fn freeze(&self, owner: GcOwner) { let mut count = 0usize; for (gen_idx, gen_list) in self.generation_lists.iter().enumerate() { @@ -1087,7 +1087,7 @@ impl GcState { /// Unfreeze the objects `owner` froze (move them from permanent to gen2). /// Lock order: generation_lists[2] → permanent_list (consistent with freeze). - fn unfreeze(&self, owner: u32) { + fn unfreeze(&self, owner: GcOwner) { let mut count = 0usize; { @@ -1148,7 +1148,7 @@ impl GcState { /// objects end up. pub struct GcInterpreterState { /// Tag written into every object this interpreter tracks. - owner: u32, + owner: GcOwner, /// Per-generation thresholds and statistics. pub generations: [GcGeneration; 3], /// GC enabled flag @@ -1288,7 +1288,7 @@ impl Drop for GcInterpreterState { /// The tag `track_object` should write for the interpreter running now. #[must_use] -pub fn current_owner() -> u32 { +pub fn current_owner() -> GcOwner { // SAFETY: the pointee is owned by the `PyGlobalState` of the VM on top of // this thread's VM stack, which outlives the section this call runs in. crate::vm::thread::current_gc_state().map_or(GC_NO_OWNER, |gc| unsafe { gc.as_ref() }.owner) diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index c81e636c3ea..6f8e3090e44 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -303,10 +303,18 @@ bitflags::bitflags! { /// GC generation constants pub(crate) const GC_UNTRACKED: u8 = 0xFF; pub(crate) const GC_PERMANENT: u8 = 3; +/// Width of an interpreter's `gc_owner` tag. +/// +/// Sized to the padding the header alignment already forces, so the tag costs +/// no space on either pointer width. Running out of tags is not an error: an +/// interpreter that gets none uses [`GC_NO_OWNER`] and its objects stay +/// collectable by every interpreter, which is how they behaved before tagging. +pub(crate) type GcOwner = u16; + /// `gc_owner` of an object that belongs to no single interpreter: everything /// the shared context allocates, and anything allocated with no interpreter /// current. Every interpreter collects these. -pub(crate) const GC_NO_OWNER: u32 = 0; +pub(crate) const GC_NO_OWNER: GcOwner = 0; /// Link implementation for GC intrusive linked list tracking pub(crate) struct GcLink; @@ -396,7 +404,7 @@ pub(super) struct PyInner { /// Interpreter that tracked this object, or `GC_NO_OWNER`. Written by /// `track_object`; read to scope a collection to one interpreter. /// Sits in what would otherwise be padding, so it costs no space. - pub(super) gc_owner: PyAtomic, + pub(super) gc_owner: PyAtomic, /// Intrusive linked list pointers for GC generational tracking pub(super) gc_pointers: Pointers, @@ -1751,7 +1759,7 @@ impl PyObject { /// The interpreter whose collections consider this object. #[inline] - pub(crate) fn gc_owner(&self) -> u32 { + pub(crate) fn gc_owner(&self) -> GcOwner { self.0.gc_owner.load(Ordering::Relaxed) } @@ -1759,7 +1767,7 @@ impl PyObject { /// enters a generation list, and reset to `GC_NO_OWNER` when the owning /// interpreter goes away. #[inline] - pub(crate) fn set_gc_owner(&self, owner: u32) { + pub(crate) fn set_gc_owner(&self, owner: GcOwner) { self.0.gc_owner.store(owner, Ordering::Relaxed); } diff --git a/crates/vm/src/object/mod.rs b/crates/vm/src/object/mod.rs index 0ba4af17df1..becfcabb1d4 100644 --- a/crates/vm/src/object/mod.rs +++ b/crates/vm/src/object/mod.rs @@ -9,5 +9,5 @@ pub use self::core::*; pub use self::ext::*; pub use self::payload::*; pub(crate) use core::SIZEOF_PYOBJECT_HEAD; -pub(crate) use core::{GC_NO_OWNER, GC_PERMANENT, GC_UNTRACKED, GcLink}; +pub(crate) use core::{GC_NO_OWNER, GC_PERMANENT, GC_UNTRACKED, GcLink, GcOwner}; pub use traverse::{MaybeTraverse, Traverse, TraverseFn}; diff --git a/crates/vm/src/vm/interpreter.rs b/crates/vm/src/vm/interpreter.rs index 20ab4681051..d6700531dde 100644 --- a/crates/vm/src/vm/interpreter.rs +++ b/crates/vm/src/vm/interpreter.rs @@ -823,7 +823,7 @@ mod tests { use core::time::Duration; use std::time::Instant; - let deadline = Instant::now() + Duration::from_secs(5); + let deadline = Instant::now() + Duration::from_secs(30); while runtime::lookup_interpreter(id).is_some() { assert!( Instant::now() < deadline, @@ -1069,7 +1069,7 @@ mod tests { let sub_worker = spawn_worker(&sub); let (lock, ready) = &*state; - let deadline = Instant::now() + Duration::from_secs(2); + let deadline = Instant::now() + Duration::from_secs(30); let mut state_guard = lock.lock().unwrap(); while state_guard.entered < 2 { let now = Instant::now(); @@ -1115,7 +1115,7 @@ mod tests { std::thread::spawn(move || { thread_vm.run(|vm| { main_started_worker.store(true, Ordering::Release); - let deadline = Instant::now() + Duration::from_secs(2); + let deadline = Instant::now() + Duration::from_secs(30); let mut operations = 0; while !sub_finished_worker.load(Ordering::Acquire) && Instant::now() < deadline { From fd279028934778d48372d1e4f015b869ba2cf550 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sat, 15 Aug 2026 16:19:44 +0900 Subject: [PATCH 11/20] vm: reuse cleared frame blocks and shorten interpreter hot paths - `datastack` remembers the most recently popped frame block. `push_frame` reports an exact LIFO reuse, and `setup_datastack_frame` then skips zero-filling localsplus. - Small-int loads push the context's cached int as a borrowed stack reference instead of taking a new one. - Calls to jitted functions go straight to `execute_call_vectorcall`. - Binary-op specialization reads ints through the new `PyInt::try_to_i64_fast` instead of the generic primitive conversion, and `try_to_bool` moves its non-bool path into a `#[cold]` helper. - Dict caches read an entry through a keys-version stamp (`get_index_if_keys_version`) rather than an entry-index hint. - Frame publishing caches a pointer to `ThreadSlot::top_iframe` in a thread-local `Cell` instead of borrowing `CURRENT_THREAD_SLOT`. --- crates/vm/src/builtins/bool.rs | 7 + crates/vm/src/builtins/dict.rs | 24 ++-- crates/vm/src/builtins/function.rs | 32 ++++- crates/vm/src/builtins/int.rs | 16 +++ crates/vm/src/datastack.rs | 47 ++++++- crates/vm/src/dict_inner.rs | 40 +++--- crates/vm/src/frame.rs | 198 +++++++++++++++++++---------- crates/vm/src/vm/context.rs | 8 ++ crates/vm/src/vm/mod.rs | 154 +++++++++++----------- crates/vm/src/vm/thread.rs | 51 +++++--- 10 files changed, 371 insertions(+), 206 deletions(-) diff --git a/crates/vm/src/builtins/bool.rs b/crates/vm/src/builtins/bool.rs index 4bb980d71a2..1cfa8cc27ee 100644 --- a/crates/vm/src/builtins/bool.rs +++ b/crates/vm/src/builtins/bool.rs @@ -34,6 +34,7 @@ impl<'a> TryFromBorrowedObject<'a> for bool { impl PyObjectRef { /// Convert Python bool into Rust bool. + #[inline(always)] pub fn try_to_bool(self, vm: &VirtualMachine) -> PyResult { if self.is(&vm.ctx.true_value) { return Ok(true); @@ -41,6 +42,12 @@ impl PyObjectRef { return Ok(false); } + self.try_to_bool_slow(vm) + } + + #[cold] + #[inline(never)] + fn try_to_bool_slow(self, vm: &VirtualMachine) -> PyResult { let slots = &self.class().slots; // 1. Try nb_bool slot first diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index 1a380d74d02..0acc3e95f89 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -114,11 +114,6 @@ impl PyDict { &self.entries } - /// Monotonically increasing version for mutation tracking. - pub(crate) fn version(&self) -> u64 { - self.entries.version() - } - /// Returns all keys as a Vec, atomically under a single read lock. /// Thread-safe: prevents "dictionary changed size during iteration" errors. pub fn keys_vec(&self) -> Vec { @@ -821,18 +816,15 @@ impl Py { } } - /// Fast lookup using a cached entry index hint. - pub(crate) fn get_item_opt_hint( + /// Read a cached exact-dict entry after validating its key-layout stamp. + #[inline] + pub(crate) fn get_item_by_index_and_keys_version( &self, - key: &K, - hint: u16, - vm: &VirtualMachine, - ) -> PyResult> { - if self.exact_dict(vm) { - self.entries.get_hint(vm, key, usize::from(hint)) - } else { - self.get_item_opt(key, vm) - } + version: u16, + index: u16, + ) -> Option { + self.entries + .get_index_if_keys_version(u32::from(version), usize::from(index)) } /// Lookup trying a cached entry index hint first. diff --git a/crates/vm/src/builtins/function.rs b/crates/vm/src/builtins/function.rs index ac35a19c013..3674f8882b1 100644 --- a/crates/vm/src/builtins/function.rs +++ b/crates/vm/src/builtins/function.rs @@ -550,6 +550,20 @@ impl Py { self.code.flags.contains(bytecode::CodeFlags::OPTIMIZED) } + /// Whether this function currently has native JIT code. Adaptive Python + /// call specializations must yield to that entry point. + #[inline] + pub(crate) fn is_jitted(&self) -> bool { + #[cfg(feature = "jit")] + { + self.jitted_code.lock().is_some() + } + #[cfg(not(feature = "jit"))] + { + false + } + } + pub fn invoke_with_locals( &self, func_args: FuncArgs, @@ -643,8 +657,8 @@ impl Py { .and_then(|()| vm.run_frame_fast(iframe)); // Release data stack memory — must happen on both success and error. unsafe { - if let Some(base) = iframe.release_datastack_frame() { - vm.datastack_pop(base); + if let Some((base, size)) = iframe.release_datastack_frame() { + vm.datastack_pop_frame(base, size); } } result @@ -823,8 +837,8 @@ impl Py { let result = vm.run_frame_fast(iframe); unsafe { - if let Some(base) = iframe.release_datastack_frame() { - vm.datastack_pop(base); + if let Some((base, size)) = iframe.release_datastack_frame() { + vm.datastack_pop_frame(base, size); } } result @@ -1619,6 +1633,16 @@ pub(crate) fn vectorcall_function( let code: &Py = &zelf.code; let has_kwargs = kwnames.is_some_and(|kw| !kw.is_empty()); + if zelf.is_jitted() { + let func_args = if has_kwargs { + FuncArgs::from_vectorcall(&args, nargs, kwnames) + } else { + args.truncate(nargs); + FuncArgs::from(args) + }; + return zelf.invoke(func_args, vm); + } + let is_simple = !has_kwargs && code.flags.contains(bytecode::CodeFlags::OPTIMIZED) && !code.flags.contains(bytecode::CodeFlags::VARARGS) diff --git a/crates/vm/src/builtins/int.rs b/crates/vm/src/builtins/int.rs index c12bf2c721c..134617ab7ea 100644 --- a/crates/vm/src/builtins/int.rs +++ b/crates/vm/src/builtins/int.rs @@ -305,6 +305,22 @@ impl PyInt { &self.value } + /// Extract the inline magnitude without the generic primitive-conversion path. + #[inline(always)] + pub(crate) fn try_to_i64_fast(&self) -> Option { + let bits = self.value.bits(); + if bits > i64::BITS as u64 { + return None; + } + let magnitude = self.value.iter_u64_digits().next().unwrap_or(0); + let signed_magnitude = i64::try_from(magnitude).ok(); + match self.value.sign() { + Sign::Minus if magnitude == 1u64 << 63 => Some(i64::MIN), + Sign::Minus => signed_magnitude.map(|value| -value), + Sign::NoSign | Sign::Plus => signed_magnitude, + } + } + /// Fast decimal string conversion, using i64 path when possible. #[inline] #[must_use] diff --git a/crates/vm/src/datastack.rs b/crates/vm/src/datastack.rs index 101369fba57..2a19a49f455 100644 --- a/crates/vm/src/datastack.rs +++ b/crates/vm/src/datastack.rs @@ -61,6 +61,9 @@ pub struct DataStack { top: *mut u8, /// End of usable space in the current chunk. limit: *mut u8, + /// Most recently popped full-frame allocation whose localsplus slots were + /// cleared before the pop. An exact LIFO reuse can skip zero-filling them. + reusable_frame: Option<(*mut u8, usize)>, } impl DataStack { @@ -73,7 +76,12 @@ impl DataStack { // Skip one ALIGN-sized slot in the root chunk so that `pop()` never // frees it (`push_chunk` convention). let top = unsafe { top.add(ALIGN) }; - Self { chunk, top, limit } + Self { + chunk, + top, + limit, + reusable_frame: None, + } } /// Check if the current chunk has at least `size` bytes available. @@ -91,6 +99,24 @@ impl DataStack { /// (LIFO order). #[inline(always)] pub fn push(&mut self, size: usize) -> *mut u8 { + self.reusable_frame = None; + self.push_inner(size) + } + + /// Allocate a full interpreter frame and report whether it exactly reuses + /// a just-cleared frame block. + #[inline(always)] + pub fn push_frame(&mut self, size: usize) -> (*mut u8, bool) { + let aligned_size = (size + ALIGN - 1) & !(ALIGN - 1); + let reusable_frame = self.reusable_frame.take(); + let ptr = self.push_inner(size); + let reused = + reusable_frame.is_some_and(|(base, old_size)| base == ptr && old_size == aligned_size); + (ptr, reused) + } + + #[inline(always)] + fn push_inner(&mut self, size: usize) -> *mut u8 { let aligned_size = (size + ALIGN - 1) & !(ALIGN - 1); unsafe { if self.top.add(aligned_size) <= self.limit { @@ -138,6 +164,25 @@ impl DataStack { /// and all allocations made after it must already have been popped. #[inline(always)] pub unsafe fn pop(&mut self, base: *mut u8) { + self.reusable_frame = None; + unsafe { self.pop_inner(base) }; + } + + /// Pop a full frame whose localsplus slots have already been cleared. + /// + /// # Safety + /// `base` and `size` must describe the most recent allocation returned by + /// `push_frame`, every later allocation must already be popped, and all + /// localsplus slots in the frame must have been cleared. + #[inline(always)] + pub unsafe fn pop_frame(&mut self, base: *mut u8, size: usize) { + unsafe { self.pop_inner(base) }; + let aligned_size = (size + ALIGN - 1) & !(ALIGN - 1); + self.reusable_frame = Some((base, aligned_size)); + } + + #[inline(always)] + unsafe fn pop_inner(&mut self, base: *mut u8) { debug_assert!(!base.is_null()); if self.is_in_current_chunk(base) { // Common case: base is within the current chunk. diff --git a/crates/vm/src/dict_inner.rs b/crates/vm/src/dict_inner.rs index 9dda6194a0c..acb8ad107a7 100644 --- a/crates/vm/src/dict_inner.rs +++ b/crates/vm/src/dict_inner.rs @@ -20,7 +20,7 @@ use alloc::fmt; use core::mem::size_of; use core::ops::ControlFlow; use core::sync::atomic::{ - AtomicU32, AtomicU64, + AtomicU32, Ordering::{AcqRel, Acquire, Relaxed, Release}, }; use num_traits::ToPrimitive; @@ -39,7 +39,6 @@ type EntryIndex = usize; pub(crate) struct Dict { inner: PyRwLock>, - version: AtomicU64, /// Keys-version stamp, assigned lazily by `assign_keys_version` and /// reset to 0 whenever the key set changes. Value-only updates keep it. /// @@ -202,7 +201,6 @@ impl Clone for Dict { fn clone(&self) -> Self { Self { inner: PyRwLock::new(self.inner.read().clone()), - version: AtomicU64::new(0), keys_version: AtomicU32::new(0), } } @@ -217,7 +215,6 @@ impl Default for Dict { indices: vec![IndexEntry::FREE; 8], entries: Vec::new(), }), - version: AtomicU64::new(0), keys_version: AtomicU32::new(0), } } @@ -362,16 +359,6 @@ impl DictInner { type PopInnerResult = ControlFlow>>; impl Dict { - /// Monotonically increasing version counter for mutation tracking. - pub(crate) fn version(&self) -> u64 { - self.version.load(Acquire) - } - - /// Bump the version counter after any mutation. - fn bump_version(&self) { - self.version.fetch_add(1, Release); - } - /// Current keys-version stamp, or 0 if none has been assigned since the /// last key-set change. Equal nonzero stamps guarantee an unchanged key /// set (values may differ). @@ -500,7 +487,6 @@ impl Dict { )] if entry.index == index_index { let removed = core::mem::replace(&mut entry.value, value); - self.bump_version(); // defer dec RC break Some(removed); } else { @@ -517,7 +503,6 @@ impl Dict { } self.invalidate_keys_version(); inner.unchecked_push(index_index, hash, key.to_pyobject(vm), value, entry_index); - self.bump_version(); break None; } }; @@ -616,7 +601,6 @@ impl Dict { match inner.entries.get_mut(hint) { Some(Some(entry)) if key.key_is(&entry.key) => { let removed = core::mem::replace(&mut entry.value, value); - self.bump_version(); drop(inner); // defer dec RC until after the lock is released drop(removed); @@ -656,6 +640,22 @@ impl Dict { } } + /// Read an entry directly when a cached keys-version still describes the + /// dictionary layout. The version is rechecked while holding the read lock + /// so the entry index and value are observed from the same key-set state. + #[inline] + pub(crate) fn get_index_if_keys_version(&self, version: u32, index: usize) -> Option { + let inner = self.read(); + if self.keys_version.load(Acquire) != version { + return None; + } + inner + .entries + .get(index) + .and_then(Option::as_ref) + .map(|entry| entry.value.clone()) + } + fn _get_inner( &self, vm: &VirtualMachine, @@ -701,7 +701,6 @@ impl Dict { inner.indices.resize(8, IndexEntry::FREE); inner.used = 0; inner.filled = 0; - self.bump_version(); // defer dec rc core::mem::take(&mut inner.entries) }; @@ -830,7 +829,6 @@ impl Dict { } self.invalidate_keys_version(); inner.unchecked_push(index_index, hash, key.to_owned(), value, entry); - self.bump_version(); break None; }; Ok(()) @@ -867,7 +865,6 @@ impl Dict { value.clone(), index_entry, ); - self.bump_version(); return Ok(value); } } @@ -905,7 +902,6 @@ impl Dict { let ret = (key_obj.clone(), value.clone()); self.invalidate_keys_version(); inner.unchecked_push(index_index, hash, key_obj, value, index_entry); - self.bump_version(); return Ok(ret); } } @@ -1117,7 +1113,6 @@ impl Dict { } = IndexEntry::DUMMY; inner.used -= 1; let removed = slot.take(); - self.bump_version(); Ok(ControlFlow::Break(removed)) } @@ -1152,7 +1147,6 @@ impl Dict { // entry.index always refers valid index inner.indices.get_unchecked_mut(entry.index) } = IndexEntry::DUMMY; - self.bump_version(); Some((entry.key, entry.value)) } diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index 3e31b29cc00..8058e805143 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -899,6 +899,7 @@ impl InterpreterFrame { /// For stack-allocated frames (future), the pointers remain valid for the /// frame's lifetime on the native stack. #[allow(clippy::too_many_arguments)] + #[inline(always)] pub(crate) fn new( code: &Py, globals: &Py, @@ -968,9 +969,10 @@ impl InterpreterFrame { /// Returns a mutable reference whose lifetime is bounded by the data /// stack's LIFO discipline. The caller must call /// `release_datastack_frame()` (unsafe) when done, then - /// `vm.datastack_pop(base)`. The reference must not be used after + /// `vm.datastack_pop_frame(base, size)`. The reference must not be used after /// `release_datastack_frame` returns. #[allow(clippy::too_many_arguments)] + #[inline(always)] pub(crate) fn new_on_datastack<'a>( code: &Py, globals: &Py, @@ -987,7 +989,7 @@ impl InterpreterFrame { .expect("LocalsPlus capacity overflow"); let total_bytes = datastack_iframe_total_bytes(nlocalsplus, stacksize); - let base = vm.datastack_push(total_bytes); + let (base, reused_cleared_frame) = vm.datastack_push_frame(total_bytes); // InterpreterFrame lives at the start of the allocation. let iframe_ptr = base as *mut Self; @@ -995,8 +997,10 @@ impl InterpreterFrame { let localsplus_data_ptr = unsafe { base.add(datastack_iframe_localsplus_offset()) } as *mut usize; - // Zero-initialize localsplus data. - unsafe { core::ptr::write_bytes(localsplus_data_ptr, 0, capacity) }; + if !reused_cleared_frame { + // Fresh or differently shaped storage may contain old frame data. + unsafe { core::ptr::write_bytes(localsplus_data_ptr, 0, capacity) }; + } let nlocalsplus_u32 = u32::try_from(nlocalsplus).expect("nlocalsplus exceeds u32"); let localsplus = LocalsPlus { @@ -1038,11 +1042,15 @@ impl InterpreterFrame { /// After this call, the InterpreterFrame at `self` is logically dead — /// the caller must not use `self` again except to pass the returned /// base to `vm.datastack_pop()`. - pub(crate) unsafe fn release_datastack_frame(&mut self) -> Option<*mut u8> { + pub(crate) unsafe fn release_datastack_frame(&mut self) -> Option<(*mut u8, usize)> { let base = self.datastack_base; if base.is_null() { return None; } + let total_bytes = datastack_iframe_total_bytes( + self.localsplus.nlocalsplus as usize, + self.localsplus.stack_capacity(), + ); self.datastack_base = core::ptr::null_mut(); // Drop all localsplus values while the backing store is still valid. self.localsplus.drop_values(); @@ -1055,7 +1063,7 @@ impl InterpreterFrame { // SAFETY: `self` points to valid, initialized memory on the data // stack. After this call the memory is logically dead. unsafe { core::ptr::drop_in_place(self) }; - Some(base) + Some((base, total_bytes)) } /// Get the last instruction index. @@ -2457,11 +2465,11 @@ pub(crate) struct ExecutingFrame<'a> { } #[inline] -fn specialization_compact_int_value(i: &PyInt, vm: &VirtualMachine) -> Option { +fn specialization_compact_int_value(i: &PyInt) -> Option { // _PyLong_IsCompact(): a one-digit PyLong (base 2^30), // i.e. abs(value) <= 2^30 - 1. const CPYTHON_COMPACT_LONG_ABS_MAX: i64 = (1i64 << 30) - 1; - let v = i.try_to_primitive::(vm).ok()?; + let v = i.try_to_i64_fast()?; if (-CPYTHON_COMPACT_LONG_ABS_MAX..=CPYTHON_COMPACT_LONG_ABS_MAX).contains(&v) { Some(v as isize) } else { @@ -2472,7 +2480,7 @@ fn specialization_compact_int_value(i: &PyInt, vm: &VirtualMachine) -> Option Option { obj.downcast_ref_if_exact::(vm) - .and_then(|i| specialization_compact_int_value(i, vm)) + .and_then(|i| specialization_compact_int_value(i)) } #[inline] @@ -4289,9 +4297,10 @@ impl ExecutingFrame<'_> { Ok(None) } Instruction::LoadSmallInt { i: idx } => { - // Push small integer (-5..=256) directly without constant table lookup - let value = vm.ctx.new_int(idx.get(arg) as i32); - self.push_value(value.into()); + // Cached small integers live for the whole Context, so the value stack can + // borrow them without touching the refcount. + let value = vm.ctx.cached_int(idx.get(arg) as i32); + unsafe { self.push_borrowed(value.as_object()) }; Ok(None) } Instruction::LoadDeref { i } => { @@ -5686,8 +5695,8 @@ impl ExecutingFrame<'_> { b.downcast_ref_if_exact::(vm), ) { let result = a_str.as_wtf8().py_add(b_str.as_wtf8()); - self.pop_value(); - self.pop_value(); + self.pop_stackref(); + self.pop_stackref(); self.push_value(result.to_pyobject(vm)); Ok(None) } else { @@ -5846,6 +5855,9 @@ impl ExecutingFrame<'_> { && func.func_version() == cached_version && cached_version != 0 { + if func.is_jitted() { + return self.execute_call_vectorcall(nargs, vm); + } let effective_nargs = nargs + u32::from(self_or_null_is_some); if !func.has_exact_argcount(effective_nargs) { return self.execute_call_vectorcall(nargs, vm); @@ -5908,6 +5920,9 @@ impl ExecutingFrame<'_> { && func.func_version() == cached_version && cached_version != 0 { + if func.is_jitted() { + return self.execute_call_vectorcall(nargs, vm); + } if !func.has_exact_argcount(nargs + 1) { return self.execute_call_vectorcall(nargs, vm); } @@ -6143,6 +6158,9 @@ impl ExecutingFrame<'_> { && func.func_version() == cached_version && cached_version != 0 { + if func.is_jitted() { + return self.execute_call_vectorcall(nargs, vm); + } if self.specialization_call_recursion_guard(vm) { return self.execute_call_vectorcall(nargs, vm); } @@ -6189,6 +6207,9 @@ impl ExecutingFrame<'_> { && func.func_version() == cached_version && cached_version != 0 { + if func.is_jitted() { + return self.execute_call_vectorcall(nargs, vm); + } if self.specialization_call_recursion_guard(vm) { return self.execute_call_vectorcall(nargs, vm); } @@ -6601,6 +6622,9 @@ impl ExecutingFrame<'_> { && func.func_version() == cached_version && cached_version != 0 { + if func.is_jitted() { + return self.execute_call_kw_vectorcall(nargs, vm); + } if self.specialization_call_recursion_guard(vm) { return self.execute_call_kw_vectorcall(nargs, vm); } @@ -6659,6 +6683,9 @@ impl ExecutingFrame<'_> { && func.func_version() == cached_version && cached_version != 0 { + if func.is_jitted() { + return self.execute_call_kw_vectorcall(nargs, vm); + } let nargs_usize = nargs as usize; let kwarg_names_obj = self.pop_value(); let kwarg_names_tuple = kwarg_names_obj @@ -6854,14 +6881,16 @@ impl ExecutingFrame<'_> { a.downcast_ref_if_exact::(vm), b.downcast_ref_if_exact::(vm), ) && let (Some(a_val), Some(b_val)) = ( - specialization_compact_int_value(a_int, vm), - specialization_compact_int_value(b_int, vm), + specialization_compact_int_value(a_int), + specialization_compact_int_value(b_int), ) { let op = self.compare_op_from_arg(arg); let result = op.eval_ord(a_val.cmp(&b_val)); - self.pop_value(); - self.pop_value(); - self.push_value(vm.ctx.new_bool(result).into()); + self.pop_stackref(); + self.pop_stackref(); + if !self.try_fused_compare_int_jump(result, vm) { + self.push_value(vm.ctx.new_bool(result).into()); + } Ok(None) } else { self.execute_compare(vm, arg) @@ -7187,17 +7216,16 @@ impl ExecutingFrame<'_> { // Keep specialized opcode on guard miss (JUMP_TO_PREDICTED behavior). let cached_version = self.code.instructions.read_cache_u16(cache_base + 1); let cached_index = self.code.instructions.read_cache_u16(cache_base + 3); - if let Ok(current_version) = u16::try_from(self.globals.version()) - && cached_version == current_version + if cached_version != 0 + && let Some(x) = self + .globals + .get_item_by_index_and_keys_version(cached_version, cached_index) { - let name = self.code.names[(oparg >> 1) as usize]; - if let Some(x) = self.globals.get_item_opt_hint(name, cached_index, vm)? { - self.push_value(x); - if (oparg & 1) != 0 { - self.push_value_opt(None); - } - return Ok(None); + self.push_value(x); + if (oparg & 1) != 0 { + self.push_value_opt(None); } + return Ok(None); } let name = self.code.names[(oparg >> 1) as usize]; let x = self.load_global_or_builtin(name, vm)?; @@ -7213,20 +7241,19 @@ impl ExecutingFrame<'_> { let cached_globals_ver = self.code.instructions.read_cache_u16(cache_base + 1); let cached_builtins_ver = self.code.instructions.read_cache_u16(cache_base + 2); let cached_index = self.code.instructions.read_cache_u16(cache_base + 3); - if let Ok(current_globals_ver) = u16::try_from(self.globals.version()) + if cached_globals_ver != 0 + && cached_builtins_ver != 0 + && let Ok(current_globals_ver) = u16::try_from(self.globals.keys_version()) && cached_globals_ver == current_globals_ver && let Some(builtins_dict) = self.builtins.downcast_ref_if_exact::(vm) - && let Ok(current_builtins_ver) = u16::try_from(builtins_dict.version()) - && cached_builtins_ver == current_builtins_ver + && let Some(x) = builtins_dict + .get_item_by_index_and_keys_version(cached_builtins_ver, cached_index) { - let name = self.code.names[(oparg >> 1) as usize]; - if let Some(x) = builtins_dict.get_item_opt_hint(name, cached_index, vm)? { - self.push_value(x); - if (oparg & 1) != 0 { - self.push_value_opt(None); - } - return Ok(None); + self.push_value(x); + if (oparg & 1) != 0 { + self.push_value_opt(None); } + return Ok(None); } let name = self.code.names[(oparg >> 1) as usize]; let x = self.load_global_or_builtin(name, vm)?; @@ -8570,7 +8597,7 @@ impl ExecutingFrame<'_> { a_ref.downcast_ref_if_exact::(vm), b_ref.downcast_ref_if_exact::(vm), ) { - Ok(Self::int_add(a.as_bigint(), b.as_bigint(), vm)) + Ok(Self::int_add(a, b, vm)) } else if matches!(op, bytecode::BinaryOperator::Add) { vm._add(a_ref, b_ref) } else { @@ -8582,7 +8609,7 @@ impl ExecutingFrame<'_> { a_ref.downcast_ref_if_exact::(vm), b_ref.downcast_ref_if_exact::(vm), ) { - Ok(Self::int_sub(a.as_bigint(), b.as_bigint(), vm)) + Ok(Self::int_sub(a, b, vm)) } else if matches!(op, bytecode::BinaryOperator::Subtract) { vm._sub(a_ref, b_ref) } else { @@ -8594,7 +8621,7 @@ impl ExecutingFrame<'_> { a_ref.downcast_ref_if_exact::(vm), b_ref.downcast_ref_if_exact::(vm), ) { - Ok(Self::int_mul(a.as_bigint(), b.as_bigint(), vm)) + Ok(Self::int_mul(a, b, vm)) } else if matches!(op, bytecode::BinaryOperator::Multiply) { vm._mul(a_ref, b_ref) } else { @@ -8660,36 +8687,37 @@ impl ExecutingFrame<'_> { /// small-int cache is consulted identically. #[inline] fn int_fast_op( - a: &BigInt, - b: &BigInt, + a: &PyInt, + b: &PyInt, vm: &VirtualMachine, checked: fn(i64, i64) -> Option, fallback: impl FnOnce(&BigInt, &BigInt) -> BigInt, ) -> PyObjectRef { - use num_traits::ToPrimitive; - if let (Some(av), Some(bv)) = (a.to_i64(), b.to_i64()) + if let (Some(av), Some(bv)) = (a.try_to_i64_fast(), b.try_to_i64_fast()) && let Some(result) = checked(av, bv) { return vm.ctx.new_int(result).into(); } - vm.ctx.new_int(fallback(a, b)).into() + vm.ctx + .new_int(fallback(a.as_bigint(), b.as_bigint())) + .into() } /// Int addition with i64 fast path to avoid BigInt heap allocation. #[inline] - fn int_add(a: &BigInt, b: &BigInt, vm: &VirtualMachine) -> PyObjectRef { + fn int_add(a: &PyInt, b: &PyInt, vm: &VirtualMachine) -> PyObjectRef { Self::int_fast_op(a, b, vm, i64::checked_add, |a, b| a + b) } /// Int subtraction with i64 fast path to avoid BigInt heap allocation. #[inline] - fn int_sub(a: &BigInt, b: &BigInt, vm: &VirtualMachine) -> PyObjectRef { + fn int_sub(a: &PyInt, b: &PyInt, vm: &VirtualMachine) -> PyObjectRef { Self::int_fast_op(a, b, vm, i64::checked_sub, |a, b| a - b) } /// Int multiplication with i64 fast path to avoid BigInt heap allocation. #[inline] - fn int_mul(a: &BigInt, b: &BigInt, vm: &VirtualMachine) -> PyObjectRef { + fn int_mul(a: &PyInt, b: &PyInt, vm: &VirtualMachine) -> PyObjectRef { Self::int_fast_op(a, b, vm, i64::checked_mul, |a, b| a * b) } @@ -9834,7 +9862,7 @@ impl ExecutingFrame<'_> { fn execute_binary_op_int( &mut self, vm: &VirtualMachine, - op: impl FnOnce(&BigInt, &BigInt, &VirtualMachine) -> PyObjectRef, + op: impl FnOnce(&PyInt, &PyInt, &VirtualMachine) -> PyObjectRef, deopt_op: bytecode::BinaryOperator, ) -> FrameResult { let b = self.top_value(); @@ -9843,9 +9871,9 @@ impl ExecutingFrame<'_> { a.downcast_ref_if_exact::(vm), b.downcast_ref_if_exact::(vm), ) { - let result = op(a_int.as_bigint(), b_int.as_bigint(), vm); - self.pop_value(); - self.pop_value(); + let result = op(a_int, b_int, vm); + self.pop_stackref(); + self.pop_stackref(); self.push_value(result); Ok(None) } else { @@ -9902,7 +9930,7 @@ impl ExecutingFrame<'_> { let callable = self.nth_value(nargs + 1); if let Some(func) = callable.downcast_ref_if_exact::(vm) { - if self.specialization_eval_frame_active(vm) { + if self.specialization_eval_frame_active(vm) || func.is_jitted() { unsafe { self.code.instructions.write_adaptive_counter( cache_base, @@ -9965,7 +9993,7 @@ impl ExecutingFrame<'_> { .function_obj() .downcast_ref_if_exact::(vm) { - if self.specialization_eval_frame_active(vm) { + if self.specialization_eval_frame_active(vm) || func.is_jitted() { unsafe { self.code.instructions.write_adaptive_counter( cache_base, @@ -10260,7 +10288,7 @@ impl ExecutingFrame<'_> { let callable = self.nth_value(nargs + 2); if let Some(func) = callable.downcast_ref_if_exact::(vm) { - if self.specialization_eval_frame_active(vm) { + if self.specialization_eval_frame_active(vm) || func.is_jitted() { unsafe { self.code.instructions.write_adaptive_counter( cache_base, @@ -10311,7 +10339,7 @@ impl ExecutingFrame<'_> { .function_obj() .downcast_ref_if_exact::(vm) { - if self.specialization_eval_frame_active(vm) { + if self.specialization_eval_frame_active(vm) || func.is_jitted() { unsafe { self.code.instructions.write_adaptive_counter( cache_base, @@ -10454,8 +10482,8 @@ impl ExecutingFrame<'_> { a.downcast_ref_if_exact::(vm), b.downcast_ref_if_exact::(vm), ) { - if specialization_compact_int_value(a_int, vm).is_some() - && specialization_compact_int_value(b_int, vm).is_some() + if specialization_compact_int_value(a_int).is_some() + && specialization_compact_int_value(b_int).is_some() { Some(Instruction::CompareOpInt) } else { @@ -10486,6 +10514,37 @@ impl ExecutingFrame<'_> { .into() } + /// Execute an immediately following conditional jump without materializing + /// the comparison result as a Python bool. This is the adaptive interpreter + /// equivalent of keeping the result virtual across the two-opcode trace. + #[inline] + fn try_fused_compare_int_jump(&mut self, result: bool, vm: &VirtualMachine) -> bool { + if self.specialization_eval_frame_active(vm) { + return false; + } + + let jump_idx = self.lasti() as usize + Instruction::CompareOpInt.cache_entries(); + if jump_idx >= self.code.instructions.len() { + return false; + } + + let jump_op = self.code.instructions.read_op(jump_idx); + let jump_on = match jump_op { + Instruction::PopJumpIfFalse { .. } => false, + Instruction::PopJumpIfTrue { .. } => true, + _ => return false, + }; + let jump_delta = self.code.instructions.read_arg(jump_idx).as_u32(); + let after_jump = jump_idx as u32 + 1 + jump_op.cache_entries() as u32; + let target = if result == jump_on { + after_jump + jump_delta + } else { + after_jump + }; + self.update_lasti(|i| *i = target); + true + } + /// Recover the BinaryOperator from the instruction arg byte. /// `replace_op` preserves the arg byte, so the original op remains accessible. fn binary_op_from_arg(&self, arg: bytecode::OpArg) -> bytecode::BinaryOperator { @@ -10686,11 +10745,10 @@ impl ExecutingFrame<'_> { } } - // Pop the callable and transfer ownership to the trampoline via - // the VM side channel, avoiding a per-frame mutex lock on - // temporary_refs. + // Pop the callable and transfer ownership to the trampoline. This one + // reference keeps every field borrowed by the callee frame alive. let callable = self.pop_value(); - unsafe { &mut *vm.pending_tailcall_refs.get() }.push(callable); + vm.set_pending_tailcall_owner(callable); vm.set_pending_tailcall(callee_iframe); } @@ -10740,13 +10798,13 @@ impl ExecutingFrame<'_> { *dst = Some(arg); } self.pop_value_opt(); // null (self_or_null) - let callable = self.pop_value(); // callable (bound method) + self.pop_value(); // callable (bound method) fastlocals[0] = Some(bound_self); - // Transfer ownership to the trampoline via the VM side channel. - let refs = unsafe { &mut *vm.pending_tailcall_refs.get() }; - refs.push(bound_function); - refs.push(callable); + // The function owns every field borrowed by the callee frame. + // bound_self is owned by fastlocals; the bound-method object itself is + // no longer needed and was dropped above, matching the recursive path. + vm.set_pending_tailcall_owner(bound_function); vm.set_pending_tailcall(callee_iframe); } @@ -10798,7 +10856,7 @@ impl ExecutingFrame<'_> { return; } let name = self.code.names[(oparg >> 1) as usize]; - let Ok(globals_version) = u16::try_from(self.globals.version()) else { + let Ok(globals_version @ 1..) = u16::try_from(self.globals.assign_keys_version(vm)) else { unsafe { self.code.instructions.write_adaptive_counter( cache_base, @@ -10826,7 +10884,7 @@ impl ExecutingFrame<'_> { if let Some(builtins_dict) = self.builtins.downcast_ref_if_exact::(vm) && let Ok(Some(builtins_hint)) = builtins_dict.hint_for_key(name, vm) - && let Ok(builtins_version) = u16::try_from(builtins_dict.version()) + && let Ok(builtins_version @ 1..) = u16::try_from(builtins_dict.assign_keys_version(vm)) { unsafe { self.code diff --git a/crates/vm/src/vm/context.rs b/crates/vm/src/vm/context.rs index 08efde1b7b4..71d017c6d1c 100644 --- a/crates/vm/src/vm/context.rs +++ b/crates/vm/src/vm/context.rs @@ -434,6 +434,14 @@ impl Context { PyInt::from(i).into_ref(self) } + /// Borrow a cached small integer whose lifetime is tied to this context. + #[inline(always)] + pub(crate) fn cached_int(&self, i: i32) -> &PyIntRef { + debug_assert!(Self::INT_CACHE_POOL_RANGE.contains(&i)); + let inner_idx = (i - Self::INT_CACHE_POOL_MIN) as usize; + &self.int_cache_pool[inner_idx] + } + #[inline] pub fn new_bigint(&self, i: &BigInt) -> PyIntRef { if let Some(i) = i.to_i32() diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index 13d402a1702..f7167ac2387 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -117,11 +117,11 @@ pub struct VirtualMachine { /// pointer here before returning `ExecutionResult::TailCall`. /// Access only via `set_pending_tailcall` / `take_pending_tailcall`. pending_tailcall_frame: Cell>, - /// Owned references that keep callee raw pointers valid during TailCall. - /// Set by `tailcall_prepare_frame`, drained by the trampoline into - /// its local `owned_refs` Vec. Uses UnsafeCell because the VM is - /// per-thread and this field is only accessed on the owning thread. - pub(crate) pending_tailcall_refs: core::cell::UnsafeCell>, + /// Owned reference that keeps callee raw pointers valid during TailCall. + /// Set by the exact-call handlers and moved into the trampoline's + /// `SuspendedFrame`. Uses UnsafeCell because the VM is per-thread and this + /// field is only accessed on the owning thread. + pending_tailcall_owner: core::cell::UnsafeCell>, } /// Non-owning frame pointer for the non-unix threading frames stack. @@ -856,11 +856,11 @@ pub(crate) struct IframeEntryState { struct SuspendedFrame { iframe: *mut crate::frame::InterpreterFrame, entry_state: IframeEntryState, - /// Owned references that keep callee's raw pointers (code, globals, - /// builtins borrowed from PyFunction) valid. Drained from - /// `vm.pending_tailcall_refs` when the callee's TailCall is consumed. + /// Function that owns the callee's raw pointers (code, globals, builtins, + /// closure, and func_obj). Moved from `vm.pending_tailcall_owner` when the + /// callee's TailCall is consumed. /// Dropped when this SuspendedFrame is popped (after callee returns/errors). - owned_refs: Vec, + callee_owner: PyObjectRef, /// True for the initial frame passed into the trampoline by the caller. /// The caller owns the datastack allocation for the entry frame, so the /// trampoline must NOT release it — only callee-allocated frames are @@ -893,6 +893,13 @@ impl VirtualMachine { unsafe { (*self.datastack.get()).push(size) } } + /// Bump-allocate a full frame, returning whether the same cleared LIFO + /// block and size were reused. + #[inline(always)] + pub(crate) fn datastack_push_frame(&self, size: usize) -> (*mut u8, bool) { + unsafe { (*self.datastack.get()).push_frame(size) } + } + /// Check whether the thread data stack currently has room for `size` bytes. #[inline(always)] pub(crate) fn datastack_has_space(&self, size: usize) -> bool { @@ -909,6 +916,12 @@ impl VirtualMachine { unsafe { (*self.datastack.get()).pop(base) } } + /// Pop a full frame after its localsplus slots have been cleared. + #[inline(always)] + pub(crate) unsafe fn datastack_pop_frame(&self, base: *mut u8, size: usize) { + unsafe { (*self.datastack.get()).pop_frame(base, size) } + } + /// Temporarily detach the current thread (ATTACHED → DETACHED) while /// running `f`, then re-attach afterwards. Allows `stop_the_world` to /// park this thread during blocking syscalls. @@ -983,7 +996,7 @@ impl VirtualMachine { callable_cache: CallableCache::default(), audit_hooks: RefCell::new(vec![]), pending_tailcall_frame: Cell::new(None), - pending_tailcall_refs: core::cell::UnsafeCell::new(Vec::with_capacity(2)), + pending_tailcall_owner: core::cell::UnsafeCell::new(None), }; if vm.state.hash_secret.hash_str("") @@ -1437,6 +1450,22 @@ impl VirtualMachine { .set(Some(PendingFrame(core::ptr::NonNull::from(iframe)))); } + /// Store the function that owns the fields borrowed by the pending callee. + #[inline(always)] + pub(crate) fn set_pending_tailcall_owner(&self, owner: PyObjectRef) { + let slot = unsafe { &mut *self.pending_tailcall_owner.get() }; + debug_assert!(slot.is_none(), "pending TailCall owner was not consumed"); + *slot = Some(owner); + } + + /// Take the pending callee owner, resetting the side channel. + #[inline(always)] + fn take_pending_tailcall_owner(&self) -> PyObjectRef { + unsafe { &mut *self.pending_tailcall_owner.get() } + .take() + .expect("TailCall without pending owner") + } + /// Take the pending tailcall frame pointer, resetting the side channel. #[inline(always)] fn take_pending_tailcall(&self) -> *mut crate::frame::InterpreterFrame { @@ -1497,18 +1526,11 @@ impl VirtualMachine { } let initial_ptr = self.take_pending_tailcall(); - // Drain the refs that keep the initial callee's raw pointers alive. - #[allow( - clippy::drain_collect, - reason = "`pending_tailcall_refs`'s allocation is intentionally reused" - )] - let initial_refs = unsafe { &mut *self.pending_tailcall_refs.get() } - .drain(..) - .collect(); + let initial_owner = self.take_pending_tailcall_owner(); frame_stack.push(SuspendedFrame { iframe: iframe as *mut crate::frame::InterpreterFrame, entry_state, - owned_refs: initial_refs, + callee_owner: initial_owner, is_entry: true, }); let mut action = Action::EnterCallee(initial_ptr); @@ -1521,8 +1543,8 @@ impl VirtualMachine { Ok(state) => state, Err(exc) => { unsafe { - if let Some(base) = callee.release_datastack_frame() { - self.datastack_pop(base); + if let Some((base, size)) = callee.release_datastack_frame() { + self.datastack_pop_frame(base, size); } } action = Action::Unwind(exc); @@ -1533,17 +1555,11 @@ impl VirtualMachine { let result = crate::frame::run_iframe(callee, self); match result { Ok(ExecutionResult::TailCall) => { - #[allow( - clippy::drain_collect, - reason = "`pending_tailcall_refs`'s allocation is intentionally reused" - )] - let refs = unsafe { &mut *self.pending_tailcall_refs.get() } - .drain(..) - .collect(); + let callee_owner = self.take_pending_tailcall_owner(); frame_stack.push(SuspendedFrame { iframe: callee_ptr, entry_state: callee_entry, - owned_refs: refs, + callee_owner, is_entry: false, }); action = Action::EnterCallee(self.take_pending_tailcall()); @@ -1551,8 +1567,8 @@ impl VirtualMachine { Ok(ExecutionResult::Return(value)) => { self.exit_iframe(callee_entry); unsafe { - if let Some(base) = callee.release_datastack_frame() { - self.datastack_pop(base); + if let Some((base, size)) = callee.release_datastack_frame() { + self.datastack_pop_frame(base, size); } } action = Action::ReturnValue(value); @@ -1561,8 +1577,8 @@ impl VirtualMachine { Err(exc) => { self.exit_iframe(callee_entry); unsafe { - if let Some(base) = callee.release_datastack_frame() { - self.datastack_pop(base); + if let Some((base, size)) = callee.release_datastack_frame() { + self.datastack_pop_frame(base, size); } } action = Action::Unwind(exc); @@ -1578,7 +1594,7 @@ impl VirtualMachine { let SuspendedFrame { iframe: caller_iframe_ptr, entry_state: caller_entry, - owned_refs: _caller_refs, + callee_owner, is_entry: caller_is_entry, } = caller; let caller_iframe = unsafe { &mut *caller_iframe_ptr }; @@ -1587,29 +1603,25 @@ impl VirtualMachine { let result = crate::frame::run_iframe(caller_iframe, self); match result { Ok(ExecutionResult::TailCall) => { - #[allow( - clippy::drain_collect, - reason = "`pending_tailcall_refs`'s allocation is intentionally reused" - )] - let refs = unsafe { &mut *self.pending_tailcall_refs.get() } - .drain(..) - .collect(); - drop(_caller_refs); + let next_callee_owner = self.take_pending_tailcall_owner(); + drop(callee_owner); frame_stack.push(SuspendedFrame { iframe: caller_iframe_ptr, entry_state: caller_entry, - owned_refs: refs, + callee_owner: next_callee_owner, is_entry: caller_is_entry, }); action = Action::EnterCallee(self.take_pending_tailcall()); } Ok(ExecutionResult::Return(value)) => { - drop(_caller_refs); + drop(callee_owner); self.exit_iframe(caller_entry); if !caller_is_entry { unsafe { - if let Some(base) = caller_iframe.release_datastack_frame() { - self.datastack_pop(base); + if let Some((base, size)) = + caller_iframe.release_datastack_frame() + { + self.datastack_pop_frame(base, size); } } } @@ -1617,12 +1629,14 @@ impl VirtualMachine { } Ok(ExecutionResult::Yield(_)) => panic!("Yield in non-generator frame"), Err(exc) => { - drop(_caller_refs); + drop(callee_owner); self.exit_iframe(caller_entry); if !caller_is_entry { unsafe { - if let Some(base) = caller_iframe.release_datastack_frame() { - self.datastack_pop(base); + if let Some((base, size)) = + caller_iframe.release_datastack_frame() + { + self.datastack_pop_frame(base, size); } } } @@ -1638,7 +1652,7 @@ impl VirtualMachine { let SuspendedFrame { iframe: caller_iframe_ptr, entry_state: caller_entry, - owned_refs: _caller_refs, + callee_owner, is_entry: caller_is_entry, } = caller; let caller_iframe = unsafe { &mut *caller_iframe_ptr }; @@ -1652,31 +1666,25 @@ impl VirtualMachine { let result = crate::frame::run_iframe(caller_iframe, self); match result { Ok(ExecutionResult::TailCall) => { - #[allow( - clippy::drain_collect, - reason = "`pending_tailcall_refs`'s allocation is intentionally reused" - )] - let refs = unsafe { &mut *self.pending_tailcall_refs.get() } - .drain(..) - .collect(); - drop(_caller_refs); + let next_callee_owner = self.take_pending_tailcall_owner(); + drop(callee_owner); frame_stack.push(SuspendedFrame { iframe: caller_iframe_ptr, entry_state: caller_entry, - owned_refs: refs, + callee_owner: next_callee_owner, is_entry: caller_is_entry, }); action = Action::EnterCallee(self.take_pending_tailcall()); } Ok(ExecutionResult::Return(value)) => { - drop(_caller_refs); + drop(callee_owner); self.exit_iframe(caller_entry); if !caller_is_entry { unsafe { - if let Some(base) = + if let Some((base, size)) = caller_iframe.release_datastack_frame() { - self.datastack_pop(base); + self.datastack_pop_frame(base, size); } } } @@ -1686,14 +1694,14 @@ impl VirtualMachine { panic!("Yield in non-generator frame") } Err(new_exc) => { - drop(_caller_refs); + drop(callee_owner); self.exit_iframe(caller_entry); if !caller_is_entry { unsafe { - if let Some(base) = + if let Some((base, size)) = caller_iframe.release_datastack_frame() { - self.datastack_pop(base); + self.datastack_pop_frame(base, size); } } } @@ -1702,12 +1710,14 @@ impl VirtualMachine { } } Ok(Some(ExecutionResult::Return(value))) => { - drop(_caller_refs); + drop(callee_owner); self.exit_iframe(caller_entry); if !caller_is_entry { unsafe { - if let Some(base) = caller_iframe.release_datastack_frame() { - self.datastack_pop(base); + if let Some((base, size)) = + caller_iframe.release_datastack_frame() + { + self.datastack_pop_frame(base, size); } } } @@ -1717,12 +1727,14 @@ impl VirtualMachine { panic!("Unexpected execution result in trampoline unwind") } Err(new_exc) => { - drop(_caller_refs); + drop(callee_owner); self.exit_iframe(caller_entry); if !caller_is_entry { unsafe { - if let Some(base) = caller_iframe.release_datastack_frame() { - self.datastack_pop(base); + if let Some((base, size)) = + caller_iframe.release_datastack_frame() + { + self.datastack_pop_frame(base, size); } } } diff --git a/crates/vm/src/vm/thread.rs b/crates/vm/src/vm/thread.rs index b69a244f639..43d9f06ec7a 100644 --- a/crates/vm/src/vm/thread.rs +++ b/crates/vm/src/vm/thread.rs @@ -122,6 +122,12 @@ thread_local! { static CURRENT_TOP_FRAME_SLOT: Cell<*const AtomicPtr> = const { Cell::new(core::ptr::null()) }; + /// Cached pointer to this thread's `ThreadSlot::top_iframe` for the hot + /// light-frame push/pop path. The slot's Arc keeps the pointee alive. + #[cfg(feature = "threading")] + static CURRENT_TOP_IFRAME_SLOT: Cell<*const AtomicUsize> = + const { Cell::new(core::ptr::null()) }; + } #[must_use] @@ -410,6 +416,7 @@ fn ensure_thread_slot(vm: &VirtualMachine) -> CurrentFrameSlot { fn set_current_thread_slot(slot: CurrentFrameSlot) { #[cfg(unix)] CURRENT_TOP_FRAME_SLOT.with(|c| c.set(&slot.top_frame)); + CURRENT_TOP_IFRAME_SLOT.with(|c| c.set(&slot.top_iframe)); CURRENT_THREAD_SLOT.with(|current| { *current.borrow_mut() = Some(slot); }); @@ -790,27 +797,25 @@ pub fn set_current_frame(frame: *const InterpreterFrame) -> *const InterpreterFr // sys._current_frames). #[cfg(feature = "threading")] { - CURRENT_THREAD_SLOT.with(|slot| { - if let Some(s) = slot.borrow().as_ref() { - if !frame.is_null() { - #[cfg(unix)] - { - let frame_obj = unsafe { (*frame).frame_obj() }; - let fo_ptr = match frame_obj { - Some(py) => { - py as *const Py as *const FrameObject - as *mut FrameObject - } - None => core::ptr::null_mut(), - }; - s.top_frame.store(fo_ptr, Ordering::Relaxed); - } - s.top_iframe.store(frame as usize, Ordering::Relaxed); + CURRENT_TOP_IFRAME_SLOT.with(|slot| { + let slot = slot.get(); + if !slot.is_null() { + unsafe { &*slot }.store(frame as usize, Ordering::Relaxed); + } + }); + #[cfg(unix)] + CURRENT_TOP_FRAME_SLOT.with(|slot| { + let slot = slot.get(); + if !slot.is_null() { + let fo_ptr = if frame.is_null() { + core::ptr::null_mut() } else { - #[cfg(unix)] - s.top_frame.store(core::ptr::null_mut(), Ordering::Relaxed); - s.top_iframe.store(0, Ordering::Relaxed); - } + let frame_obj = unsafe { (*frame).frame_obj() }; + frame_obj.map_or(core::ptr::null_mut(), |py| { + py as *const Py as *const FrameObject as *mut FrameObject + }) + }; + unsafe { &*slot }.store(fo_ptr, Ordering::Relaxed); } }); } @@ -913,6 +918,8 @@ pub fn cleanup_current_thread_frames(vm: &VirtualMachine) { *s.borrow_mut() = None; #[cfg(all(unix, feature = "threading"))] CURRENT_TOP_FRAME_SLOT.with(|c| c.set(core::ptr::null())); + #[cfg(feature = "threading")] + CURRENT_TOP_IFRAME_SLOT.with(|c| c.set(core::ptr::null())); } }); } @@ -974,6 +981,8 @@ pub fn reinit_frame_slot_after_fork(vm: &VirtualMachine) { }); #[cfg(all(unix, feature = "threading"))] CURRENT_TOP_FRAME_SLOT.with(|c| c.set(&new_slot.top_frame)); + #[cfg(feature = "threading")] + CURRENT_TOP_IFRAME_SLOT.with(|c| c.set(&new_slot.top_iframe)); // Lock is safe: reinit_locks_after_fork() already reset it to unlocked. let mut registry = vm.state.thread_frames.lock(); @@ -1148,7 +1157,7 @@ impl VirtualMachine { callable_cache: self.callable_cache.clone(), audit_hooks: RefCell::new(vec![]), pending_tailcall_frame: Cell::new(None), - pending_tailcall_refs: core::cell::UnsafeCell::new(Vec::with_capacity(2)), + pending_tailcall_owner: core::cell::UnsafeCell::new(None), }; ThreadedVirtualMachine { vm } } From c3fcc702e09d474abfdc10b56711114a41066a7d Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sat, 15 Aug 2026 23:45:38 +0900 Subject: [PATCH 12/20] gc, vm: address review notes on owner tags and interpreter docs Look up retired owner tags with a sorted binary search instead of a linear scan, which every scanned object paid for once per dropped interpreter. Drop the claim that clearing a tag frees it for reuse; `alloc_owner` only ever hands out new tags. Also correct the tag space it mentions, which is 16-bit since the tag was sized to the header padding. Document `is_main` as "top-level interpreter" rather than "the process main": every top-level interpreter sets it, and only the first registered one becomes the main `main_interpreter_id` reports. Assert membership rather than an exact owned-interpreter count delta; the owned table is process-global and other tests store into it in parallel. Assisted-by: Claude --- crates/vm/src/gc_state.rs | 24 ++++++++++++++++++------ crates/vm/src/vm/interpreter.rs | 10 +++++++--- crates/vm/src/vm/mod.rs | 4 +++- 3 files changed, 28 insertions(+), 10 deletions(-) diff --git a/crates/vm/src/gc_state.rs b/crates/vm/src/gc_state.rs index a7e39d697fd..7038a181ab6 100644 --- a/crates/vm/src/gc_state.rs +++ b/crates/vm/src/gc_state.rs @@ -306,7 +306,7 @@ impl GcState { } /// Reserve a tag for a new interpreter. Tags are never reused; exhausting - /// the 32-bit space falls back to `GC_NO_OWNER`, which costs isolation but + /// the tag space falls back to `GC_NO_OWNER`, which costs isolation but /// stays correct, rather than aliasing a live interpreter. fn alloc_owner(&self) -> GcOwner { self.next_owner @@ -536,11 +536,18 @@ impl GcState { // Another interpreter's objects stay out of the candidate set, so they // act as external roots: anything they reference survives this pass. let owner = gc.owner; - let retired = self.retired.lock().clone(); + // Sorted so that the test below, which every scanned object pays for, + // stays logarithmic in the number of interpreters that have been + // dropped instead of linear. + let retired = { + let mut retired = self.retired.lock().clone(); + retired.sort_unstable(); + retired + }; let mut collecting: HashSet = HashSet::new(); for gen_list in &gen_locks { for obj in gen_list.iter() { - if retired.contains(&obj.gc_owner()) { + if retired.binary_search(&obj.gc_owner()).is_ok() { obj.set_gc_owner(GC_NO_OWNER); } if obj.strong_count() > 0 && is_owned_by(obj, owner) { @@ -553,11 +560,15 @@ impl GcState { // is where adoption finishes and the tags stop being tracked. if generation == 2 && !retired.is_empty() { for obj in self.permanent_list.read().iter() { - if retired.contains(&obj.gc_owner()) { + if retired.binary_search(&obj.gc_owner()).is_ok() { obj.set_gc_owner(GC_NO_OWNER); } } - self.retired.lock().retain(|tag| !retired.contains(tag)); + // Only the tags this scan saw: one retired while it ran still has + // objects nobody has adopted. + self.retired + .lock() + .retain(|tag| retired.binary_search(tag).is_err()); } if collecting.is_empty() { @@ -1281,7 +1292,8 @@ impl Drop for GcInterpreterState { fn drop(&mut self) { // Objects this interpreter tracked can outlive it (another interpreter // may still hold one). Clearing the tag hands them to every collection - // instead of stranding them, and frees the tag for reuse. + // instead of stranding them. The tag itself is not handed back: it stays + // retired so that a later interpreter cannot inherit these objects. gc_state().retire_owner(self.owner); } } diff --git a/crates/vm/src/vm/interpreter.rs b/crates/vm/src/vm/interpreter.rs index d6700531dde..969197bb290 100644 --- a/crates/vm/src/vm/interpreter.rs +++ b/crates/vm/src/vm/interpreter.rs @@ -449,7 +449,10 @@ impl Interpreter { self.global_state.whence } - /// Whether this is the process main interpreter. + /// Whether this is a top-level interpreter rather than a subinterpreter. + /// + /// Every top-level interpreter answers `true`; for *the* process main, use + /// [`Interpreter::is_process_main`]. #[inline] #[must_use] pub fn is_main(&self) -> bool { @@ -1358,11 +1361,12 @@ mod tests { let sub = main.create_subinterpreter(); let id = sub.id(); - let before = runtime::owned_interpreter_count(); assert_eq!(runtime::store_owned_interpreter(sub), id); assert!(runtime::is_owned_interpreter(id)); assert!(runtime::lookup_interpreter(id).is_some()); - assert_eq!(runtime::owned_interpreter_count(), before + 1); + // The owned table is process-global and other tests store into it in + // parallel, so only this entry's own membership is deterministic. + assert!(runtime::owned_interpreter_count() >= 1); // Reclaiming removes ownership but keeps the interpreter alive while the // returned handle is held. diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index f7167ac2387..364aaa7a940 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -749,7 +749,9 @@ pub struct PyGlobalState { pub interpreter_id: i64, /// How this interpreter was created. pub whence: runtime::InterpreterWhence, - /// True only for the process main interpreter. + /// True for every top-level (non-sub) interpreter, each of which keeps its + /// own signal and main-thread bookkeeping. Only the first one registered + /// becomes *the* process main — see [`runtime::main_interpreter_id`]. pub is_main: bool, pub config: PyConfig, pub module_defs: BTreeMap<&'static str, &'static builtins::PyModuleDef>, From 57428cd92e4eca2df9d2818ae78847423ce86199 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sat, 15 Aug 2026 23:45:51 +0900 Subject: [PATCH 13/20] vm: gate interpreter registration on stop-the-world admission MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A collection snapshots the registry, stops the interpreters it found, and then reads tracked objects with their threads parked. An interpreter that registered after the snapshot was taken was absent from it, so nothing stopped it and its bootstrap ran Python — allocating and mutating the shared generation lists — underneath that scan. Registration and the stop now share a process-global gate: the collection holds it from before the snapshot until the restart, and registration takes it around the registry insert. The insert runs detached, since a thread that waited for the gate, or re-attached while holding it, would leave the stop it waits for no safepoint to complete at. Assisted-by: Claude --- crates/vm/src/gc_state.rs | 9 +++++ crates/vm/src/vm/interpreter.rs | 38 ++++++++++++++++++ crates/vm/src/vm/runtime.rs | 69 ++++++++++++++++++++++++++------- 3 files changed, 101 insertions(+), 15 deletions(-) diff --git a/crates/vm/src/gc_state.rs b/crates/vm/src/gc_state.rs index 7038a181ab6..8d10b5d3780 100644 --- a/crates/vm/src/gc_state.rs +++ b/crates/vm/src/gc_state.rs @@ -180,6 +180,11 @@ struct CollectStopTheWorld { /// interpreter's objects, and so removes them from these lists — happens /// after the collection has let go of the generation locks. stopped: Vec>, + /// Keeps interpreters from registering between the snapshot below and the + /// restart. One registered in that window would be missing from `stopped`, + /// so its bootstrap would keep running — and mutating the shared generation + /// lists — while this collection reads them. + admission: Option>, restarted: bool, } @@ -195,6 +200,7 @@ impl CollectStopTheWorld { if !crate::vm::thread::current_vm_is_set() { return Self { stopped: Vec::new(), + admission: None, restarted: true, }; } @@ -205,6 +211,7 @@ impl CollectStopTheWorld { // and their exclusion held forever. let mut guard = Self { stopped: Vec::new(), + admission: Some(crate::vm::runtime::lock_admission_for_stop()), restarted: false, }; for state in crate::vm::runtime::live_interpreter_states() { @@ -225,6 +232,8 @@ impl CollectStopTheWorld { for state in self.stopped.iter().rev() { state.stop_the_world.start_the_world(state); } + // Nothing is parked any more, so registration may resume. + self.admission = None; } /// Whether this collection actually stopped the world. diff --git a/crates/vm/src/vm/interpreter.rs b/crates/vm/src/vm/interpreter.rs index 969197bb290..46c10d2e6c9 100644 --- a/crates/vm/src/vm/interpreter.rs +++ b/crates/vm/src/vm/interpreter.rs @@ -836,6 +836,44 @@ mod tests { } } + /// A collection snapshots the registry and then reads tracked objects with + /// the interpreters it found parked. An interpreter that registered inside + /// that window would be missing from the snapshot, so nothing would stop it + /// and its bootstrap would run under the scan; registration therefore waits + /// for the stop to end. + #[cfg(feature = "threading")] + #[test] + fn registering_waits_for_an_in_flight_stop() { + use core::time::Duration; + use std::sync::mpsc; + + // Stands in for a collector between its snapshot and its restart. + let admission = runtime::lock_admission_for_stop(); + + let (tx, rx) = mpsc::channel(); + let worker = std::thread::spawn(move || { + let interp = Interpreter::without_stdlib(Default::default()); + tx.send(interp.id()).expect("receiver is alive"); + interp + }); + + assert!( + matches!( + rx.recv_timeout(Duration::from_millis(200)), + Err(mpsc::RecvTimeoutError::Timeout) + ), + "an interpreter registered while a stop-the-world was in flight" + ); + + drop(admission); + let id = rx + .recv_timeout(Duration::from_secs(30)) + .expect("registration proceeds once the world restarts"); + assert!(runtime::lookup_interpreter(id).is_some()); + drop(worker.join().expect("worker did not panic")); + wait_until_unregistered(id); + } + /// Dropping a subinterpreter releases it; main remains. #[test] fn drop_subinterpreter_unregisters() { diff --git a/crates/vm/src/vm/runtime.rs b/crates/vm/src/vm/runtime.rs index 13005b616bd..7c5168c45b1 100644 --- a/crates/vm/src/vm/runtime.rs +++ b/crates/vm/src/vm/runtime.rs @@ -134,10 +134,53 @@ pub(crate) fn alloc_interpreter_id() -> i64 { registry().next_id.fetch_add(1, Ordering::Relaxed) } +/// Gate between registering an interpreter and a collection's stop-the-world. +/// +/// A collection snapshots the registry, stops every interpreter in the +/// snapshot, and then reads tracked objects with those threads parked. An +/// interpreter that registered after the snapshot was taken would not be in it, +/// so nothing would stop it, and its bootstrap — which runs Python and mutates +/// the shared generation lists — would run underneath that scan. Registration +/// therefore waits for an in-flight stop to end; the next collection's snapshot +/// then contains the new interpreter. +fn admission() -> &'static Mutex<()> { + static ADMISSION: std::sync::OnceLock> = std::sync::OnceLock::new(); + ADMISSION.get_or_init(|| Mutex::new(())) +} + +/// Take the admission gate for the duration of a stop-the-world. +#[cfg(feature = "threading")] +pub(crate) fn lock_admission_for_stop() -> parking_lot::MutexGuard<'static, ()> { + admission().lock() +} + +/// Add the registry entry, behind the admission gate. +/// +/// Only ever called with this thread detached, because the gate is held across +/// a stop-the-world: an attached thread waiting here, or re-attaching while +/// holding the gate, would leave that stop no safepoint to complete at. Nothing +/// under the gate blocks or allocates a tracked object, so this cannot re-enter +/// the collection it waits for. +fn insert_registry_entry(state: &PyRc) { + let _admission = admission().lock(); + let mut entries = registry().entries.lock(); + // Entries are weak and an interpreter's lifetime is decided by its last + // `PyRc` — which outlives the `Interpreter` handle whenever + // `new_thread()` workers are still running — so nothing removes them at a + // fixed point. Reap the dead ones here to bound the table instead. + entries.retain(|_, entry| entry.state.strong_count() > 0); + entries.insert( + state.interpreter_id, + RegistryEntry { + whence: state.whence, + state: PyRc::downgrade(state), + }, + ); +} + /// Register an interpreter state in the registry. pub(crate) fn register_interpreter(state: &PyRc) { let id = state.interpreter_id; - let whence = state.whence; if state.is_main { // First `is_main` interpreter defines the main for `get_main()`. // Additional top-level Interpreters (embedding) keep their own `is_main` @@ -149,19 +192,14 @@ pub(crate) fn register_interpreter(state: &PyRc) { Ordering::Relaxed, ); } - let mut entries = registry().entries.lock(); - // Entries are weak and an interpreter's lifetime is decided by its last - // `PyRc` — which outlives the `Interpreter` handle whenever - // `new_thread()` workers are still running — so nothing removes them at a - // fixed point. Reap the dead ones here to bound the table instead. - entries.retain(|_, entry| entry.state.strong_count() > 0); - entries.insert( - id, - RegistryEntry { - whence, - state: PyRc::downgrade(state), - }, - ); + // A subinterpreter is registered by a thread that is running its parent, so + // detach for the whole insert rather than only for the wait. + let detached = crate::vm::thread::try_with_current_vm(|vm| { + vm.allow_threads(|| insert_registry_entry(state)) + }); + if detached.is_none() { + insert_registry_entry(state); + } } /// Look up a live interpreter state by id. @@ -206,12 +244,13 @@ pub fn interpreter_count() -> usize { /// /// # Safety /// Must only be called after `fork()` in the child process, when no other -/// threads exist and the calling thread holds neither lock. +/// threads exist and the calling thread holds none of these locks. #[cfg(all(unix, feature = "threading"))] pub unsafe fn reinit_after_fork() { unsafe { crate::common::lock::reinit_mutex_after_fork(®istry().entries); crate::common::lock::reinit_mutex_after_fork(owned_interpreters()); + crate::common::lock::reinit_mutex_after_fork(admission()); } } From 8d07831d8828ac6a84b2ee61242da5d99fbed7f4 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 16 Aug 2026 03:45:12 +0900 Subject: [PATCH 14/20] gc: keep the tracking counters off the barrier path MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every tracked allocation and every free went through sequentially consistent counter updates, and each free through a `fetch_update` CAS loop. The counters drive only the gen0 threshold and `gc.get_count()`, and the generation locks — not the counters — order the list changes they describe, so they are relaxed now and the decrement is a load plus a conditional `fetch_sub`. `is_enabled` and `threshold`, both read once per allocation, are relaxed for the same reason: an allocation racing `gc.disable()` may use either value. Drop `alloc_count`, which nothing has ever read. Assisted-by: Claude --- crates/vm/src/gc_state.rs | 64 +++++++++++++++++++++------------------ 1 file changed, 35 insertions(+), 29 deletions(-) diff --git a/crates/vm/src/gc_state.rs b/crates/vm/src/gc_state.rs index 8d10b5d3780..a9b8c7be171 100644 --- a/crates/vm/src/gc_state.rs +++ b/crates/vm/src/gc_state.rs @@ -83,12 +83,14 @@ impl GcGeneration { } } + /// Relaxed: this is policy read once per allocation, and a collection + /// racing `gc.set_threshold()` may use either value. pub fn threshold(&self) -> u32 { - self.threshold.load(Ordering::SeqCst) + self.threshold.load(Ordering::Relaxed) } pub fn set_threshold(&self, value: u32) { - self.threshold.store(value, Ordering::SeqCst); + self.threshold.store(value, Ordering::Relaxed); } pub fn stats(&self) -> GcStats { @@ -134,9 +136,9 @@ impl GcGeneration { /// empties its own interpreter's objects; another interpreter's stay behind with /// the count already zeroed, and untracking one of those must not wrap. fn release_count(count: &AtomicUsize) { - let _ = count.fetch_update(Ordering::SeqCst, Ordering::SeqCst, |count| { - Some(count.saturating_sub(1)) - }); + if count.load(Ordering::Relaxed) > 0 { + count.fetch_sub(1, Ordering::Relaxed); + } } /// Whether `owner`'s collections act on `obj`. @@ -263,13 +265,16 @@ pub struct GcState { /// Frozen/permanent objects (excluded from normal GC) permanent_list: PyRwLock>, /// Number of tracked objects per generation, across all interpreters. + /// + /// Advisory: they drive the collection threshold and `gc.get_count()`, and + /// the generation locks — not these counters — order the list changes they + /// describe. Every access is therefore relaxed, which keeps the tracking and + /// untracking of every object off the barrier path. counts: [AtomicUsize; 3], - /// Number of frozen objects. + /// Number of frozen objects. Advisory, like `counts`. permanent_count: AtomicUsize, /// Mutex for collection (prevents concurrent collections) collecting: PyMutex<()>, - /// Allocation counter for gen0 - alloc_count: AtomicUsize, /// Next `gc_owner` tag to hand to an interpreter. next_owner: AtomicU16, /// Tags of interpreters that are gone. Their objects outlived them, so a @@ -308,7 +313,6 @@ impl GcState { ], permanent_count: AtomicUsize::new(0), collecting: PyMutex::new(()), - alloc_count: AtomicUsize::new(0), next_owner: AtomicU16::new(GC_NO_OWNER + 1), retired: PyMutex::new(Vec::new()), } @@ -341,9 +345,9 @@ impl GcState { /// per interpreter. pub fn get_count(&self) -> (usize, usize, usize) { ( - self.counts[0].load(Ordering::SeqCst), - self.counts[1].load(Ordering::SeqCst), - self.counts[2].load(Ordering::SeqCst), + self.counts[0].load(Ordering::Relaxed), + self.counts[1].load(Ordering::Relaxed), + self.counts[2].load(Ordering::Relaxed), ) } @@ -359,8 +363,7 @@ impl GcState { obj_ref.set_gc_owner(owner); self.generation_lists[0].write().push_front(obj); - self.counts[0].fetch_add(1, Ordering::SeqCst); - self.alloc_count.fetch_add(1, Ordering::SeqCst); + self.counts[0].fetch_add(1, Ordering::Relaxed); } /// Untrack an object (remove from GC lists). @@ -453,7 +456,7 @@ impl GcState { } // Check gen0 threshold - let count0 = self.counts[0].load(Ordering::SeqCst) as u32; + let count0 = self.counts[0].load(Ordering::Relaxed) as u32; let threshold0 = gc.generations[0].threshold(); if threshold0 > 0 && count0 >= threshold0 { #[cfg(feature = "threading")] @@ -585,7 +588,7 @@ impl GcState { // For gen2 (oldest), survivors stay in-place so don't reset gen2 count. let reset_end = if generation >= 2 { 2 } else { generation + 1 }; for i in 0..reset_end { - self.counts[i].store(0, Ordering::SeqCst); + self.counts[i].store(0, Ordering::Relaxed); } let duration = elapsed_secs(start_time); @@ -761,7 +764,7 @@ impl GcState { self.promote_survivors(generation, &survivor_refs); let reset_end = if generation >= 2 { 2 } else { generation + 1 }; for i in 0..reset_end { - self.counts[i].store(0, Ordering::SeqCst); + self.counts[i].store(0, Ordering::Relaxed); } let duration = elapsed_secs(start_time); @@ -784,7 +787,7 @@ impl GcState { self.promote_survivors(generation, &survivor_refs); let reset_end = if generation >= 2 { 2 } else { generation + 1 }; for i in 0..reset_end { - self.counts[i].store(0, Ordering::SeqCst); + self.counts[i].store(0, Ordering::Relaxed); } let duration = elapsed_secs(start_time); @@ -1013,7 +1016,7 @@ impl GcState { // For gen2 (oldest), survivors stay in-place so don't reset gen2 count. let reset_end = if generation >= 2 { 2 } else { generation + 1 }; for i in 0..reset_end { - self.counts[i].store(0, Ordering::SeqCst); + self.counts[i].store(0, Ordering::Relaxed); } let duration = elapsed_secs(start_time); @@ -1064,7 +1067,7 @@ impl GcState { release_count(&self.counts[src_gen]); dst.push_front(ptr); - self.counts[next_gen].fetch_add(1, Ordering::SeqCst); + self.counts[next_gen].fetch_add(1, Ordering::Relaxed); obj.set_gc_generation(next_gen as u8); } @@ -1074,7 +1077,7 @@ impl GcState { /// Get count of frozen objects pub fn get_freeze_count(&self) -> usize { - self.permanent_count.load(Ordering::SeqCst) + self.permanent_count.load(Ordering::Relaxed) } /// Freeze the objects `owner` could collect (move them to the permanent @@ -1102,7 +1105,7 @@ impl GcState { } } - self.permanent_count.fetch_add(count, Ordering::SeqCst); + self.permanent_count.fetch_add(count, Ordering::Relaxed); } /// Unfreeze the objects `owner` froze (move them from permanent to gen2). @@ -1127,13 +1130,13 @@ impl GcState { count += 1; } let _ = self.permanent_count.fetch_update( - Ordering::SeqCst, - Ordering::SeqCst, + Ordering::Relaxed, + Ordering::Relaxed, |permanent| Some(permanent.saturating_sub(count)), ); } - self.counts[2].fetch_add(count, Ordering::SeqCst); + self.counts[2].fetch_add(count, Ordering::Relaxed); } /// Reset all locks to unlocked state after fork(). @@ -1201,19 +1204,22 @@ impl GcInterpreterState { } } - /// Check if GC is enabled + /// Check if GC is enabled. + /// + /// Relaxed, like [`GcGeneration::threshold`]: it is read once per + /// allocation, and an allocation racing `gc.disable()` may use either value. pub fn is_enabled(&self) -> bool { - self.enabled.load(Ordering::SeqCst) + self.enabled.load(Ordering::Relaxed) } /// Enable GC pub fn enable(&self) { - self.enabled.store(true, Ordering::SeqCst); + self.enabled.store(true, Ordering::Relaxed); } /// Disable GC pub fn disable(&self) { - self.enabled.store(false, Ordering::SeqCst); + self.enabled.store(false, Ordering::Relaxed); } /// Get debug flags From 4e518d362e4c23819f09db7c33b24fd2036ede32 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 16 Aug 2026 03:46:04 +0900 Subject: [PATCH 15/20] dict: settle lookups and iteration steps under one read guard A lookup probed under a read guard, dropped it with the matched entry in hand, and then took the lock again to re-find the entry. `lookup_extract` reads the entry while the probe still holds the guard when key identity settles the match, which is the case that cannot run Python. Dict and set iterators took one lock to compare the size and another to read the entry, then cloned both the key and the value even though a keys or values view keeps only one of them. `next_entry_checked` does the size check and the read under one guard, and clones through a per-view projection. A store that missed its inline-cache hint re-probed the dict afterwards only to recover the entry index the store had just computed; `unchecked_push` reports that index instead. Assisted-by: Claude --- crates/vm/src/builtins/dict.rs | 65 +++++++++------- crates/vm/src/builtins/set.rs | 14 ++-- crates/vm/src/dict_inner.rs | 138 +++++++++++++++++++++++++++++---- 3 files changed, 171 insertions(+), 46 deletions(-) diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index 0acc3e95f89..d2b9dea31fa 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -1090,6 +1090,7 @@ macro_rules! dict_view { $class_name: literal, $iter_class_name: literal, $reverse_iter_class_name: literal, + $project_fn: expr, $result_fn: expr ) => { #[pyclass(module = false, name = $class_name)] @@ -1112,7 +1113,7 @@ macro_rules! dict_view { } fn item(vm: &VirtualMachine, key: PyObjectRef, value: PyObjectRef) -> PyObjectRef { - $result_fn(vm, key, value) + $result_fn(vm, $project_fn(&key, &value)) } fn __reversed__(&self) -> Self::ReverseIter { @@ -1198,7 +1199,7 @@ macro_rules! dict_view { while let Some((next_position, key, value)) = dict.entries.next_entry(position) { - entries.push(($result_fn)(vm, key, value)); + entries.push(($result_fn)(vm, ($project_fn)(&key, &value))); position = next_position; } entries @@ -1215,18 +1216,22 @@ macro_rules! dict_view { fn next(zelf: &Py, vm: &VirtualMachine) -> PyResult { let mut internal = zelf.internal.lock(); let next = if let IterStatus::Active(dict) = &internal.status { - if dict.entries.has_changed_size(&zelf.size) { - internal.status = IterStatus::Exhausted; - return Err( - vm.new_runtime_error("dictionary changed size during iteration") - ); - } - match dict.entries.next_entry(internal.position) { - Some((position, key, value)) => { + match dict.entries.next_entry_checked( + internal.position, + &zelf.size, + $project_fn, + ) { + Err(dict_inner::DictChanged) => { + internal.status = IterStatus::Exhausted; + return Err( + vm.new_runtime_error("dictionary changed size during iteration") + ); + } + Ok(Some((position, item))) => { internal.position = position; - PyIterReturn::Return(($result_fn)(vm, key, value)) + PyIterReturn::Return(($result_fn)(vm, item)) } - None => { + Ok(None) => { internal.status = IterStatus::Exhausted; PyIterReturn::StopIteration(None) } @@ -1274,7 +1279,7 @@ macro_rules! dict_view { while let Some((found_index, key, value)) = dict.entries.prev_entry(position) { - entries.push(($result_fn)(vm, key, value)); + entries.push(($result_fn)(vm, ($project_fn)(&key, &value))); if found_index == 0 { break; } @@ -1301,22 +1306,26 @@ macro_rules! dict_view { fn next(zelf: &Py, vm: &VirtualMachine) -> PyResult { let mut internal = zelf.internal.lock(); let next = if let IterStatus::Active(dict) = &internal.status { - if dict.entries.has_changed_size(&zelf.size) { - internal.status = IterStatus::Exhausted; - return Err( - vm.new_runtime_error("dictionary changed size during iteration") - ); - } - match dict.entries.prev_entry(internal.position) { - Some((found_index, key, value)) => { + match dict.entries.prev_entry_checked( + internal.position, + &zelf.size, + $project_fn, + ) { + Err(dict_inner::DictChanged) => { + internal.status = IterStatus::Exhausted; + return Err( + vm.new_runtime_error("dictionary changed size during iteration") + ); + } + Ok(Some((found_index, item))) => { if found_index == 0 { internal.status = IterStatus::Exhausted; } else { internal.position = found_index - 1; } - PyIterReturn::Return(($result_fn)(vm, key, value)) + PyIterReturn::Return(($result_fn)(vm, item)) } - None => { + Ok(None) => { internal.status = IterStatus::Exhausted; PyIterReturn::StopIteration(None) } @@ -1340,7 +1349,8 @@ dict_view! { "dict_keys", "dict_keyiterator", "dict_reversekeyiterator", - |_vm: &VirtualMachine, key: PyObjectRef, _value: PyObjectRef| key + |key: &PyObjectRef, _value: &PyObjectRef| key.clone(), + |_vm: &VirtualMachine, key: PyObjectRef| key } dict_view! { @@ -1353,7 +1363,8 @@ dict_view! { "dict_values", "dict_valueiterator", "dict_reversevalueiterator", - |_vm: &VirtualMachine, _key: PyObjectRef, value: PyObjectRef| value + |_key: &PyObjectRef, value: &PyObjectRef| value.clone(), + |_vm: &VirtualMachine, value: PyObjectRef| value } dict_view! { @@ -1366,7 +1377,9 @@ dict_view! { "dict_items", "dict_itemiterator", "dict_reverseitemiterator", - |vm: &VirtualMachine, key: PyObjectRef, value: PyObjectRef| + |key: &PyObjectRef, value: &PyObjectRef| (key.clone(), value.clone()), + // Builds a tuple, so it runs after the dict's read guard is released. + |vm: &VirtualMachine, (key, value): (PyObjectRef, PyObjectRef)| vm.new_tuple((key, value)).into() } diff --git a/crates/vm/src/builtins/set.rs b/crates/vm/src/builtins/set.rs index 860f86f4319..d737612b158 100644 --- a/crates/vm/src/builtins/set.rs +++ b/crates/vm/src/builtins/set.rs @@ -1533,16 +1533,16 @@ impl IterNext for PySetIterator { fn next(zelf: &crate::Py, vm: &VirtualMachine) -> PyResult { let mut internal = zelf.internal.lock(); let next = if let IterStatus::Active(dict) = &internal.status { - if dict.has_changed_size(&zelf.size) { - internal.status = IterStatus::Exhausted; - return Err(vm.new_runtime_error("set changed size during iteration")); - } - match dict.next_entry(internal.position) { - Some((position, key, _)) => { + match dict.next_entry_checked(internal.position, &zelf.size, |key, ()| key.clone()) { + Err(crate::dict_inner::DictChanged) => { + internal.status = IterStatus::Exhausted; + return Err(vm.new_runtime_error("set changed size during iteration")); + } + Ok(Some((position, key))) => { internal.position = position; PyIterReturn::Return(key) } - None => { + Ok(None) => { internal.status = IterStatus::Exhausted; PyIterReturn::StopIteration(None) } diff --git a/crates/vm/src/dict_inner.rs b/crates/vm/src/dict_inner.rs index acb8ad107a7..76d2c50f0cb 100644 --- a/crates/vm/src/dict_inner.rs +++ b/crates/vm/src/dict_inner.rs @@ -237,6 +237,10 @@ pub struct DictSize { filled: usize, } +/// The dict was resized under an iterator holding an older [`DictSize`]. +#[derive(Debug)] +pub(crate) struct DictChanged; + struct GenIndexes { idx: HashIndex, perturb: HashValue, @@ -306,7 +310,7 @@ impl DictInner { key: PyObjectRef, value: T, index_entry: IndexEntry, - ) { + ) -> usize { let entry = DictEntry { hash: hash_value, key, @@ -327,6 +331,9 @@ impl DictInner { self.resize(new_size) } } + // A resize keeps entry positions and rewrites only the index-index, so + // this stays the entry's index afterwards. + entry_index } const fn size(&self) -> DictSize { @@ -468,7 +475,26 @@ impl Dict { where K: DictKey + ?Sized, { - let _removed = loop { + self.insert_known_hash_indexed(vm, key, hash, value)?; + Ok(()) + } + + /// [`Self::insert_known_hash`], also reporting the entry index it stored to. + /// + /// The index doubles as a `hint` for [`Self::get_hint`] / + /// [`Self::insert_with_hint`], so a caller that wants one gets it from the + /// store itself instead of probing the dict a second time. + fn insert_known_hash_indexed( + &self, + vm: &VirtualMachine, + key: &K, + hash: HashValue, + value: T, + ) -> PyResult + where + K: DictKey + ?Sized, + { + let (stored_index, _removed) = loop { let (entry_index, index_index) = self.lookup(vm, key, hash, None)?; let mut inner = self.write(); if let Some(index) = entry_index.index() { @@ -488,7 +514,7 @@ impl Dict { if entry.index == index_index { let removed = core::mem::replace(&mut entry.value, value); // defer dec RC - break Some(removed); + break (index, Some(removed)); } else { // stuff shifted around, let's try again } @@ -502,11 +528,17 @@ impl Dict { continue; } self.invalidate_keys_version(); - inner.unchecked_push(index_index, hash, key.to_pyobject(vm), value, entry_index); - break None; + let stored = inner.unchecked_push( + index_index, + hash, + key.to_pyobject(vm), + value, + entry_index, + ); + break (stored, None); } }; - Ok(()) + Ok(stored_index) } pub(crate) fn contains( @@ -609,8 +641,9 @@ impl Dict { _ => value, } }; - self.insert(vm, key, value)?; - self.hint_for_key(vm, key) + let hash = key.key_hash(vm)?; + let stored = self.insert_known_hash_indexed(vm, key, hash, value)?; + Ok(u16::try_from(stored).ok()) } /// Fast path lookup using a cached entry index (`hint`). @@ -663,7 +696,12 @@ impl Dict { hash: HashValue, ) -> PyResult> { let ret = loop { - let (entry, index_index) = self.lookup(vm, key, hash, None)?; + let (entry, index_index) = + match self.lookup_extract(vm, key, hash, None, |entry| entry.value.clone())? { + // Read under the probe's own guard: nothing to re-check. + (_, Some(value)) => break Some(value), + (lookup, None) => lookup, + }; if let Some(index) = entry.index() { let inner = self.read(); if let Some(entry) = inner.get_entry_checked(index, index_index) { @@ -914,6 +952,58 @@ impl Dict { self.read().size() } + /// Step to the first live entry at or after `position`, verifying the size + /// against `old` under the same read guard. + /// + /// `project` runs under that guard, so it must not run Python or take + /// another dict lock; it is there so an iterator clones only the field it + /// keeps rather than both the key and the value. + pub(crate) fn next_entry_checked( + &self, + mut position: EntryIndex, + old: &DictSize, + project: impl FnOnce(&PyObjectRef, &T) -> R, + ) -> Result, DictChanged> { + let inner = self.read(); + if inner.size() != *old { + return Err(DictChanged); + } + loop { + let Some(entry) = inner.entries.get(position) else { + return Ok(None); + }; + position += 1; + if let Some(entry) = entry { + return Ok(Some((position, project(&entry.key, &entry.value)))); + } + } + } + + /// [`Self::next_entry_checked`] in reverse. + pub(crate) fn prev_entry_checked( + &self, + mut position: EntryIndex, + old: &DictSize, + project: impl FnOnce(&PyObjectRef, &T) -> R, + ) -> Result, DictChanged> { + let inner = self.read(); + if inner.size() != *old { + return Err(DictChanged); + } + loop { + let Some(entry) = inner.entries.get(position) else { + return Ok(None); + }; + if let Some(entry) = entry { + return Ok(Some((position, project(&entry.key, &entry.value)))); + } + if position == 0 { + return Ok(None); + } + position -= 1; + } + } + pub(crate) fn next_entry(&self, mut position: EntryIndex) -> Option<(usize, PyObjectRef, T)> { let inner = self.read(); loop { @@ -1000,8 +1090,30 @@ impl Dict { vm: &VirtualMachine, key: &K, hash_value: HashValue, - mut lock: Option>>, + lock: Option>>, ) -> PyResult { + let (ret, _) = self.lookup_extract(vm, key, hash_value, lock, |_| ())?; + Ok(ret) + } + + /// [`Self::lookup`], additionally reading the matched entry when the probe + /// settles it by key identity. + /// + /// That is the common case, and it is decided while the read guard is still + /// held — so a caller that only wants the entry's value gets it here instead + /// of taking the lock a second time to re-find what the probe already had. + /// `extract` therefore runs under the guard and must not run Python. It is + /// not called when the key had to be compared with `key_eq`, which does run + /// Python and so releases the guard first. + #[cfg_attr(feature = "flame-it", flame("Dict"))] + fn lookup_extract( + &self, + vm: &VirtualMachine, + key: &K, + hash_value: HashValue, + mut lock: Option>>, + extract: impl Fn(&DictEntry) -> R, + ) -> PyResult<(LookupResult, Option)> { let mut idxs = None; let mut free_slot = None; let ret = 'outer: loop { @@ -1031,7 +1143,7 @@ impl Dict { Some(free) => (IndexEntry::DUMMY, free), None => (IndexEntry::FREE, index_index), }; - return Ok(idxs); + return Ok((idxs, None)); } idx => { let entry = unsafe { @@ -1047,7 +1159,7 @@ impl Dict { reason = "Keeping the empty `else` block here for documentation" )] if key.key_is(&entry.key) { - break 'outer ret; + return Ok((ret, Some(extract(entry)))); } else if entry.hash == hash_value { break (entry.key.clone(), ret); } else { @@ -1072,7 +1184,7 @@ impl Dict { // warn!("Perturb value: {}", i); }; - Ok(ret) + Ok((ret, None)) } // returns Err(()) if changed since lookup From eb94dafe3fb87a1c15f05a1570af30fb2429867a Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 16 Aug 2026 03:46:20 +0900 Subject: [PATCH 16/20] vm: build a call's argument vector once MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The specialized CALL handlers collected the positional arguments into one vector and then copied them into a second one to put `self` in front, so every specialized builtin, method-descriptor, class and non-Python call paid two allocations, a copy and two frees. The arguments are already laid out on the value stack in vectorcall order, so `take_call_args` fills one vector by index — the shape `execute_call_vectorcall` already used. `vectorcall_native_function` and the keyword path of `vectorcall_function` then cloned that vector again to build `FuncArgs`; both now move it in through `from_vectorcall_owned`, as the other vectorcall slots do. Assisted-by: Claude --- crates/vm/src/builtins/builtin_func.rs | 4 +- crates/vm/src/builtins/function.rs | 4 +- crates/vm/src/frame.rs | 183 ++++++++++--------------- 3 files changed, 76 insertions(+), 115 deletions(-) diff --git a/crates/vm/src/builtins/builtin_func.rs b/crates/vm/src/builtins/builtin_func.rs index eabe8d4ea27..b34447b79bf 100644 --- a/crates/vm/src/builtins/builtin_func.rs +++ b/crates/vm/src/builtins/builtin_func.rs @@ -247,9 +247,9 @@ fn vectorcall_native_function( let mut all_args = Vec::with_capacity(args.len() + 1); all_args.push(self_obj); all_args.extend(args); - FuncArgs::from_vectorcall(&all_args, nargs + 1, kwnames) + FuncArgs::from_vectorcall_owned(all_args, nargs + 1, kwnames) } else { - FuncArgs::from_vectorcall(&args, nargs, kwnames) + FuncArgs::from_vectorcall_owned(args, nargs, kwnames) }; (zelf.value.func)(vm, func_args) diff --git a/crates/vm/src/builtins/function.rs b/crates/vm/src/builtins/function.rs index 3674f8882b1..a5342d1df3a 100644 --- a/crates/vm/src/builtins/function.rs +++ b/crates/vm/src/builtins/function.rs @@ -1635,7 +1635,7 @@ pub(crate) fn vectorcall_function( let has_kwargs = kwnames.is_some_and(|kw| !kw.is_empty()); if zelf.is_jitted() { let func_args = if has_kwargs { - FuncArgs::from_vectorcall(&args, nargs, kwnames) + FuncArgs::from_vectorcall_owned(args, nargs, kwnames) } else { args.truncate(nargs); FuncArgs::from(args) @@ -1667,7 +1667,7 @@ pub(crate) fn vectorcall_function( // SLOW PATH: construct FuncArgs from owned Vec and delegate to invoke() let func_args = if has_kwargs { - FuncArgs::from_vectorcall(&args, nargs, kwnames) + FuncArgs::from_vectorcall_owned(args, nargs, kwnames) } else { args.truncate(nargs); FuncArgs::from(args) diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index 8058e805143..e4cc3173822 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -6094,15 +6094,8 @@ impl ExecutingFrame<'_> { | PyMethodFlags::O | PyMethodFlags::KEYWORDS); if call_conv == PyMethodFlags::O && effective_nargs == 1 { - let nargs_usize = nargs as usize; - let pos_args: Vec = self.pop_multiple(nargs_usize).collect(); - let self_or_null = self.pop_value_opt(); - let callable = self.pop_value(); - let mut args_vec = Vec::with_capacity(effective_nargs as usize); - if let Some(self_val) = self_or_null { - args_vec.push(self_val); - } - args_vec.extend(pos_args); + let (callable, args_vec) = self.take_call_args(nargs as usize); + debug_assert_eq!(args_vec.len(), effective_nargs as usize); let result = callable.vectorcall(args_vec, effective_nargs as usize, None, vm)?; self.push_value(result); @@ -6128,15 +6121,8 @@ impl ExecutingFrame<'_> { | PyMethodFlags::O | PyMethodFlags::KEYWORDS); if call_conv == PyMethodFlags::FASTCALL { - let nargs_usize = nargs as usize; - let pos_args: Vec = self.pop_multiple(nargs_usize).collect(); - let self_or_null = self.pop_value_opt(); - let callable = self.pop_value(); - let mut args_vec = Vec::with_capacity(effective_nargs as usize); - if let Some(self_val) = self_or_null { - args_vec.push(self_val); - } - args_vec.extend(pos_args); + let (callable, args_vec) = self.take_call_args(nargs as usize); + debug_assert_eq!(args_vec.len(), effective_nargs as usize); let result = callable.vectorcall(args_vec, effective_nargs as usize, None, vm)?; self.push_value(result); @@ -6164,18 +6150,8 @@ impl ExecutingFrame<'_> { if self.specialization_call_recursion_guard(vm) { return self.execute_call_vectorcall(nargs, vm); } - let nargs_usize = nargs as usize; - let pos_args: Vec = self.pop_multiple(nargs_usize).collect(); - let self_or_null = self.pop_value_opt(); - let callable = self.pop_value(); - let (args_vec, effective_nargs) = if let Some(self_val) = self_or_null { - let mut v = Vec::with_capacity(nargs_usize + 1); - v.push(self_val); - v.extend(pos_args); - (v, nargs_usize + 1) - } else { - (pos_args, nargs_usize) - }; + let (callable, args_vec) = self.take_call_args(nargs as usize); + let effective_nargs = args_vec.len(); let result = vectorcall_function(&callable, args_vec, effective_nargs, None, vm)?; self.push_value(result); @@ -6214,12 +6190,11 @@ impl ExecutingFrame<'_> { return self.execute_call_vectorcall(nargs, vm); } let nargs_usize = nargs as usize; - let pos_args: Vec = self.pop_multiple(nargs_usize).collect(); - self.pop_value_opt(); // null (self_or_null) - self.pop_value(); // callable (bound method) let mut args_vec = Vec::with_capacity(nargs_usize + 1); args_vec.push(bound_self); - args_vec.extend(pos_args); + args_vec.extend(self.pop_multiple(nargs_usize)); + self.pop_value_opt(); // null (self_or_null) + self.pop_value(); // callable (bound method) let result = vectorcall_function( &bound_function, args_vec, @@ -6301,15 +6276,8 @@ impl ExecutingFrame<'_> { .is_some_and(|self_obj| self_obj.class().is(descr.objclass)) { let func = descr.method.func; - let positional_args: Vec = - self.pop_multiple(nargs as usize).collect(); - let self_or_null = self.pop_value_opt(); - self.pop_value(); // callable - let mut all_args = Vec::with_capacity(total_nargs as usize); - if let Some(self_val) = self_or_null { - all_args.push(self_val); - } - all_args.extend(positional_args); + let (_callable, all_args) = self.take_call_args(nargs as usize); + debug_assert_eq!(all_args.len(), total_nargs as usize); let args = FuncArgs { args: all_args, kwargs: Default::default(), @@ -6348,15 +6316,8 @@ impl ExecutingFrame<'_> { .is_some_and(|self_obj| self_obj.class().is(descr.objclass)) { let func = descr.method.func; - let positional_args: Vec = - self.pop_multiple(nargs as usize).collect(); - let self_or_null = self.pop_value_opt(); - self.pop_value(); // callable - let mut all_args = Vec::with_capacity(total_nargs as usize); - if let Some(self_val) = self_or_null { - all_args.push(self_val); - } - all_args.extend(positional_args); + let (_callable, all_args) = self.take_call_args(nargs as usize); + debug_assert_eq!(all_args.len(), total_nargs as usize); let args = FuncArgs { args: all_args, kwargs: Default::default(), @@ -6395,15 +6356,8 @@ impl ExecutingFrame<'_> { .is_some_and(|self_obj| self_obj.class().is(descr.objclass)) { let func = descr.method.func; - let positional_args: Vec = - self.pop_multiple(nargs as usize).collect(); - let self_or_null = self.pop_value_opt(); - self.pop_value(); // callable - let mut all_args = Vec::with_capacity(total_nargs as usize); - if let Some(self_val) = self_or_null { - all_args.push(self_val); - } - all_args.extend(positional_args); + let (_callable, all_args) = self.take_call_args(nargs as usize); + debug_assert_eq!(all_args.len(), total_nargs as usize); let args = FuncArgs { args: all_args, kwargs: Default::default(), @@ -6420,22 +6374,9 @@ impl ExecutingFrame<'_> { if let Some(cls) = callable.downcast_ref::() && cls.slots.vectorcall.load().is_some() { - let nargs_usize = nargs as usize; - let pos_args: Vec = self.pop_multiple(nargs_usize).collect(); - let self_or_null = self.pop_value_opt(); - let callable = self.pop_value(); - let self_is_some = self_or_null.is_some(); - let mut args_vec = Vec::with_capacity(nargs_usize + usize::from(self_is_some)); - if let Some(self_val) = self_or_null { - args_vec.push(self_val); - } - args_vec.extend(pos_args); - let result = callable.vectorcall( - args_vec, - nargs_usize + usize::from(self_is_some), - None, - vm, - )?; + let (callable, args_vec) = self.take_call_args(nargs as usize); + let effective_nargs = args_vec.len(); + let result = callable.vectorcall(args_vec, effective_nargs, None, vm)?; self.push_value(result); return Ok(None); } @@ -6520,15 +6461,8 @@ impl ExecutingFrame<'_> { .is_some_and(|self_obj| self_obj.class().is(descr.objclass)) { let func = descr.method.func; - let positional_args: Vec = - self.pop_multiple(nargs as usize).collect(); - let self_or_null = self.pop_value_opt(); - self.pop_value(); // callable - let mut all_args = Vec::with_capacity(total_nargs as usize); - if let Some(self_val) = self_or_null { - all_args.push(self_val); - } - all_args.extend(positional_args); + let (_callable, all_args) = self.take_call_args(nargs as usize); + debug_assert_eq!(all_args.len(), total_nargs as usize); let args = FuncArgs { args: all_args, kwargs: Default::default(), @@ -6557,15 +6491,8 @@ impl ExecutingFrame<'_> { | PyMethodFlags::O | PyMethodFlags::KEYWORDS); if call_conv == (PyMethodFlags::FASTCALL | PyMethodFlags::KEYWORDS) { - let nargs_usize = nargs as usize; - let pos_args: Vec = self.pop_multiple(nargs_usize).collect(); - let self_or_null = self.pop_value_opt(); - let callable = self.pop_value(); - let mut args_vec = Vec::with_capacity(effective_nargs as usize); - if let Some(self_val) = self_or_null { - args_vec.push(self_val); - } - args_vec.extend(pos_args); + let (callable, args_vec) = self.take_call_args(nargs as usize); + debug_assert_eq!(args_vec.len(), effective_nargs as usize); let result = callable.vectorcall(args_vec, effective_nargs as usize, None, vm)?; self.push_value(result); @@ -6589,22 +6516,13 @@ impl ExecutingFrame<'_> { { return self.execute_call_vectorcall(nargs, vm); } - let nargs_usize = nargs as usize; - let pos_args: Vec = self.pop_multiple(nargs_usize).collect(); - let self_or_null = self.pop_value_opt(); - let callable = self.pop_value(); - let mut args_vec = - Vec::with_capacity(nargs_usize + usize::from(self_or_null_is_some)); - if let Some(self_val) = self_or_null { - args_vec.push(self_val); - } - args_vec.extend(pos_args); - let result = callable.vectorcall( - args_vec, - nargs_usize + usize::from(self_or_null_is_some), - None, - vm, - )?; + let (callable, args_vec) = self.take_call_args(nargs as usize); + debug_assert_eq!( + args_vec.len(), + nargs as usize + usize::from(self_or_null_is_some) + ); + let effective_nargs = args_vec.len(); + let result = callable.vectorcall(args_vec, effective_nargs, None, vm)?; self.push_value(result); Ok(None) } @@ -11413,6 +11331,49 @@ impl ExecutingFrame<'_> { } } + /// Take a call's `[self_or_null, arg1, ..., argN]` off the stack as one + /// vectorcall argument list, along with the callable underneath them. + /// + /// The stack already holds the arguments in vectorcall order, so filling a + /// single vector by index costs one allocation — collecting the positional + /// arguments first and then pushing `self` in front of them costs two plus + /// a copy. + fn take_call_args(&mut self, nargs: usize) -> (PyObjectRef, Vec) { + let stack_len = self.localsplus.stack_len(); + debug_assert!( + stack_len >= nargs + 2, + "CALL stack underflow: need callable + self_or_null + {nargs} args, have {stack_len}" + ); + let callable_idx = stack_len - nargs - 2; + let self_or_null_idx = callable_idx + 1; + + let self_or_null = self + .localsplus + .stack_index_mut(self_or_null_idx) + .take() + .map(|sr| sr.to_pyobj()); + let mut args = Vec::with_capacity(nargs + usize::from(self_or_null.is_some())); + args.extend(self_or_null); + for stack_idx in self_or_null_idx + 1..stack_len { + let val = self + .localsplus + .stack_index_mut(stack_idx) + .take() + .unwrap() + .to_pyobj(); + args.push(val); + } + + let callable = self + .localsplus + .stack_index_mut(callable_idx) + .take() + .unwrap() + .to_pyobj(); + self.localsplus.stack_truncate(callable_idx); + (callable, args) + } + /// Pop multiple values from the stack. Panics if any slot is NULL. fn pop_multiple(&mut self, count: usize) -> impl ExactSizeIterator + '_ { let stack_len = self.localsplus.stack_len(); From f70db351e2b009044024f24ed9b45df9b401f5c1 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 16 Aug 2026 03:46:36 +0900 Subject: [PATCH 17/20] vm: reach an instance dict without cloning it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The specialized attribute instructions cloned the instance dict — a rwlock round-trip plus a refcount round-trip — for two things that only look at it: `LoadAttrMethodLazyDict` asking whether the dict exists, and the keys-version stamp check that is the whole of `shadowing_instance_attr`'s fast path. Both now read it borrowed, through `has_instance_dict` / `with_instance_dict`. `generic_getattr_opt` probed the dict with the name's `&Wtf8`, which hashes the name on every lookup and can never match a key by pointer. Passing the `Py` uses the string's cached hash and the interned-key identity check, and drops an allocation when a stored key is not an exact `str`. Assisted-by: Claude --- crates/vm/src/frame.rs | 16 +++++++++++----- crates/vm/src/object/core.rs | 32 ++++++++++++++++++++++++++++++++ crates/vm/src/protocol/object.rs | 5 +++-- 3 files changed, 46 insertions(+), 7 deletions(-) diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index e4cc3173822..dd1f6f284fb 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -5271,7 +5271,7 @@ impl ExecutingFrame<'_> { if type_version != 0 && owner.class().tp_version_tag.load(Acquire) == type_version - && owner.dict().is_none() + && !owner.has_instance_dict() && let Some(func) = self.try_read_cached_descriptor(cache_base, type_version) { let owner = self.pop_value(); @@ -8933,13 +8933,19 @@ impl ExecutingFrame<'_> { attr_name: &'static PyStrInterned, vm: &VirtualMachine, ) -> PyResult> { - let Some(dict) = self.top_value().dict() else { - return Ok(None); - }; let stamp = self.code.instructions.read_cache_ptr(cache_base + 3); - if stamp != 0 && stamp == dict.keys_version() as usize { + // Take the stamp check first, on a borrowed dict: a hit is the whole + // fast path, and cloning the dict for it would cost more than the + // comparison it exists to make. + let stamped = self.top_value().with_instance_dict(|dict| { + dict.is_some_and(|d| stamp != 0 && stamp == d.keys_version() as usize) + }); + if stamped { return Ok(None); } + let Some(dict) = self.top_value().dict() else { + return Ok(None); + }; // Take the stamp before probing so it attests the probed key set. let stamp = dict.assign_keys_version(vm); if let Some(value) = dict.get_item_opt(attr_name, vm)? { diff --git a/crates/vm/src/object/core.rs b/crates/vm/src/object/core.rs index 6f8e3090e44..1e36d57ab31 100644 --- a/crates/vm/src/object/core.rs +++ b/crates/vm/src/object/core.rs @@ -1073,6 +1073,16 @@ impl InstanceDict { self.d.read().clone() } + /// Run `f` on the dict without cloning it. + /// + /// For callers that only need to look at the dict — a predicate, a version + /// stamp — this drops the refcount round-trip [`Self::get`] pays. `f` runs + /// under the read guard, so it must not run Python or take this lock again. + #[inline] + pub(crate) fn with(&self, f: impl FnOnce(Option<&Py>) -> R) -> R { + f(self.d.read().as_deref()) + } + #[inline] pub(crate) fn set(&self, d: Option) { self.replace(d); @@ -1638,6 +1648,28 @@ impl PyObject { self.instance_dict().and_then(|d| d.get()) } + /// Whether this object currently has an instance dict, without cloning it. + /// + /// `false` both for an object with no dict slot and for one whose slot is + /// still empty, which is what `dict().is_none()` reports. + #[inline(always)] + pub fn has_instance_dict(&self) -> bool { + self.instance_dict() + .is_some_and(|d| d.with(|dict| dict.is_some())) + } + + /// Run `f` on the instance dict without cloning it; see [`InstanceDict::with`]. + #[inline(always)] + pub(crate) fn with_instance_dict( + &self, + f: impl FnOnce(Option<&Py>) -> R, + ) -> R { + match self.instance_dict() { + Some(d) => d.with(f), + None => f(None), + } + } + /// Set the dict field. Returns `Err(dict)` if this object does not have a dict field /// in the first place. pub fn set_dict(&self, dict: Option) -> Result<(), Option> { diff --git a/crates/vm/src/protocol/object.rs b/crates/vm/src/protocol/object.rs index 4974fca9343..993f3442aa3 100644 --- a/crates/vm/src/protocol/object.rs +++ b/crates/vm/src/protocol/object.rs @@ -230,7 +230,6 @@ impl PyObject { dict: Option, vm: &VirtualMachine, ) -> PyResult> { - let name = name_str.as_wtf8(); let obj_cls = self.class(); let cls_attr_name = vm.ctx.interned_str(name_str); let cls_attr = match cls_attr_name.and_then(|name| obj_cls.get_attr(name)) { @@ -251,7 +250,9 @@ impl PyObject { let dict = dict.or_else(|| self.dict()); let attr = if let Some(dict) = dict { - dict.get_item_opt(name, vm)? + // `Py` rather than its `&Wtf8`: the key type carries the + // cached hash and compares interned keys by pointer. + dict.get_item_opt(name_str, vm)? } else { None }; From 4a76424812ea9fe088c141fdaae262a3dc2a4c7b Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 16 Aug 2026 03:46:47 +0900 Subject: [PATCH 18/20] vm: shorten the per-instruction safepoint and the call preamble MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The dispatch loop asks once per instruction whether stop-the-world wants this thread, and that read went through `CURRENT_THREAD_SLOT` — a `RefCell` borrow, so two stores to thread-local memory around an `Option` test. Cache the `stop_requested` pointer in a plain `Cell` at the same three places the frame pointers are cached, and the safepoint becomes one relaxed load. `lasti` is advanced from the index the loop just read, rather than reloaded to increment it, and `Resume` reads `quickened` before swapping it, so a call to an already-quickened code object costs a load instead of an atomic read-modify-write. An exact-args vectorcall to a Python function built a heap `FrameObject` where the equivalent `invoke` path uses a data stack frame; it now does the same when tracing is off. Assisted-by: Claude --- crates/vm/src/builtins/function.rs | 6 ++++++ crates/vm/src/frame.rs | 13 +++++++++---- crates/vm/src/vm/thread.rs | 24 ++++++++++++++++++++---- 3 files changed, 35 insertions(+), 8 deletions(-) diff --git a/crates/vm/src/builtins/function.rs b/crates/vm/src/builtins/function.rs index a5342d1df3a..9cc100a3a33 100644 --- a/crates/vm/src/builtins/function.rs +++ b/crates/vm/src/builtins/function.rs @@ -1658,6 +1658,12 @@ pub(crate) fn vectorcall_function( // FAST PATH: simple positional-only call, exact arg count. // Move owned args directly into fastlocals — no clone needed. args.truncate(nargs); + if !vm.use_tracing.get() { + // Nothing here can escape the frame, so keep it on the data stack + // instead of allocating a `FrameObject`. Tracing needs the heap + // frame, since it hands the frame to the trace function. + return zelf.invoke_prepared_exact_args(args.into_iter(), vm); + } let frame = zelf.prepare_exact_args_frame(args.into_iter(), vm); let result = vm.run_frame(frame.clone()); diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index dd1f6f284fb..d2c14498f17 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -2987,8 +2987,9 @@ impl ExecutingFrame<'_> { // Advance lasti past the current instruction BEFORE firing the // line event. This ensures that f_lineno (which reads // locations[lasti - 1]) returns the line of the instruction - // being traced, not the previous one. - self.update_lasti(|i| *i += 1); + // being traced, not the previous one. Stored from `idx` rather + // than read-modify-written, which would re-load what was just read. + self.lasti.store(idx as u32 + 1, Relaxed); // Fire 'line' trace event when line number changes. // Only fire if this frame has a per-frame trace function set @@ -4803,8 +4804,12 @@ impl ExecutingFrame<'_> { } Instruction::RaiseVarargs { argc: kind } => self.execute_raise(vm, kind.get(arg)), Instruction::Resume { .. } | Instruction::ResumeCheck => { - // Lazy quickening: initialize adaptive counters on first execution - if !self.code.quickened.swap(true, atomic::Ordering::Relaxed) { + // Lazy quickening: initialize adaptive counters on first execution. + // Read before the swap so that the steady state — every call after + // the first — costs a load rather than a read-modify-write. + if !self.code.quickened.load(atomic::Ordering::Relaxed) + && !self.code.quickened.swap(true, atomic::Ordering::Relaxed) + { self.code.instructions.quicken(); atomic::fence(atomic::Ordering::Release); } diff --git a/crates/vm/src/vm/thread.rs b/crates/vm/src/vm/thread.rs index 43d9f06ec7a..c9792fe32a4 100644 --- a/crates/vm/src/vm/thread.rs +++ b/crates/vm/src/vm/thread.rs @@ -128,6 +128,15 @@ thread_local! { static CURRENT_TOP_IFRAME_SLOT: Cell<*const AtomicUsize> = const { Cell::new(core::ptr::null()) }; + /// Cached pointer to this thread's `ThreadSlot::stop_requested`, for the + /// safepoint the dispatch loop takes once per instruction. Reading it + /// through `CURRENT_THREAD_SLOT` costs a `RefCell` borrow — two stores to + /// thread-local memory — where this costs one relaxed load. The slot's Arc + /// keeps the pointee alive, as with the frame pointers above. + #[cfg(feature = "threading")] + static CURRENT_STOP_REQUESTED: Cell<*const core::sync::atomic::AtomicBool> = + const { Cell::new(core::ptr::null()) }; + } #[must_use] @@ -417,6 +426,7 @@ fn set_current_thread_slot(slot: CurrentFrameSlot) { #[cfg(unix)] CURRENT_TOP_FRAME_SLOT.with(|c| c.set(&slot.top_frame)); CURRENT_TOP_IFRAME_SLOT.with(|c| c.set(&slot.top_iframe)); + CURRENT_STOP_REQUESTED.with(|c| c.set(&slot.stop_requested)); CURRENT_THREAD_SLOT.with(|current| { *current.borrow_mut() = Some(slot); }); @@ -703,10 +713,12 @@ fn do_suspend(stw: &super::StopTheWorldState) { #[inline] #[must_use] pub fn stop_requested_for_current_thread() -> bool { - CURRENT_THREAD_SLOT.with(|slot| { - slot.borrow() - .as_ref() - .is_some_and(|s| s.stop_requested.load(Ordering::Relaxed)) + CURRENT_STOP_REQUESTED.with(|cached| { + let flag = cached.get(); + // SAFETY: the pointer is non-null only while `CURRENT_THREAD_SLOT` + // holds the `Arc` that owns the flag; both are cleared + // together in `cleanup_current_thread_frames`. + !flag.is_null() && unsafe { &*flag }.load(Ordering::Relaxed) }) } @@ -920,6 +932,8 @@ pub fn cleanup_current_thread_frames(vm: &VirtualMachine) { CURRENT_TOP_FRAME_SLOT.with(|c| c.set(core::ptr::null())); #[cfg(feature = "threading")] CURRENT_TOP_IFRAME_SLOT.with(|c| c.set(core::ptr::null())); + #[cfg(feature = "threading")] + CURRENT_STOP_REQUESTED.with(|c| c.set(core::ptr::null())); } }); } @@ -983,6 +997,8 @@ pub fn reinit_frame_slot_after_fork(vm: &VirtualMachine) { CURRENT_TOP_FRAME_SLOT.with(|c| c.set(&new_slot.top_frame)); #[cfg(feature = "threading")] CURRENT_TOP_IFRAME_SLOT.with(|c| c.set(&new_slot.top_iframe)); + #[cfg(feature = "threading")] + CURRENT_STOP_REQUESTED.with(|c| c.set(&new_slot.stop_requested)); // Lock is safe: reinit_locks_after_fork() already reset it to unlocked. let mut registry = vm.state.thread_frames.lock(); From b0f0dcc5b7a20b38a96f3e84009a6bff7d996f58 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 16 Aug 2026 03:54:36 +0900 Subject: [PATCH 19/20] vm: give KwArgs a zero-sized hasher MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `KwArgs::default()` is built for every call, keyword-less ones included, and it seeded a `RandomState` each time — a thread-local read and 16 bytes in every `FuncArgs`. Keyword names come from the program text, so the map now uses `BuildHasherDefault`, whose `Default` is a zero-init. Assisted-by: Claude --- crates/vm/src/function/argument.rs | 19 ++++++++++++++----- crates/vm/src/function/mod.rs | 4 ++-- crates/vm/src/stdlib/_ast/python.rs | 4 +--- crates/vm/src/stdlib/_ctypes/structure.rs | 3 +-- crates/vm/src/stdlib/_ctypes/union.rs | 3 +-- crates/vm/src/stdlib/_functools.rs | 3 +-- 6 files changed, 20 insertions(+), 16 deletions(-) diff --git a/crates/vm/src/function/argument.rs b/crates/vm/src/function/argument.rs index aabe484c282..6bf4ae2107b 100644 --- a/crates/vm/src/function/argument.rs +++ b/crates/vm/src/function/argument.rs @@ -8,6 +8,7 @@ use crate::{ use core::ops::{Deref, DerefMut, RangeInclusive}; use indexmap::IndexMap; use itertools::Itertools; +use std::hash::DefaultHasher; pub trait IntoFuncArgs: Sized { fn into_args(self, vm: &VirtualMachine) -> FuncArgs; @@ -414,16 +415,24 @@ impl FromArgOptional for T { // issue #8228). `PyStr` is WTF-8 backed, and CPython only requires that a // keyword key be a `str`, not that it be valid UTF-8. #[derive(Clone, Debug)] -pub struct KwArgs(IndexMap); +pub struct KwArgs(KwArgsMap); + +/// The map behind [`KwArgs`]. +/// +/// The hasher is zero-sized rather than the randomly seeded default: a +/// `KwArgs` is built for every call, including the far more common +/// keyword-less one, and seeding reads a thread-local. Keyword names come +/// from the program text, so per-process hash randomization buys nothing. +pub type KwArgsMap = IndexMap>; impl Default for KwArgs { fn default() -> Self { - Self(IndexMap::new()) + Self(KwArgsMap::default()) } } impl Deref for KwArgs { - type Target = IndexMap; + type Target = KwArgsMap; fn deref(&self) -> &Self::Target { &self.0 @@ -447,7 +456,7 @@ where impl KwArgs { #[must_use] - pub const fn new(map: IndexMap) -> Self { + pub const fn new(map: KwArgsMap) -> Self { Self(map) } @@ -508,7 +517,7 @@ where T: TryFromObject, { fn from_args(vm: &VirtualMachine, args: &mut FuncArgs) -> Result { - let mut kwargs = IndexMap::new(); + let mut kwargs = KwArgsMap::default(); for (name, value) in args.remaining_keywords() { kwargs.insert(name, value.try_into_value(vm)?); } diff --git a/crates/vm/src/function/mod.rs b/crates/vm/src/function/mod.rs index 7eb87fea3ed..2ec1d09e8ef 100644 --- a/crates/vm/src/function/mod.rs +++ b/crates/vm/src/function/mod.rs @@ -11,8 +11,8 @@ mod protocol; mod time; pub use argument::{ - ArgumentError, FromArgOptional, FromArgs, FuncArgs, IntoFuncArgs, KwArgs, OptionalArg, - OptionalOption, PosArgs, + ArgumentError, FromArgOptional, FromArgs, FuncArgs, IntoFuncArgs, KwArgs, KwArgsMap, + OptionalArg, OptionalOption, PosArgs, }; pub use arithmetic::{PyArithmeticValue, PyComparisonValue}; pub use buffer::{ArgAsciiBuffer, ArgBytesLike, ArgMemoryBuffer, ArgStrOrBytesLike}; diff --git a/crates/vm/src/stdlib/_ast/python.rs b/crates/vm/src/stdlib/_ast/python.rs index db92f20db17..b6f7948293d 100644 --- a/crates/vm/src/stdlib/_ast/python.rs +++ b/crates/vm/src/stdlib/_ast/python.rs @@ -10,13 +10,11 @@ pub(crate) mod _ast { AsObject, Context, Py, PyObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine, builtins::{PyDictRef, PySet, PyStr, PyTupleRef, PyType, PyTypeRef}, class::{PyClassImpl, StaticType}, - common::wtf8::Wtf8Buf, function::{ArgIterable, FuncArgs, KwArgs, PyMethodDef, PyMethodFlags}, stdlib::_ast::repr, types::{Constructor, Initializer}, warn, }; - use indexmap::IndexMap; #[pyattr] #[pyclass(module = "_ast", name = "AST")] #[derive(Debug, PyPayload)] @@ -295,7 +293,7 @@ pub(crate) mod _ast { .map_err(|_| vm.new_type_error("keywords must be strings"))?; Ok((key.as_wtf8().to_owned(), value)) }) - .collect::>>()?; + .collect::>>()?; let result = type_obj.call(FuncArgs::new(vec![], KwArgs::new(kwargs)), vm)?; Ok(result) } diff --git a/crates/vm/src/stdlib/_ctypes/structure.rs b/crates/vm/src/stdlib/_ctypes/structure.rs index 34f53f52d60..1632d745dc6 100644 --- a/crates/vm/src/stdlib/_ctypes/structure.rs +++ b/crates/vm/src/stdlib/_ctypes/structure.rs @@ -1,6 +1,5 @@ use super::base::{CDATA_BUFFER_METHODS, PyCData, PyCField, StgInfo, StgInfoFlags}; use crate::builtins::{PyList, PyStr, PyTuple, PyType, PyTypeRef, PyUtf8Str}; -use crate::common::wtf8::Wtf8Buf; use crate::convert::ToPyObject; use crate::function::{FuncArgs, OptionalArg, PySetterValue}; use crate::protocol::{BufferDescriptor, PyBuffer, PyNumberMethods}; @@ -713,7 +712,7 @@ impl PyCStructure { self_obj: &Py, type_obj: &Py, args: &[PyObjectRef], - kwargs: &indexmap::IndexMap, + kwargs: &crate::function::KwArgsMap, index: usize, vm: &VirtualMachine, ) -> PyResult { diff --git a/crates/vm/src/stdlib/_ctypes/union.rs b/crates/vm/src/stdlib/_ctypes/union.rs index 727ad0118ad..8fe2e8348a5 100644 --- a/crates/vm/src/stdlib/_ctypes/union.rs +++ b/crates/vm/src/stdlib/_ctypes/union.rs @@ -1,7 +1,6 @@ use super::base::{CDATA_BUFFER_METHODS, StgInfoFlags}; use super::{PyCData, PyCField, StgInfo}; use crate::builtins::{PyList, PyStr, PyTuple, PyType, PyTypeRef, PyUtf8Str}; -use crate::common::wtf8::Wtf8Buf; use crate::convert::ToPyObject; use crate::function::{ArgBytesLike, FuncArgs, OptionalArg, PySetterValue}; use crate::protocol::{BufferDescriptor, PyBuffer}; @@ -582,7 +581,7 @@ impl PyCUnion { self_obj: &Py, type_obj: &Py, args: &[PyObjectRef], - kwargs: &indexmap::IndexMap, + kwargs: &crate::function::KwArgsMap, index: usize, vm: &VirtualMachine, ) -> PyResult { diff --git a/crates/vm/src/stdlib/_functools.rs b/crates/vm/src/stdlib/_functools.rs index 9b49e564562..944a2e8abdb 100644 --- a/crates/vm/src/stdlib/_functools.rs +++ b/crates/vm/src/stdlib/_functools.rs @@ -15,7 +15,6 @@ mod _functools { recursion::ReprGuard, types::{Callable, Constructor, GetDescriptor, Representable}, }; - use indexmap::IndexMap; use rustpython_common::wtf8::Wtf8Buf; #[derive(FromArgs)] @@ -432,7 +431,7 @@ mod _functools { combined_args.extend(new_args_iter.cloned()); // Merge keywords from self.keywords and args.kwargs - let mut final_kwargs = IndexMap::new(); + let mut final_kwargs = crate::function::KwArgsMap::default(); // Add keywords from self.keywords for (key, value) in &*keywords { From f9bbc6d5a17aad3e74c97b52fba48ef7a638ad24 Mon Sep 17 00:00:00 2001 From: Jeong YunWon Date: Sun, 16 Aug 2026 03:58:38 +0900 Subject: [PATCH 20/20] vm: promote borrowed stack refs before a yield `yield ` tripped the "borrowed refs on stack at yield point" assertion: `LoadSmallInt` pushes a borrowed ref, and a yield saves the stack with the frame, which is exactly what that assertion forbids. Promote the stack first, so a suspended frame owns everything it holds. Assisted-by: Claude --- crates/vm/src/frame.rs | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index d2c14498f17..67c22e2faa9 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -562,6 +562,19 @@ impl LocalsPlus { unsafe { core::mem::transmute::>(raw) } } + /// Give every borrowed stack ref its own reference. + /// + /// A borrowed ref is only sound while whatever it points at is guaranteed + /// to outlive it, which stops holding where the frame itself outlives the + /// running block — at a yield, where the stack is saved with the frame. + fn promote_stack(&mut self) { + for idx in 0..self.stack_top as usize { + if let Some(stack_ref) = self.stack_index_mut(idx) { + stack_ref.promote(); + } + } + } + /// Immutable view of the active stack as `Option` slice. #[inline(always)] fn stack_as_slice(&self) -> &[Option] { @@ -5067,6 +5080,9 @@ impl ExecutingFrame<'_> { Ok(None) } Instruction::YieldValue { .. } => { + // The frame outlives this block from here on, so nothing it + // still holds may be a borrow of something else's slot. + self.localsplus.promote_stack(); debug_assert!( self.localsplus .stack_as_slice() @@ -7264,6 +7280,7 @@ impl ExecutingFrame<'_> { self.unwind_blocks(vm, UnwindReason::Returning { value }) } Instruction::InstrumentedYieldValue => { + self.localsplus.promote_stack(); debug_assert!( self.localsplus .stack_as_slice()