From 6e9adfb7c1e2468577de47fd3603328e8b6fca85 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EA=B9=80=EC=9D=80=EB=B9=88?= Date: Fri, 20 Mar 2026 21:40:54 +0900 Subject: [PATCH] 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 | 2 -- crates/vm/src/builtins/list.rs | 22 ++++++++++++++++++---- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_sort.py b/Lib/test/test_sort.py index a01f5fc9c..dfb050a34 100644 --- a/Lib/test/test_sort.py +++ b/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 = [] diff --git a/crates/vm/src/builtins/list.rs b/crates/vm/src/builtins/list.rs index 06b2469c1..f3ea11b59 100644 --- 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>, + mutation_counter: AtomicU32, } impl fmt::Debug for PyList { @@ -48,6 +50,7 @@ impl From> for PyList { fn from(elements: Vec) -> 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> { - 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> { @@ -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")); }