Detect list mutation during sort even when list length is unchanged (#7432)

* Detect list mutation during sort even when list length is unchanged

* Use mutation counter instead of capacity check for sort mutation detection

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.

* Hold write guard during sort mutation counter reads

* Fix list mutation counter race in `borrow_vec_mut`

Lib/test/test_sort.py

@@ -153,7 +153,6 @@ class TestBase(unittest.TestCase):
 
 class TestBugs(unittest.TestCase):
 
-    @unittest.skip("TODO: RUSTPYTHON; figure out how to detect sort mutation that doesn't change list length")
     def test_bug453523(self):
         # bug 453523 -- list.sort() crasher.
         # If this fails, the most likely outcome is a core dump.
@@ -170,7 +169,6 @@ class TestBugs(unittest.TestCase):
         L = [C() for i in range(50)]
         self.assertRaises(ValueError, L.sort)
 
-    @unittest.expectedFailure  # TODO: RUSTPYTHON; figure out how to detect sort mutation that doesn't change list length
     def test_undetected_mutation(self):
         # Python 2.4a1 did not always detect mutation
         memorywaster = []

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,7 +138,9 @@ impl PyList {
     }
 
     pub fn borrow_vec_mut(&self) -> PyRwLockWriteGuard<'_, Vec<PyObjectRef>> {
-        self.elements.write()
+        let guard = self.elements.write();
+        self.mutation_counter.fetch_add(1, Ordering::Relaxed);
+        guard
     }
 
     fn repeat(&self, n: isize, vm: &VirtualMachine) -> PyResult<PyRef<Self>> {
@@ -391,12 +396,21 @@ 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, version_before) = {
+            let mut guard = self.elements.write();
+            let version_before = self.mutation_counter.load(Ordering::Relaxed);
+            (core::mem::take(guard.deref_mut()), version_before)
+        };
         let res = do_sort(vm, &mut elements, options.key, options.reverse);
-        core::mem::swap(self.borrow_vec_mut().deref_mut(), &mut elements);
+        let mutated = {
+            let mut guard = self.elements.write();
+            let mutated = self.mutation_counter.load(Ordering::Relaxed) != version_before;
+            core::mem::swap(guard.deref_mut(), &mut elements);
+            mutated
+        };
         res?;
 
-        if !elements.is_empty() {
+        if mutated {
             return Err(vm.new_value_error("list modified during sort"));
         }