diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index a0de012fc86..a0f58236a32 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -136,6 +136,10 @@ impl PyDict { self.entries.items() } + pub(crate) fn items_with_hashes_vec(&self) -> Vec<(PyObjectRef, PyObjectRef, PyHash)> { + self.entries.items_with_hashes() + } + fn merge_object_with_override( &self, other: PyObjectRef, diff --git a/crates/vm/src/dict_inner.rs b/crates/vm/src/dict_inner.rs index 513b2ca7089..aaa3b9276d1 100644 --- a/crates/vm/src/dict_inner.rs +++ b/crates/vm/src/dict_inner.rs @@ -581,7 +581,8 @@ impl Dict { /// Store a key. /// /// `hash` must be computed for `key` or read from [`Self::keys_with_hashes`] - /// or [`Self::next_entry_with_hash`] on a container holding this same key. + /// or [`Self::items_with_hashes`] or [`Self::next_entry_with_hash`] on a + /// container holding this same key. /// A wrong hash lands the entry in a bucket no lookup probes, silently /// losing the key. pub(crate) fn insert( @@ -1182,6 +1183,14 @@ impl Dict { .collect() } + pub(crate) fn items_with_hashes(&self) -> Vec<(PyObjectRef, T, HashValue)> { + self.read() + .entries + .iter() + .filter_map(|v| v.as_ref().map(|v| (v.key.clone(), v.value.clone(), v.hash))) + .collect() + } + /// Fold stored hashes under the read lock; the callback must not run Python. pub(crate) fn fold_hashes(&self, init: Acc, f: Fold) -> Acc where diff --git a/crates/vm/src/frame.rs b/crates/vm/src/frame.rs index 90ea0d2db61..b56dbc9e96e 100644 --- a/crates/vm/src/frame.rs +++ b/crates/vm/src/frame.rs @@ -4623,11 +4623,11 @@ impl ExecutingFrame<'_> { let src_dict = source .downcast_ref::() .expect("exact dict must have a PyDict payload"); - // Snapshot under a single read lock so a mutation of `source` - // triggered by `dict.set_item` (e.g. via a target subclass, or - // aliasing) can't be observed mid-iteration. - for (key, value) in src_dict.items_vec() { - if dict.contains_key(&*key, vm) { + // Preserve stored hashes: copying an exact dict must not call + // a key's __hash__ before the callable receives its arguments. + // Snapshot under one lock before any equality callbacks run. + for (key, value, hash) in src_dict.items_with_hashes_vec() { + if dict.contains_known_hash(&key, hash, vm)? { let key_str = key.str(vm)?; return Err(vm.new_type_error(format!( "{} got multiple values for keyword argument '{}'", @@ -4635,7 +4635,7 @@ impl ExecutingFrame<'_> { key_str.as_wtf8() ))); } - dict.set_item(&*key, value, vm)?; + dict.set_item_known_hash(&key, hash, value, vm)?; } return Ok(None); } else { diff --git a/extra_tests/snippets/syntax_function_args.py b/extra_tests/snippets/syntax_function_args.py index 96cbaf37272..6fd39c951e7 100644 --- a/extra_tests/snippets/syntax_function_args.py +++ b/extra_tests/snippets/syntax_function_args.py @@ -213,3 +213,50 @@ def f(a, b, /, c, d, *, e, f): f(1,2,3,4,5,f=6) with assert_raises(TypeError): f(1,2,3,4,5,6) + + +def test_keyword_dict_stored_hashes(): + class Keyword(str): + def __hash__(self): + assert not getattr(self, "hashed", False), "keyword was rehashed" + self.hashed = True + return str.__hash__(self) + + def accept(*, value): + return value + + source = {Keyword("value"): 42} + assert accept(**source) == 42 + with assert_raises(TypeError) as raised: + accept(value=1, **source) + assert str(raised.exception).endswith( + "got multiple values for keyword argument 'value'" + ) + + class NonString: + def __hash__(self): + assert not getattr(self, "hashed", False), "non-string key was rehashed" + self.hashed = True + return 0 + + source = {NonString(): 42} + with assert_raises(TypeError) as raised: + accept(**source) + assert str(raised.exception) == "keywords must be strings" + + failure = RuntimeError("keyword comparison failed") + + class BadEquality(str): + __hash__ = str.__hash__ + + def __eq__(self, other): + raise failure + + left = {BadEquality("value"): 1} + right = {BadEquality("value"): 2} + with assert_raises(RuntimeError) as raised: + accept(**left, **right) + assert raised.exception is failure + + +test_keyword_dict_stored_hashes()