Skip to content

Commit f31f23b

Browse files
committed
Preserve reverse iterator invalidation on atomic relocation
Assisted-by: OpenAI Codex:model identifier unavailable
1 parent 072deca commit f31f23b

2 files changed

Lines changed: 28 additions & 22 deletions

File tree

‎crates/vm/src/builtins/dict.rs‎

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1519,7 +1519,9 @@ macro_rules! dict_view {
15191519
}
15201520
let internal = self.internal.lock();
15211521
match &internal.status {
1522-
IterStatus::Active(dict) if dict.size().used == self.size.used => {
1522+
IterStatus::Active(dict)
1523+
if dict.size().has_same_used_and_relocations(&self.size) =>
1524+
{
15231525
self.remaining.load(Ordering::Relaxed)
15241526
}
15251527
_ => 0,
@@ -1543,20 +1545,9 @@ macro_rules! dict_view {
15431545
// raising.
15441546
return (Err(mutated()), None);
15451547
}
1546-
let entry = loop {
1547-
let size = dict.size();
1548-
if size.used != zelf.size.used {
1549-
break Err(dict_inner::DictChanged);
1550-
}
1551-
// Storage may change while preserving the number of keys.
1552-
// Retry if it changes between the snapshot and the lookup.
1553-
if let Ok(entry) =
1554-
dict.entries
1555-
.prev_entry_checked(internal.position, &size, $project_fn)
1556-
{
1557-
break Ok(entry);
1558-
}
1559-
};
1548+
let entry =
1549+
dict.entries
1550+
.prev_entry_checked(internal.position, &zelf.size, $project_fn);
15601551
match entry {
15611552
Err(dict_inner::DictChanged) => {
15621553
zelf.changed.store(true, Ordering::Relaxed);

‎crates/vm/src/dict_inner.rs‎

Lines changed: 22 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -184,6 +184,8 @@ impl IndexEntry {
184184
struct DictInner<T> {
185185
used: usize,
186186
filled: usize,
187+
// Only private atomic relocations invalidate same-size reverse iteration.
188+
relocation_epoch: u64,
187189
indices: Vec<IndexEntry>,
188190
entries: Vec<Option<DictEntry<T>>>,
189191
}
@@ -210,14 +212,15 @@ impl<T: Clone> Clone for Dict<T> {
210212
}
211213
// Keep dense copies cheap, but do not carry mostly empty storage into
212214
// a new dictionary. pop_back can leave oversized indices without holes.
213-
let inner = if inner.used >= inner.entries.len() - inner.entries.len() / 3
215+
let mut inner = if inner.used >= inner.entries.len() - inner.entries.len() / 3
214216
&& (inner.indices.len() <= 8 || inner.used >= inner.indices.len() / 4)
215217
{
216218
inner.clone()
217219
} else {
218220
let mut copy = DictInner {
219221
used: inner.used,
220222
filled: inner.used,
223+
relocation_epoch: 0,
221224
indices: Vec::new(),
222225
entries: Vec::with_capacity(inner.used),
223226
};
@@ -226,6 +229,7 @@ impl<T: Clone> Clone for Dict<T> {
226229
copy.resize(inner.used * 2);
227230
copy
228231
};
232+
inner.relocation_epoch = 0;
229233
Self {
230234
inner: PyRwLock::new(inner),
231235
keys_version: AtomicU32::new(0),
@@ -239,6 +243,7 @@ impl<T> Default for Dict<T> {
239243
inner: PyRwLock::new(DictInner {
240244
used: 0,
241245
filled: 0,
246+
relocation_epoch: 0,
242247
indices: vec![IndexEntry::FREE; 8],
243248
entries: Vec::new(),
244249
}),
@@ -262,9 +267,16 @@ pub struct DictSize {
262267
pub entries_size: usize,
263268
pub used: usize,
264269
filled: usize,
270+
relocation_epoch: u64,
265271
}
266272

267-
/// The dict was resized under an iterator holding an older [`DictSize`].
273+
impl DictSize {
274+
pub(crate) fn has_same_used_and_relocations(&self, other: &Self) -> bool {
275+
self.used == other.used && self.relocation_epoch == other.relocation_epoch
276+
}
277+
}
278+
279+
/// The dict changed under an iterator holding an older [`DictSize`].
268280
#[derive(Debug)]
269281
pub(crate) struct DictChanged;
270282

@@ -387,6 +399,7 @@ impl<T> DictInner<T> {
387399
entries_size: self.entries.len(),
388400
used: self.used,
389401
filled: self.filled,
402+
relocation_epoch: self.relocation_epoch,
390403
}
391404
}
392405

@@ -855,10 +868,12 @@ impl<T: Clone> Dict<T> {
855868

856869
/// Install an owned table, releasing the old entries after unlocking.
857870
pub(crate) fn replace_contents(&self, other: Self) {
858-
let replacement = other.inner.into_inner();
871+
let mut replacement = other.inner.into_inner();
859872
let _removed = {
860873
let mut inner = self.write();
861874
self.invalidate_keys_version();
875+
// Relocation history belongs to this dict, not the incoming table.
876+
replacement.relocation_epoch = inner.relocation_epoch;
862877
core::mem::replace(&mut *inner, replacement)
863878
};
864879
}
@@ -1081,15 +1096,16 @@ impl<T: Clone> Dict<T> {
10811096
}
10821097
}
10831098

1084-
/// [`Self::next_entry_checked`] in reverse.
1099+
/// Step backward, validating the live size and private relocation history
1100+
/// under the same read guard. Ordinary same-size mutations may change layout.
10851101
pub(crate) fn prev_entry_checked<R>(
10861102
&self,
10871103
mut position: EntryIndex,
10881104
old: &DictSize,
10891105
project: impl FnOnce(&PyObject, &T) -> R,
10901106
) -> Result<Option<(usize, R)>, DictChanged> {
10911107
let inner = self.read();
1092-
if inner.size() != *old {
1108+
if !inner.size().has_same_used_and_relocations(old) {
10931109
return Err(DictChanged);
10941110
}
10951111
loop {
@@ -1462,6 +1478,7 @@ impl<T: Clone> Dict<T> {
14621478
// this critical section.
14631479
inner.entries.reserve(1);
14641480
self.invalidate_keys_version();
1481+
inner.relocation_epoch = inner.relocation_epoch.wrapping_add(1);
14651482
let entry = inner.entries[entry_index].take().unwrap();
14661483
let new_index = inner.entries.len();
14671484
inner.indices[entry.index] = unsafe {
@@ -1473,8 +1490,6 @@ impl<T: Clone> Dict<T> {
14731490
if holes >= 2 && holes >= inner.used {
14741491
// Moves do not increase `filled`, so ordinary insertion's
14751492
// resize threshold cannot bound their accumulated holes.
1476-
// Two holes also ensure this operation changes DictSize,
1477-
// allowing existing iterators to detect the relocation.
14781493
let indices_size = inner.indices.len();
14791494
inner.resize(indices_size);
14801495
}

0 commit comments

Comments
 (0)