mirror of
https://github.com/RustPython/RustPython.git
synced 2026-10-28 18:59:04 +08:00
Keep module cache entries present during import reordering (#8948)
* Reorder the module cache atomically during imports Add a private exact-dict move primitive and importlib dispatcher so shutdown-order updates never temporarily remove a module. Preserve current values and custom-mapping fallback behavior; validate reentrant equality probe witnesses, invalidate layout caches, and compact moved entries. Cover all four existing bootstrap reorder sites, including legacy-loader cleanup. Add deterministic trace-window, loader replacement/removal, reentrancy, cache, iterator and compaction regressions. The private native primitive uses true-move semantics; it does not promise arbitrary pop/set callback equivalence. Assisted-by: Codex:model-version-unavailable * Keep atomic import regression compatible with modern bootstrap Run legacy-loader cases when the bootstrap exposes that helper, preserving all four 3.14 paths and the two remaining 3.15 paths. Always retain modern load and exec regressions. Assisted-by: Codex:model-version-unavailable * Keep the copied bootstrap unchanged in the native relocation PR Restore the exact CPython 3.14 bootstrap and retain only the private exact-dict relocation primitive and its direct native regressions. Remove the PR-added callback integration tests while preserving native helper assertions. This prepares a primitive only; imports do not call it and the import race remains unresolved. Assisted-by: Codex:model-version-unavailable
This commit is contained in:
1 parent
8e581e6cd5
commit
d3ff1eb5ea
4 files changed
+447
-2
No files matched your search
@@ -1085,6 +1085,16 @@ impl Py<PyDict> {
|
||||
}
|
||||
|
||||
impl PyExact<PyDict> {
|
||||
pub(crate) fn move_to_end(
|
||||
&self,
|
||||
key: PyObjectRef,
|
||||
vm: &VirtualMachine,
|
||||
) -> PyResult<PyObjectRef> {
|
||||
self.entries
|
||||
.move_to_end(vm, &*key)?
|
||||
.ok_or_else(|| vm.new_key_error(key))
|
||||
}
|
||||
|
||||
/// Look up `key` in `self`, falling back to `other`.
|
||||
/// Both dicts must be exact `dict` types (enforced by `PyExact`).
|
||||
pub(crate) fn get_chain_exact<K: DictKey + ?Sized>(
|
||||
|
||||
@@ -1392,6 +1392,134 @@ impl<T: Clone> Dict<T> {
|
||||
Ok(removed)
|
||||
}
|
||||
|
||||
/// Move an existing entry to the end and return its current value without
|
||||
/// making the key temporarily absent. Hashing and equality run unlocked.
|
||||
pub(crate) fn move_to_end<K: DictKey + ?Sized>(
|
||||
&self,
|
||||
vm: &VirtualMachine,
|
||||
key: &K,
|
||||
) -> PyResult<Option<T>> {
|
||||
struct ProbeWitness {
|
||||
index_index: IndexIndex,
|
||||
entry_index: IndexEntry,
|
||||
key: Option<(HashValue, PyObjectRef)>,
|
||||
}
|
||||
|
||||
let hash_value = key.key_hash(vm)?;
|
||||
let mut idxs = None;
|
||||
let mut prefix = Vec::<ProbeWitness>::new();
|
||||
let mut compared: Option<(EntryIndex, IndexIndex, PyObjectRef, bool)> = None;
|
||||
'lookup: loop {
|
||||
// Keep the compared key alive until after the guard is released,
|
||||
// including when equality cleared the dictionary and recycled its
|
||||
// entry and bucket for a different key.
|
||||
let comparison = compared.take();
|
||||
let mut inner = self.write();
|
||||
let mask = (inner.indices.len() - 1) as i64;
|
||||
let probes = idxs.get_or_insert_with(|| GenIndexes::new(hash_value, mask));
|
||||
if probes.mask != mask {
|
||||
drop(inner);
|
||||
prefix.clear();
|
||||
idxs = None;
|
||||
continue;
|
||||
}
|
||||
let matched = if let Some((entry_index, index_index, candidate, equal)) = &comparison {
|
||||
let valid = inner.indices.get(*index_index).and_then(|i| i.index())
|
||||
== Some(*entry_index)
|
||||
&& inner
|
||||
.get_entry_checked(*entry_index, *index_index)
|
||||
.is_some_and(|entry| entry.hash == hash_value && entry.key.is(candidate));
|
||||
// A false comparison can survive while another previously
|
||||
// probed bucket changes (including after nested compaction).
|
||||
// Revalidate the whole prefix before continuing past it.
|
||||
let prefix_valid = *equal
|
||||
|| prefix.iter().all(|witness| {
|
||||
inner.indices.get(witness.index_index) == Some(&witness.entry_index)
|
||||
&& witness.key.as_ref().is_none_or(|(hash, key)| {
|
||||
inner
|
||||
.get_entry_checked(
|
||||
witness.entry_index.index().unwrap(),
|
||||
witness.index_index,
|
||||
)
|
||||
.is_some_and(|entry| entry.hash == *hash && entry.key.is(key))
|
||||
})
|
||||
});
|
||||
if !valid || !prefix_valid {
|
||||
drop(inner);
|
||||
prefix.clear();
|
||||
idxs = None;
|
||||
continue;
|
||||
}
|
||||
equal.then_some(*entry_index)
|
||||
} else {
|
||||
None
|
||||
};
|
||||
let entry_index = match matched {
|
||||
Some(index) => index,
|
||||
None => loop {
|
||||
let index_index = probes.next();
|
||||
let index_entry = inner.indices[index_index];
|
||||
match index_entry {
|
||||
IndexEntry::FREE => return Ok(None),
|
||||
IndexEntry::DUMMY => {
|
||||
prefix.push(ProbeWitness {
|
||||
index_index,
|
||||
entry_index: index_entry,
|
||||
key: None,
|
||||
});
|
||||
continue;
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
let entry_index = index_entry.index().unwrap();
|
||||
let entry = inner.entries[entry_index].as_ref().unwrap();
|
||||
if key.key_is(&entry.key) {
|
||||
break entry_index;
|
||||
}
|
||||
prefix.push(ProbeWitness {
|
||||
index_index,
|
||||
entry_index: index_entry,
|
||||
key: Some((entry.hash, entry.key.clone())),
|
||||
});
|
||||
if entry.hash == hash_value {
|
||||
let candidate = entry.key.clone();
|
||||
drop(inner);
|
||||
drop(comparison);
|
||||
let equal = key.key_eq(vm, &candidate)?;
|
||||
compared = Some((entry_index, index_index, candidate, equal));
|
||||
continue 'lookup;
|
||||
}
|
||||
},
|
||||
};
|
||||
let value = inner.entries[entry_index].as_ref().unwrap().value.clone();
|
||||
if inner.entries[entry_index + 1..].iter().any(Option::is_some) {
|
||||
// Reserve before taking the entry. No reader can observe an
|
||||
// absent entry, and no hashing, equality or finalizer runs in
|
||||
// this critical section.
|
||||
inner.entries.reserve(1);
|
||||
self.invalidate_keys_version();
|
||||
let entry = inner.entries[entry_index].take().unwrap();
|
||||
let new_index = inner.entries.len();
|
||||
inner.indices[entry.index] = unsafe {
|
||||
// SAFETY: new_index is a valid nonnegative entry index.
|
||||
IndexEntry::from_index_unchecked(new_index)
|
||||
};
|
||||
inner.entries.push(Some(entry));
|
||||
let holes = inner.entries.len() - inner.used;
|
||||
if holes >= 2 && holes >= inner.used {
|
||||
// Moves do not increase `filled`, so ordinary insertion's
|
||||
// resize threshold cannot bound their accumulated holes.
|
||||
// Two holes also ensure this operation changes DictSize,
|
||||
// allowing existing iterators to detect the relocation.
|
||||
let indices_size = inner.indices.len();
|
||||
inner.resize(indices_size);
|
||||
}
|
||||
}
|
||||
drop(inner);
|
||||
return Ok(Some(value));
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn pop_back(&self) -> Option<(PyObjectRef, T)> {
|
||||
let inner = &mut *self.write();
|
||||
let entry = loop {
|
||||
@@ -1838,6 +1966,114 @@ mod tests {
|
||||
use super::*;
|
||||
use crate::{Interpreter, common::ascii};
|
||||
|
||||
#[test]
|
||||
fn move_to_end_preserves_values_and_invalidates_layout() {
|
||||
Interpreter::without_stdlib(Default::default()).enter(|vm| {
|
||||
let dict = Dict::default();
|
||||
let keys = [
|
||||
vm.ctx.intern_str("first"),
|
||||
vm.ctx.intern_str("middle"),
|
||||
vm.ctx.intern_str("last"),
|
||||
];
|
||||
for (value, key) in keys.iter().enumerate() {
|
||||
dict.insert(vm, *key, value).unwrap();
|
||||
}
|
||||
let hint = dict.hint_for_key(vm, keys[1]).unwrap().unwrap() as usize;
|
||||
let version = dict.assign_keys_version();
|
||||
let size = dict.size();
|
||||
assert_ne!(version, 0);
|
||||
assert_eq!(dict.move_to_end(vm, keys[1]).unwrap(), Some(1));
|
||||
assert_eq!(dict.values(), vec![0, 2, 1]);
|
||||
assert_eq!(dict.keys_version(), 0);
|
||||
assert_eq!(dict.get_index_if_keys_version(version, hint), None);
|
||||
assert_eq!(dict.get_hint(vm, keys[1], hint).unwrap(), None);
|
||||
assert!(dict.next_entry_checked(0, &size, |_, v| *v).is_err());
|
||||
assert!(
|
||||
dict.prev_entry_checked(size.entries_size - 1, &size, |_, v| *v)
|
||||
.is_err()
|
||||
);
|
||||
|
||||
let version = dict.assign_keys_version();
|
||||
let size = dict.size();
|
||||
assert_eq!(dict.move_to_end(vm, keys[1]).unwrap(), Some(1));
|
||||
assert_eq!(dict.keys_version(), version);
|
||||
assert_eq!(dict.size(), size);
|
||||
assert_eq!(dict.move_to_end(vm, "absent").unwrap(), None);
|
||||
assert_eq!(dict.keys_version(), version);
|
||||
assert_eq!(dict.size(), size);
|
||||
assert_eq!(dict.values(), vec![0, 2, 1]);
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn move_to_end_compacts_and_keeps_indices_consistent() {
|
||||
Interpreter::without_stdlib(Default::default()).enter(|vm| {
|
||||
for count in [2, 3, 32] {
|
||||
let dict = Dict::default();
|
||||
for key in 0..count {
|
||||
dict.insert(vm, &key, key).unwrap();
|
||||
}
|
||||
for step in 0..1024 {
|
||||
let key = step % count;
|
||||
let size = dict.size();
|
||||
let hint = dict.hint_for_key(vm, &key).unwrap().unwrap() as usize;
|
||||
let version = dict.assign_keys_version();
|
||||
assert_eq!(dict.move_to_end(vm, &key).unwrap(), Some(key));
|
||||
assert_eq!(dict.get_index_if_keys_version(version, hint), None);
|
||||
assert!(dict.next_entry_checked(0, &size, |_, v| *v).is_err());
|
||||
assert!(
|
||||
dict.prev_entry_checked(size.entries_size - 1, &size, |_, v| *v)
|
||||
.is_err()
|
||||
);
|
||||
let inner = dict.read();
|
||||
assert_eq!(inner.used, count);
|
||||
assert_eq!(inner.filled, count);
|
||||
assert!(inner.entries.len() < 2 * count);
|
||||
assert!(inner.entries.capacity() <= 4 * count);
|
||||
let values: Vec<_> = inner.entries.iter().flatten().map(|e| e.value).collect();
|
||||
assert_eq!(
|
||||
values,
|
||||
(1..=count).map(|i| (key + i) % count).collect::<Vec<_>>()
|
||||
);
|
||||
for (index, entry) in inner.entries.iter().enumerate() {
|
||||
if let Some(entry) = entry {
|
||||
assert_eq!(inner.indices[entry.index].index(), Some(index));
|
||||
assert_eq!(entry.hash, entry.value.key_hash(vm).unwrap());
|
||||
}
|
||||
}
|
||||
for (bucket, index) in inner.indices.iter().enumerate() {
|
||||
if let Some(index) = index.index() {
|
||||
assert_eq!(inner.entries[index].as_ref().unwrap().index, bucket);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn move_to_end_restores_only_matching_shared_shapes() {
|
||||
Interpreter::without_stdlib(Default::default()).enter(|vm| {
|
||||
let first = vm.ctx.intern_str("move_first");
|
||||
let last = vm.ctx.intern_str("move_last");
|
||||
let dict = Dict::default();
|
||||
let peer = Dict::default();
|
||||
for table in [&dict, &peer] {
|
||||
table.insert(vm, first, 1).unwrap();
|
||||
table.insert(vm, last, 2).unwrap();
|
||||
}
|
||||
let version = dict.assign_keys_version();
|
||||
assert_eq!(peer.assign_keys_version(), version);
|
||||
dict.move_to_end(vm, first).unwrap();
|
||||
assert_ne!(dict.assign_keys_version(), version);
|
||||
dict.move_to_end(vm, last).unwrap();
|
||||
// The second move compacts back to the original hole-free layout.
|
||||
assert_eq!(dict.assign_keys_version(), version);
|
||||
assert_eq!(dict.get_index_if_keys_version(version, 0), Some(1));
|
||||
assert_eq!(dict.get_index_if_keys_version(version, 1), Some(2));
|
||||
});
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn clone_compacts_deleted_entries() {
|
||||
Interpreter::without_stdlib(Default::default()).enter(|vm| {
|
||||
|
||||
@@ -193,13 +193,23 @@ fn find_frozen(name: &str, vm: &VirtualMachine) -> Result<FrozenModule, FrozenEr
|
||||
#[pymodule(with(lock))]
|
||||
mod _imp {
|
||||
use crate::{
|
||||
AsObject, PyObjectRef, PyPayload, PyRef, PyResult, VirtualMachine,
|
||||
builtins::{PyBytesRef, PyCode, PyMemoryView, PyModule, PyStrRef, PyUtf8StrRef},
|
||||
AsObject, PyObjectRef, PyPayload, PyRef, PyRefExact, PyResult, VirtualMachine,
|
||||
builtins::{PyBytesRef, PyCode, PyDict, PyMemoryView, PyModule, PyStrRef, PyUtf8StrRef},
|
||||
import, version,
|
||||
};
|
||||
|
||||
use super::FrozenError;
|
||||
|
||||
// Private exact-dict relocation primitive for future import-cache ordering.
|
||||
#[pyfunction]
|
||||
fn _dict_move_to_end(
|
||||
modules: PyRefExact<PyDict>,
|
||||
key: PyObjectRef,
|
||||
vm: &VirtualMachine,
|
||||
) -> PyResult<PyObjectRef> {
|
||||
modules.move_to_end(key, vm)
|
||||
}
|
||||
|
||||
#[pyattr]
|
||||
fn check_hash_based_pycs(vm: &VirtualMachine) -> PyStrRef {
|
||||
vm.ctx
|
||||
|
||||
@@ -0,0 +1,189 @@
|
||||
"""Private exact-dict relocation and callback-reentry regression coverage."""
|
||||
|
||||
import _imp
|
||||
import sys
|
||||
|
||||
|
||||
def raises(kind, call, expected=None):
|
||||
try:
|
||||
call()
|
||||
except kind as exc:
|
||||
if expected is not None:
|
||||
assert exc is expected
|
||||
return exc
|
||||
raise AssertionError(f"{kind.__name__} was not raised")
|
||||
|
||||
|
||||
def check_native_helper():
|
||||
move = _imp._dict_move_to_end
|
||||
first, middle, last = object(), object(), object()
|
||||
mapping = {"first": first, "middle": middle, "last": last}
|
||||
assert move(mapping, "middle") is middle
|
||||
assert list(mapping) == ["first", "last", "middle"]
|
||||
assert len(mapping) == 3
|
||||
assert move(mapping, "middle") is middle
|
||||
assert list(mapping) == ["first", "last", "middle"]
|
||||
single = {"only": None}
|
||||
assert move(single, "only") is None
|
||||
assert single == {"only": None}
|
||||
missing = object()
|
||||
exc = raises(KeyError, lambda: move(mapping, missing))
|
||||
assert exc.args[0] is missing
|
||||
assert list(mapping) == ["first", "last", "middle"]
|
||||
|
||||
class Subclass(dict):
|
||||
pass
|
||||
|
||||
raises(TypeError, lambda: move(Subclass(mapping), "middle"))
|
||||
raises(TypeError, lambda: move([], "middle"))
|
||||
stored = "".join(["equal", " key"])
|
||||
equal = "".join(["equal ", "key"])
|
||||
assert stored == equal and stored is not equal
|
||||
mapping = {stored: middle, "after": last}
|
||||
assert move(mapping, equal) is middle
|
||||
assert list(mapping)[-1] is stored
|
||||
|
||||
failure = ValueError("hash failure")
|
||||
|
||||
class Hashed(str):
|
||||
count = 0
|
||||
fail = False
|
||||
|
||||
def __hash__(self):
|
||||
self.count += 1
|
||||
if self.fail:
|
||||
raise failure
|
||||
return super().__hash__()
|
||||
|
||||
lookup = Hashed(stored)
|
||||
assert move(mapping, lookup) is middle
|
||||
assert lookup.count == 1
|
||||
lookup.fail = True
|
||||
raises(ValueError, lambda: move(mapping, lookup), failure)
|
||||
assert lookup.count == 2
|
||||
|
||||
class Collision:
|
||||
def __init__(self, label, action=None):
|
||||
self.label = label
|
||||
self.action = action
|
||||
|
||||
def __hash__(self):
|
||||
return 42
|
||||
|
||||
def __eq__(self, other):
|
||||
action, self.action = self.action, None
|
||||
if action is not None:
|
||||
action()
|
||||
return isinstance(other, Collision) and self.label == other.label
|
||||
|
||||
lookup = Collision("target")
|
||||
stored = Collision("target")
|
||||
impostor = Collision("different")
|
||||
mapping = {stored: first, "after": last}
|
||||
|
||||
def replace_key():
|
||||
mapping.clear()
|
||||
mapping[impostor] = middle
|
||||
|
||||
stored.action = replace_key
|
||||
raises(KeyError, lambda: move(mapping, lookup))
|
||||
assert list(mapping) == [impostor]
|
||||
assert mapping[impostor] is middle
|
||||
|
||||
stored = Collision("target")
|
||||
mapping = {stored: first, "after": last}
|
||||
stored.action = lambda: mapping.__setitem__(stored, middle)
|
||||
assert move(mapping, lookup) is middle
|
||||
assert list(mapping)[-1] is stored
|
||||
|
||||
stored = Collision("target")
|
||||
mapping = {stored: first, "after": last}
|
||||
stored.action = lambda: mapping.clear()
|
||||
raises(KeyError, lambda: move(mapping, lookup))
|
||||
assert mapping == {}
|
||||
|
||||
stored = Collision("target")
|
||||
mapping = {stored: first, "after": last}
|
||||
|
||||
def resize():
|
||||
mapping.clear()
|
||||
for i in range(100):
|
||||
mapping[i] = i
|
||||
mapping[stored] = middle
|
||||
mapping["after"] = last
|
||||
|
||||
stored.action = resize
|
||||
assert move(mapping, lookup) is middle
|
||||
assert list(mapping)[-1] is stored
|
||||
assert len(mapping) == 102
|
||||
|
||||
# A surviving false candidate alone does not certify the probe prefix.
|
||||
# Nested relocation compacts T into P's earlier bucket, while A keeps its
|
||||
# key identity, bucket and physical entry index. T never becomes absent.
|
||||
prefix_key = Collision("prefix")
|
||||
candidate = Collision("candidate")
|
||||
target = Collision("target")
|
||||
mapping = {prefix_key: first, candidate: last, target: middle}
|
||||
|
||||
def compact_prefix():
|
||||
del mapping[prefix_key]
|
||||
assert move(mapping, candidate) is last
|
||||
assert list(mapping) == [target, candidate]
|
||||
|
||||
candidate.action = compact_prefix
|
||||
assert move(mapping, lookup) is middle
|
||||
assert list(mapping) == [candidate, target]
|
||||
|
||||
# Inserting in a previously visited DUMMY bucket also requires a restart.
|
||||
mapping = {prefix_key: first, candidate: last}
|
||||
del mapping[prefix_key]
|
||||
candidate.action = lambda: mapping.__setitem__(lookup, middle)
|
||||
assert move(mapping, lookup) is middle
|
||||
assert list(mapping) == [candidate, lookup]
|
||||
|
||||
failure = RuntimeError("equality failure")
|
||||
|
||||
def fail():
|
||||
raise failure
|
||||
|
||||
stored = Collision("target", fail)
|
||||
mapping = {stored: first, "after": last}
|
||||
before = list(mapping)
|
||||
raises(RuntimeError, lambda: move(mapping, lookup), failure)
|
||||
assert list(mapping) == before
|
||||
|
||||
class HashMutation:
|
||||
def __hash__(self):
|
||||
mapping.clear()
|
||||
mapping["kept"] = last
|
||||
return 42
|
||||
|
||||
raises(KeyError, lambda: move(mapping, HashMutation()))
|
||||
assert mapping == {"kept": last}
|
||||
|
||||
# Releasing the removed candidate must not run its finalizer under the
|
||||
# dictionary lock. The only owners are the dict and the lookup's witness.
|
||||
finalized = []
|
||||
|
||||
class Finalized(Collision):
|
||||
def __del__(self):
|
||||
mapping["finalized"] = True
|
||||
finalized.append(True)
|
||||
|
||||
mapping = {Finalized("target", lambda: mapping.clear()): first}
|
||||
raises(KeyError, lambda: move(mapping, lookup))
|
||||
assert finalized == [True]
|
||||
assert mapping == {"finalized": True}
|
||||
|
||||
mapping = {"a": 1, "b": 2}
|
||||
for i in range(100):
|
||||
forward, backward = iter(mapping), reversed(mapping)
|
||||
key = "a" if i % 2 == 0 else "b"
|
||||
move(mapping, key)
|
||||
raises(RuntimeError, lambda: next(forward))
|
||||
raises(RuntimeError, lambda: next(backward))
|
||||
assert list(mapping)[-1] == key
|
||||
|
||||
|
||||
if sys.implementation.name == "rustpython":
|
||||
check_native_helper()
|
||||
Reference in new issue
Block a user