From dd7ee6dcc879230510e3bdf5a3f8ae22970fc7dd Mon Sep 17 00:00:00 2001 From: YujinBae Date: Mon, 27 Jul 2026 21:36:27 +0900 Subject: [PATCH] dict iterator __reduce__ to resume from current position (#8384) * Fix dict iterator __reduce__ to resume from current position Pickling a partially-consumed dict / dict-view iterator restarted from the beginning because __reduce__ materialized every entry and ignored the iterator's position. Walk from the current position (mirroring next) so only the not-yet-yielded entries are captured, matching CPython, which reduces both directions to iter(remaining). Fixing the reverse iterator also uncovered two pre-existing bugs in reverse iteration itself, both from prev_entry saturating at 0: a hole just above index 0 made the entry at 0 yield twice, and deleting the first-inserted key made reversed() loop forever. prev_entry now returns the found entry's actual index and stops cleanly at index 0, and the reverse iterator/reduce detect exhaustion from that index. Verified against CPython 3.14 across all pickle protocols, dict states (including deletions), iterator kinds, and consumption counts. No regressions in test_dict, test_dictviews, test_ordered_dict, test_collections, test_userdict, test_iter, or test_copy. Co-Authored-By: Claude Opus 4.8 (1M context) * Apply cargo fmt to dict.rs imports Collapse the builtins import block left multi-line after removing the now-unused builtins_reversed import. Co-Authored-By: Claude Opus 4.8 (1M context) * Unmark TestSyncManagerTypes.test_dict as expectedFailure The dict iterator __reduce__ fix makes SyncManager dict proxies pickle correctly, so this test now passes under spawn and forkserver. Remove the stale expectedFailure marker that was causing an UNEXPECTED SUCCESS CI failure. Co-Authored-By: Claude Opus 4.8 (1M context) --------- Co-authored-by: Claude Opus 4.8 (1M context) --- Lib/test/_test_multiprocessing.py | 1 - crates/vm/src/builtins/dict.rs | 48 +++++++++++++++++++------------ crates/vm/src/dict_inner.rs | 5 +++- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index c3a980598..9cbe6dc64 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -6791,7 +6791,6 @@ class TestSyncManagerTypes(unittest.TestCase): obj.clear() case.assertEqual(len(obj), 0) - @unittest.expectedFailure # TODO: RUSTPYTHON def test_dict(self): o = self.manager.dict() o['foo'] = 5 diff --git a/crates/vm/src/builtins/dict.rs b/crates/vm/src/builtins/dict.rs index d14536eef..af74a2591 100644 --- a/crates/vm/src/builtins/dict.rs +++ b/crates/vm/src/builtins/dict.rs @@ -7,11 +7,7 @@ use crate::object::{Traverse, TraverseFn}; use crate::{ AsObject, Context, Py, PyExact, PyObject, PyObjectRef, PyPayload, PyRef, PyRefExact, PyResult, TryFromObject, atomic_func, - builtins::{ - PyTuple, - iter::{builtins_iter, builtins_reversed}, - type_::PyAttributes, - }, + builtins::{PyTuple, iter::builtins_iter, type_::PyAttributes}, class::{PyClassDef, PyClassImpl}, common::ascii, dict_inner::{self, DictKey}, @@ -1118,10 +1114,17 @@ macro_rules! dict_view { let iter = builtins_iter(vm); let internal = self.internal.lock(); let entries = match &internal.status { - IterStatus::Active(dict) => dict - .into_iter() - .map(|(key, value)| ($result_fn)(vm, key, value)) - .collect::>(), + IterStatus::Active(dict) => { + let mut position = internal.position; + let mut entries = Vec::new(); + while let Some((next_position, key, value)) = + dict.entries.next_entry(position) + { + entries.push(($result_fn)(vm, key, value)); + position = next_position; + } + entries + } IterStatus::Exhausted => vec![], }; vm.new_tuple((iter, (vm.ctx.new_list(entries),))) @@ -1184,14 +1187,23 @@ macro_rules! dict_view { #[pymethod] fn __reduce__(&self, vm: &VirtualMachine) -> PyTupleRef { - let iter = builtins_reversed(vm); + let iter = builtins_iter(vm); let internal = self.internal.lock(); - // TODO: entries must be reversed too let entries = match &internal.status { - IterStatus::Active(dict) => dict - .into_iter() - .map(|(key, value)| ($result_fn)(vm, key, value)) - .collect::>(), + IterStatus::Active(dict) => { + let mut position = internal.position; + let mut entries = Vec::new(); + while let Some((found_index, key, value)) = + dict.entries.prev_entry(position) + { + entries.push(($result_fn)(vm, key, value)); + if found_index == 0 { + break; + } + position = found_index - 1; + } + entries + } IterStatus::Exhausted => vec![], }; vm.new_tuple((iter, (vm.ctx.new_list(entries),))) @@ -1218,11 +1230,11 @@ macro_rules! dict_view { ); } match dict.entries.prev_entry(internal.position) { - Some((position, key, value)) => { - if internal.position == position { + Some((found_index, key, value)) => { + if found_index == 0 { internal.status = IterStatus::Exhausted; } else { - internal.position = position; + internal.position = found_index - 1; } PyIterReturn::Return(($result_fn)(vm, key, value)) } diff --git a/crates/vm/src/dict_inner.rs b/crates/vm/src/dict_inner.rs index 3b5f5617f..cc26383d9 100644 --- a/crates/vm/src/dict_inner.rs +++ b/crates/vm/src/dict_inner.rs @@ -861,10 +861,13 @@ impl Dict { let inner = self.read(); loop { let entry = inner.entries.get(position)?; - position = position.saturating_sub(1); if let Some(entry) = entry { break Some((position, entry.key.clone(), entry.value.clone())); } + if position == 0 { + break None; + } + position -= 1; } }