Use mutation counter instead of capacity check for sort mutation detection

rlaisqls · rlaisqls · commit ce3dd25b1e61 · 2026-03-17T22:20:10.000+09:00
The capacity heuristic missed mutations when `clear()` reset capacity to
0 via `mem::take`. An AtomicU32 counter on PyList, incremented in
`borrow_vec_mut()`, reliably detects all mutations during sort.
diff --git a/crates/vm/src/builtins/list.rs b/crates/vm/src/builtins/list.rs
@@ -30,11 +30,13 @@ use alloc::fmt;
 use core::cell::Cell;
 use core::ops::DerefMut;
 use core::ptr::NonNull;
+use core::sync::atomic::{AtomicU32, Ordering};
 
 #[pyclass(module = false, name = "list", unhashable = true, traverse = "manual")]
 #[derive(Default)]
 pub struct PyList {
     elements: PyRwLock<Vec<PyObjectRef>>,
+    mutation_counter: AtomicU32,
 }
 
 impl fmt::Debug for PyList {
@@ -48,6 +50,7 @@ impl From<Vec<PyObjectRef>> for PyList {
     fn from(elements: Vec<PyObjectRef>) -> Self {
         Self {
             elements: PyRwLock::new(elements),
+            mutation_counter: AtomicU32::new(0),
         }
     }
 }
@@ -135,6 +138,7 @@ impl PyList {
     }
 
     pub fn borrow_vec_mut(&self) -> PyRwLockWriteGuard<'_, Vec<PyObjectRef>> {
+        self.mutation_counter.fetch_add(1, Ordering::Relaxed);
         self.elements.write()
     }
 
@@ -391,12 +395,14 @@ impl PyList {
         // replace list contents with [] for duration of sort.
         // this prevents keyfunc from messing with the list and makes it easy to
         // check if it tries to append elements to it.
-        let mut elements = core::mem::take(self.borrow_vec_mut().deref_mut());
+        let mut elements = core::mem::take(self.elements.write().deref_mut());
+        let version_before = self.mutation_counter.load(Ordering::Relaxed);
         let res = do_sort(vm, &mut elements, options.key, options.reverse);
-        core::mem::swap(self.borrow_vec_mut().deref_mut(), &mut elements);
+        let mutated = self.mutation_counter.load(Ordering::Relaxed) != version_before;
+        core::mem::swap(self.elements.write().deref_mut(), &mut elements);
         res?;
 
-        if elements.capacity() > 0 {
+        if mutated {
             return Err(vm.new_value_error("list modified during sort"));
         }
 

Original file line number	Diff line number	Diff line change
`@@ -30,11 +30,13 @@ use alloc::fmt;`
`30`	`30`	`use core::cell::Cell;`
`31`	`31`	`use core::ops::DerefMut;`
`32`	`32`	`use core::ptr::NonNull;`
	`33`	`+use core::sync::atomic::{AtomicU32, Ordering};`
`33`	`34`
`34`	`35`	`#[pyclass(module = false, name = "list", unhashable = true, traverse = "manual")]`
`35`	`36`	`#[derive(Default)]`
`36`	`37`	`pub struct PyList {`
`37`	`38`	`elements: PyRwLock<Vec<PyObjectRef>>,`
	`39`	`+ mutation_counter: AtomicU32,`
`38`	`40`	`}`
`39`	`41`
`40`	`42`	`impl fmt::Debug for PyList {`
`@@ -48,6 +50,7 @@ impl From<Vec<PyObjectRef>> for PyList {`
`48`	`50`	`fn from(elements: Vec<PyObjectRef>) -> Self {`
`49`	`51`	`Self {`
`50`	`52`	`elements: PyRwLock::new(elements),`
	`53`	`+ mutation_counter: AtomicU32::new(0),`
`51`	`54`	`}`
`52`	`55`	`}`
`53`	`56`	`}`
`@@ -135,6 +138,7 @@ impl PyList {`
`135`	`138`	`}`
`136`	`139`
`137`	`140`	`pub fn borrow_vec_mut(&self) -> PyRwLockWriteGuard<'_, Vec<PyObjectRef>> {`
	`141`	`+ self.mutation_counter.fetch_add(1, Ordering::Relaxed);`
`138`	`142`	`self.elements.write()`
`139`	`143`	`}`
`140`	`144`
`@@ -391,12 +395,14 @@ impl PyList {`
`391`	`395`	`// replace list contents with [] for duration of sort.`
`392`	`396`	`// this prevents keyfunc from messing with the list and makes it easy to`
`393`	`397`	`// check if it tries to append elements to it.`
`394`		`- let mut elements = core::mem::take(self.borrow_vec_mut().deref_mut());`
	`398`	`+ let mut elements = core::mem::take(self.elements.write().deref_mut());`
	`399`	`+ let version_before = self.mutation_counter.load(Ordering::Relaxed);`
`395`	`400`	`let res = do_sort(vm, &mut elements, options.key, options.reverse);`
`396`		`- core::mem::swap(self.borrow_vec_mut().deref_mut(), &mut elements);`
	`401`	`+ let mutated = self.mutation_counter.load(Ordering::Relaxed) != version_before;`
	`402`	`+ core::mem::swap(self.elements.write().deref_mut(), &mut elements);`
`397`	`403`	`res?;`
`398`	`404`
`399`		`- if elements.capacity() > 0 {`
	`405`	`+ if mutated {`
`400`	`406`	`return Err(vm.new_value_error("list modified during sort"));`
`401`	`407`	`}`
`402`	`408`