diff --git a/crates/vm/src/builtins/memory.rs b/crates/vm/src/builtins/memory.rs index 49bbcc354..3ceb297dd 100644 --- a/crates/vm/src/builtins/memory.rs +++ b/crates/vm/src/builtins/memory.rs @@ -1403,7 +1403,7 @@ impl Hashable for PyMemoryView { if !zelf.buffer.obj.downcastable::() { zelf.while_exported(|| zelf.buffer.obj.hash(vm))?; } - let val = zelf.contiguous_or_collect(|bytes| vm.state.hash_secret.hash_bytes(bytes)); + let val = zelf.contiguous_or_collect(|bytes| crate::vm::hash_secret().hash_bytes(bytes)); let _ = zelf.hash.set(val); Ok(*zelf.hash.get().unwrap()) } diff --git a/crates/vm/src/builtins/str.rs b/crates/vm/src/builtins/str.rs index 62578dcb0..cac1db24e 100644 --- a/crates/vm/src/builtins/str.rs +++ b/crates/vm/src/builtins/str.rs @@ -721,8 +721,8 @@ impl PyStr { } #[cold] - fn _compute_hash(&self, vm: &VirtualMachine) -> hash::PyHash { - let hash_val = vm.state.hash_secret.hash_bytes(self.as_bytes()); + fn _compute_hash(&self, _vm: &VirtualMachine) -> hash::PyHash { + let hash_val = crate::vm::hash_secret().hash_bytes(self.as_bytes()); debug_assert_ne!(hash_val, hash::SENTINEL); // spell-checker:ignore cmpxchg // like with char_len, we don't need a cmpxchg loop, since it'll always be the same value diff --git a/crates/vm/src/bytes_inner.rs b/crates/vm/src/bytes_inner.rs index 7ebb636f6..392a941a0 100644 --- a/crates/vm/src/bytes_inner.rs +++ b/crates/vm/src/bytes_inner.rs @@ -402,8 +402,8 @@ impl PyBytesInner { ) } - pub fn hash(&self, vm: &VirtualMachine) -> hash::PyHash { - vm.state.hash_secret.hash_bytes(&self.elements) + pub fn hash(&self, _vm: &VirtualMachine) -> hash::PyHash { + crate::vm::hash_secret().hash_bytes(&self.elements) } pub fn add(&self, other: &[u8]) -> Vec { diff --git a/crates/vm/src/dict_inner.rs b/crates/vm/src/dict_inner.rs index 71c62cff0..ea8578789 100644 --- a/crates/vm/src/dict_inner.rs +++ b/crates/vm/src/dict_inner.rs @@ -1510,9 +1510,9 @@ impl DictKey for str { } #[inline] - fn key_hash(&self, vm: &VirtualMachine) -> PyResult { + fn key_hash(&self, _vm: &VirtualMachine) -> PyResult { // follow a similar route as the hashing of PyStrRef - Ok(vm.state.hash_secret.hash_str(self)) + Ok(crate::vm::hash_secret().hash_str(self)) } #[inline(always)] @@ -1571,9 +1571,9 @@ impl DictKey for Wtf8 { } #[inline] - fn key_hash(&self, vm: &VirtualMachine) -> PyResult { + fn key_hash(&self, _vm: &VirtualMachine) -> PyResult { // follow a similar route as the hashing of PyStrRef - Ok(vm.state.hash_secret.hash_bytes(self.as_bytes())) + Ok(crate::vm::hash_secret().hash_bytes(self.as_bytes())) } #[inline(always)] @@ -1632,9 +1632,9 @@ impl DictKey for [u8] { } #[inline] - fn key_hash(&self, vm: &VirtualMachine) -> PyResult { + fn key_hash(&self, _vm: &VirtualMachine) -> PyResult { // follow a similar route as the hashing of PyStrRef - Ok(vm.state.hash_secret.hash_bytes(self)) + Ok(crate::vm::hash_secret().hash_bytes(self)) } #[inline(always)] diff --git a/crates/vm/src/stdlib/_sre.rs b/crates/vm/src/stdlib/_sre.rs index a48192b5c..88e380f40 100644 --- a/crates/vm/src/stdlib/_sre.rs +++ b/crates/vm/src/stdlib/_sre.rs @@ -727,7 +727,7 @@ mod _sre { fn hash(zelf: &crate::Py, vm: &VirtualMachine) -> PyResult { let hash = zelf.pattern.hash(vm)?; let (_, code, _) = unsafe { zelf.code.align_to::() }; - let hash = hash ^ vm.state.hash_secret.hash_bytes(code); + let hash = hash ^ crate::vm::hash_secret().hash_bytes(code); let hash = hash ^ (zelf.flags.bits() as PyHash); let hash = hash ^ (zelf.isbytes as i64); Ok(hash) diff --git a/crates/vm/src/utils.rs b/crates/vm/src/utils.rs index f7478a006..50022ad61 100644 --- a/crates/vm/src/utils.rs +++ b/crates/vm/src/utils.rs @@ -10,7 +10,7 @@ pub fn hash_iter<'a, I: IntoIterator>( iter: I, vm: &VirtualMachine, ) -> PyResult { - vm.state.hash_secret.hash_iter(iter, |obj| obj.hash(vm)) + crate::vm::hash_secret().hash_iter(iter, |obj| obj.hash(vm)) } impl ToPyObject for core::convert::Infallible { diff --git a/crates/vm/src/vm/interpreter.rs b/crates/vm/src/vm/interpreter.rs index 4927db059..93f60edf0 100644 --- a/crates/vm/src/vm/interpreter.rs +++ b/crates/vm/src/vm/interpreter.rs @@ -72,7 +72,6 @@ where interp_config, } = opts; use crate::codecs::CodecsRegistry; - use crate::common::hash::HashSecret; use crate::common::lock::PyMutex; use crate::warn::WarningsState; use core::sync::atomic::{AtomicBool, AtomicI64, AtomicU64}; @@ -82,66 +81,54 @@ where #[cfg(feature = "threading")] thread::install_blocking_wait_hook(); - 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); + let (config, all_module_defs, frozen, 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(), + int_max_str_digits, + ) + } else { + // First top-level interpreter wins. Later seeds are ignored. + super::init_hash_secret(settings.hash_seed); + 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); + } - let seed = match config.settings.hash_seed { - Some(seed) => seed, - None => super::process_hash_secret_seed(), - }; - let hash_secret = HashSecret::new(seed); + let int_max_str_digits = AtomicCell::new(match config.settings.int_max_str_digits { + -1 => 4300, + other => other, + } as usize); - 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); - 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, - ) - }; + (config, all_module_defs, frozen, int_max_str_digits) + }; // Per-interpreter ephemeral state (must not be shared across interpreters). let codec_registry = CodecsRegistry::new(&ctx); @@ -171,7 +158,6 @@ where frozen, stacksize: AtomicCell::new(0), thread_count: AtomicCell::new(0), - hash_secret, atexit_funcs: PyMutex::default(), audit_hooks: PyMutex::default(), codec_registry, @@ -2129,4 +2115,44 @@ for _ in range(40): let _main2 = Interpreter::without_stdlib(Default::default()); assert_eq!(runtime::main_interpreter_id(), Some(recorded)); } + + /// A second top-level interpreter must keep the first interpreter's hash + /// secret. Interned strings cache that hash for the whole process. + #[test] + fn second_interpreter_reuses_process_hash_secret() { + let settings = Settings { + hash_seed: Some(7), + ..Settings::default() + }; + let first = Interpreter::without_stdlib(settings); + first.enter(|vm| { + vm.ctx + .intern_str("zz_hash_seed_regression") + .to_object() + .hash(vm) + .unwrap(); + }); + + let settings = Settings { + hash_seed: Some(8), + ..Settings::default() + }; + let second = Interpreter::without_stdlib(settings); + second.enter(|vm| { + let interned = vm.ctx.intern_str("zz_hash_seed_regression").to_object(); + let dict = vm.ctx.new_dict(); + dict.set_item(&*interned, vm.ctx.new_int(1).into(), vm) + .unwrap(); + + let scope = vm.new_scope_with_builtins(); + scope.globals.set_item("d", dict.into(), vm).unwrap(); + run( + vm, + &scope, + "k = 'zz_hash_seed_' + 'regression'\nresult = k in d\n", + ); + let result = scope.globals.get_item("result", vm).unwrap(); + assert!(result.try_to_bool(vm).unwrap()); + }); + } } diff --git a/crates/vm/src/vm/mod.rs b/crates/vm/src/vm/mod.rs index 85f87ab5b..fb03e0ce6 100644 --- a/crates/vm/src/vm/mod.rs +++ b/crates/vm/src/vm/mod.rs @@ -815,7 +815,6 @@ pub struct PyGlobalState { pub frozen: HashMap<&'static str, FrozenModule, rapidhash::quality::RandomState>, pub stacksize: AtomicCell, pub thread_count: AtomicCell, - pub hash_secret: HashSecret, /// Registered `atexit` callbacks, newest first. Shared ownership so /// `atexit.unregister` can keep the entry it is comparing alive while the /// list is unlocked, and still recognize it afterwards by identity. @@ -914,11 +913,31 @@ impl PyGlobalState { } } -pub fn process_hash_secret_seed() -> u32 { - use std::sync::OnceLock; - static SEED: OnceLock = OnceLock::new(); - // os_random is expensive, but this is only ever called once - *SEED.get_or_init(|| u32::from_ne_bytes(rustpython_common::rand::os_random())) +/// Process-wide `_Py_HashSecret`. The first top-level interpreter sets it; +/// later calls keep that value. +static HASH_SECRET: std::sync::OnceLock = std::sync::OnceLock::new(); + +/// Set the process-wide hash secret from the first top-level interpreter. +/// +/// `hash_seed` is used only when the secret is not set yet. `None` draws a +/// random seed. A later call keeps the existing secret and ignores `hash_seed`. +pub(crate) fn init_hash_secret(hash_seed: Option) { + let _ = HASH_SECRET.get_or_init(|| { + let seed = hash_seed.unwrap_or_else(|| { + // os_random is expensive, but this runs only once per process. + u32::from_ne_bytes(rustpython_common::rand::os_random()) + }); + HashSecret::new(seed) + }); +} + +/// Process-wide `_Py_HashSecret` used for str/bytes hashing. +#[inline] +#[must_use] +pub(crate) fn hash_secret() -> &'static HashSecret { + HASH_SECRET + .get() + .expect("hash secret is set by the first top-level interpreter") } /// A `NonNull` wrapper that implements `Send + Sync`. @@ -1261,16 +1280,6 @@ impl VirtualMachine { trampoline_stack: core::cell::UnsafeCell::new(Vec::new()), }; - if vm.state.hash_secret.hash_str("") - != vm - .ctx - .interned_str("") - .expect("empty str must be interned") - .hash(&vm) - { - panic!("Interpreters in same process must share the hash seed"); - } - vm.builtins.init_dict( vm.ctx.intern_str("builtins"), Some(vm.ctx.intern_str(stdlib::builtins::DOC.unwrap()).to_owned()),