Do not lock an object while running code that can reach it

Several places held a lock or a borrow of an object across a call back into Python, so a callback that touched the same object waited on a lock its own caller was holding: - memoryview slice assignment read a source overlapping the destination, and __setitem__ converted the value while holding the write borrow - BytesIO.readinto() read into a buffer viewing the same BytesIO - array.__setitem__ converted the value under the array's write lock, and mmap.write() read a source viewing the same map - bytearray.join() and bytearray.__mod__ drove Python with the bytearray borrowed - array and bytearray answered "is this resizable" after taking the write lock, though an export is exactly a borrow someone else holds A TextIOWrapper cookie now has to name a position inside what was decoded in characters as well as in bytes; only the byte offset was checked, and the character count is what read() and tell() index with. Assisted-by: Claude Assisted-by: Codex:GPT-5
youknowone · Aug 16, 2026 · Aug 14, 2026 · Aug 14, 2026 · Aug 14, 2026 · Aug 14, 2026
commit d7cf971d9051757c84d18ac2d7d897feffcfa96f
diff --git a/crates/stdlib/src/array.rs b/crates/stdlib/src/array.rs
@@ -55,6 +55,11 @@ pub mod array {
                 $($n(Vec<$t>),)*
             }
 
+            /// One item, already converted to the array's element type.
+            enum ArrayItem {
+                $($n($t),)*
+            }
+
             impl ArrayContentType {
                 fn from_char(c: char) -> Result<Self, String> {
                     match c {
@@ -303,17 +308,31 @@ pub mod array {
                     }
                 }
 
-                fn setitem_by_index(
+                /// Convert an object to the element type of the array with
+                /// this typecode. This runs the object's conversion methods,
+                /// which can reach the array, so it takes the typecode by
+                /// value and holds no lock on it.
+                fn item_from_object(
+                    typecode: char,
+                    value: PyObjectRef,
+                    vm: &VirtualMachine
+                ) -> PyResult<ArrayItem> {
+                    match typecode {
+                        $($c => Ok(ArrayItem::$n(<$t>::try_into_from_object(vm, value)?)),)*
+                        _ => unreachable!("array has a typecode"),
+                    }
+                }
+
+                fn setitem_by_item(
                     &mut self,
                     i: isize,
-                    value: PyObjectRef,
+                    item: ArrayItem,
                     vm: &VirtualMachine
                 ) -> PyResult<()> {
-                    match self {
-                        $(ArrayContentType::$n(v) => {
-                            let value = <$t>::try_into_from_object(vm, value)?;
-                            v.setitem_by_index(vm, i, value)
-                        })*
+                    match (self, item) {
+                        $((ArrayContentType::$n(v), ArrayItem::$n(value)) =>
+                            v.setitem_by_index(vm, i, value),)*
+                        _ => unreachable!("item was converted for this array"),
                     }
                 }
 
@@ -1047,7 +1066,11 @@ pub mod array {
             vm: &VirtualMachine,
         ) -> PyResult<()> {
             match SequenceIndex::try_from_borrowed_object(vm, needle, "array")? {
-                SequenceIndex::Int(i) => zelf.write().setitem_by_index(i, value, vm),
+                SequenceIndex::Int(i) => {
+                    let typecode = zelf.read().typecode();
+                    let item = ArrayContentType::item_from_object(typecode, value, vm)?;
+                    zelf.write().setitem_by_item(i, item, vm)
+                }
                 SequenceIndex::Slice(slice) => {
                     let cloned;
                     let guard;
@@ -1408,7 +1431,9 @@ pub mod array {
                 ass_item: atomic_func!(|seq, i, value, vm| {
                     let zelf = PyArray::sequence_downcast(seq);
                     if let Some(value) = value {
-                        zelf.write().setitem_by_index(i, value, vm)
+                        let typecode = zelf.read().typecode();
+                        let item = ArrayContentType::item_from_object(typecode, value, vm)?;
+                        zelf.write().setitem_by_item(i, item, vm)
                     } else {
                         zelf.write().delitem_by_index(i, vm)
                     }
@@ -1443,8 +1468,9 @@ pub mod array {
         type Resizable<'a> = PyRwLockWriteGuard<'a, ArrayContentType>;
 
         fn try_resizable_opt(&self) -> Option<Self::Resizable<'_>> {
-            let w = self.write();
-            (self.exports.load(atomic::Ordering::SeqCst) == 0).then_some(w)
+            // An export is a borrow someone else still holds, so it is
+            // answered before the lock rather than by waiting on it.
+            (self.exports.load(atomic::Ordering::SeqCst) == 0).then(|| self.write())
         }
     }
 

diff --git a/crates/stdlib/src/mmap.rs b/crates/stdlib/src/mmap.rs
@@ -1152,24 +1152,35 @@ mod mmap {
         }
 
         #[pymethod]
-        fn write(&self, bytes: ArgBytesLike, vm: &VirtualMachine) -> PyResult<PyIntRef> {
-            let pos = self.pos();
-            let size = self.__len__();
-
-            let data = bytes.borrow_buf();
+        fn write(zelf: &Py<Self>, bytes: ArgBytesLike, vm: &VirtualMachine) -> PyResult<PyIntRef> {
+            let self_ = &**zelf;
+            let pos = self_.pos();
+            let size = self_.__len__();
+
+            // Writing locks the map, and reading a source that views this same
+            // map locks it too, so such a source is copied out first.
+            let copied;
+            let borrowed;
+            let data: &[u8] = if bytes.source_object().is(zelf.as_object()) {
+                copied = bytes.borrow_buf().to_vec();
+                &copied
+            } else {
+                borrowed = bytes.borrow_buf();
+                &borrowed
+            };
 
             if pos > size || size - pos < data.len() {
                 return Err(vm.new_value_error("data out of range"));
             }
 
-            let len = self.try_writable(vm, |mmap| {
+            let len = self_.try_writable(vm, |mmap| {
                 (&mut mmap[pos..(pos + data.len())])
-                    .write(&data)
+                    .write(data)
                     .map_err(|err| err.to_pyexception(vm))?;
                 Ok(data.len())
             })??;
 
-            self.advance_pos(len);
+            self_.advance_pos(len);
 
             Ok(PyInt::from(len).into_ref(&vm.ctx))
         }

diff --git a/crates/vm/src/builtins/bytearray.rs b/crates/vm/src/builtins/bytearray.rs
@@ -354,7 +354,10 @@ impl PyByteArray {
 
     #[pymethod]
     fn join(&self, iter: ArgIterable<PyBytesInner>, vm: &VirtualMachine) -> PyResult<Self> {
-        Ok(self.inner().join(iter, vm)?.into())
+        // Driving the iterable runs Python, which can reach this bytearray,
+        // so the separator is taken by value rather than left borrowed.
+        let separator = self.inner().clone();
+        Ok(separator.join(iter, vm)?.into())
     }
 
     #[pymethod]
@@ -532,7 +535,10 @@ impl PyByteArray {
     }
 
     fn __mod__(&self, values: PyObjectRef, vm: &VirtualMachine) -> PyResult<Self> {
-        let formatted = self.inner().cformat(values, vm)?;
+        // Formatting calls the values' conversion methods, which can reach
+        // this bytearray, so the format is taken by value.
+        let format = self.inner().clone();
+        let formatted = format.cformat(values, vm)?;
         Ok(formatted.into())
     }
 
@@ -778,8 +784,9 @@ impl BufferResizeGuard for PyByteArray {
     type Resizable<'a> = PyRwLockWriteGuard<'a, PyBytesInner>;
 
     fn try_resizable_opt(&self) -> Option<Self::Resizable<'_>> {
-        let w = self.inner.write();
-        (self.exports.load(Ordering::SeqCst) == 0).then_some(w)
+        // An export is a borrow someone else still holds, so it is answered
+        // before the lock rather than by waiting on it.
+        (self.exports.load(Ordering::SeqCst) == 0).then(|| self.inner.write())
     }
 }
 

diff --git a/crates/vm/src/builtins/memory.rs b/crates/vm/src/builtins/memory.rs
@@ -192,6 +192,11 @@ impl PyMemoryView {
         }
     }
 
+    /// The object this view looks at, whose storage it borrows.
+    pub fn viewed_object(&self) -> &PyObject {
+        &self.buffer.obj
+    }
+
     fn try_not_released(&self, vm: &VirtualMachine) -> PyResult<()> {
         if self.released.load() {
             Err(vm.new_value_error("operation forbidden on released memoryview object"))

diff --git a/crates/vm/src/function/buffer.rs b/crates/vm/src/function/buffer.rs
@@ -63,6 +63,16 @@ impl ArgBytesLike {
     pub fn as_object(&self) -> &PyObject {
         &self.0.obj
     }
+
+    /// The object whose storage is borrowed while this buffer is read: a view
+    /// borrows the object it looks at, not itself.
+    #[must_use]
+    pub fn source_object(&self) -> &PyObject {
+        self.0
+            .obj
+            .downcast_ref::<crate::builtins::PyMemoryView>()
+            .map_or(&self.0.obj, |view| view.viewed_object())
+    }
 }
 
 impl From<ArgBytesLike> for PyBuffer {
@@ -139,6 +149,16 @@ impl ArgMemoryBuffer {
     pub const fn is_empty(&self) -> bool {
         self.len() == 0
     }
+
+    /// The object whose storage is borrowed while this buffer is written: a
+    /// view borrows the object it looks at, not itself.
+    #[must_use]
+    pub fn source_object(&self) -> &PyObject {
+        self.0
+            .obj
+            .downcast_ref::<crate::builtins::PyMemoryView>()
+            .map_or(&self.0.obj, |view| view.viewed_object())
+    }
 }
 
 impl From<ArgMemoryBuffer> for PyBuffer {

diff --git a/crates/vm/src/stdlib/_io.rs b/crates/vm/src/stdlib/_io.rs
@@ -3364,14 +3364,17 @@ mod _io {
                 *snapshot = Some((cookie.dec_flags, input_chunk.clone()));
                 let decoded = vm.call_method(decoder, "decode", (input_chunk, cookie.need_eof))?;
                 let decoded = check_decoded(decoded, vm)?;
-                let pos_is_valid = decoded
-                    .as_wtf8()
-                    .is_code_point_boundary(cookie.bytes_to_skip as usize);
+                // The position is stored both as a count of characters and as
+                // an offset in bytes, so both have to land inside what was
+                // just decoded: everything read back from here indexes it.
+                let num_to_skip = cookie.num_to_skip();
+                let pos_is_valid = num_to_skip.chars <= decoded.char_len()
+                    && decoded.as_wtf8().is_code_point_boundary(num_to_skip.bytes);
                 textio.set_decoded_chars(Some(decoded));
                 if !pos_is_valid {
                     return Err(vm.new_os_error("can't restore logical file position"));
                 }
-                textio.decoded_chars_used = cookie.num_to_skip();
+                textio.decoded_chars_used = num_to_skip;
             } else {
                 textio.snapshot = Some((cookie.dec_flags, PyBytes::from(vec![]).into_ref(&vm.ctx)))
             }
@@ -4813,8 +4816,20 @@ mod _io {
         }
 
         #[pymethod]
-        fn readinto(&self, obj: ArgMemoryBuffer, vm: &VirtualMachine) -> PyResult<usize> {
-            let mut buf = self.buffer(vm)?;
+        fn readinto(zelf: &Py<Self>, obj: ArgMemoryBuffer, vm: &VirtualMachine) -> PyResult<usize> {
+            // Reading locks this object, and a destination that views it locks
+            // it too, so such a destination is filled after the read is done.
+            if obj.source_object().is(zelf.as_object()) {
+                let mut data = vec![0u8; obj.len()];
+                let ret = zelf
+                    .buffer(vm)?
+                    .cursor
+                    .read(&mut data)
+                    .map_err(|_| vm.new_value_error("Error readinto from Take"))?;
+                obj.borrow_buf_mut()[..ret].copy_from_slice(&data[..ret]);
+                return Ok(ret);
+            }
+            let mut buf = zelf.buffer(vm)?;
             let ret = buf
                 .cursor
                 .read(&mut obj.borrow_buf_mut())

diff --git a/extra_tests/snippets/builtin_memoryview.py b/extra_tests/snippets/builtin_memoryview.py
@@ -562,3 +562,33 @@ def test_negative_stride():
 
 
 test_negative_stride()
+
+
+def test_write_through_same_object():
+    # Reading the source and writing the destination lock the same object
+    # when they overlap, and converting a value runs Python that can reach it.
+    b = bytearray(b"abcd")
+    memoryview(b)[0:4] = b
+    assert b == bytearray(b"abcd"), b
+
+    b = bytearray(b"abcd")
+    memoryview(b)[0:4] = memoryview(b)[::-1]
+    assert b == bytearray(b"dcba"), b
+
+    b = bytearray(b"abcd")
+    memoryview(b)[0:2] = memoryview(b)[2:4]
+    assert b == bytearray(b"cdcd"), b
+
+    b = bytearray(b"abcd")
+    view = memoryview(b)
+
+    class Index:
+        def __index__(self):
+            view[1] = 66
+            return 65
+
+    view[0] = Index()
+    assert b == bytearray(b"ABcd"), b
+
+
+test_write_through_same_object()
diff --git a/extra_tests/snippets/stdlib_array.py b/extra_tests/snippets/stdlib_array.py
@@ -143,3 +143,31 @@ def write(self, chunk):
 arr = array("b", range(128))
 arr.tofile(_ReenteringWriter(arr))
 assert len(arr) == 129
+
+
+def test_setitem_reentrant():
+    # Converting the value runs Python, which can reach the array, so the
+    # array is not locked while it happens.
+    a = array("i", [1, 2, 3])
+
+    class Index:
+        def __index__(self):
+            a[1] = 9
+            return 7
+
+    a[0] = Index()
+    assert a == array("i", [7, 9, 3]), a
+
+
+test_setitem_reentrant()
+
+
+def test_frombytes_of_itself():
+    # Resizing is refused while a buffer is exported, before any lock is taken
+    a = array("i", [1, 2, 3])
+    m = memoryview(a)
+    try:
+        a.frombytes(m)
+    except (BufferError, TypeError):
+        pass
+    del m
diff --git a/extra_tests/snippets/stdlib_io.py b/extra_tests/snippets/stdlib_io.py
@@ -204,3 +204,41 @@ def __index__(self):
 assert_raises(MemoryError, lambda: BufferedReader(BytesIO(b"a"), buffer_size=2**62))
 assert_raises(MemoryError, lambda: BufferedReader(BytesIO(b"a")).read(2**62))
 assert_raises(MemoryError, lambda: BufferedReader(BytesIO(b"a")).read1(2**62))
+
+
+def _text_cookie(
+    start_pos=0,
+    dec_flags=0,
+    bytes_to_feed=0,
+    chars_to_skip=0,
+    need_eof=0,
+    bytes_to_skip=0,
+):
+    packed = (
+        start_pos.to_bytes(8, "little", signed=True)
+        + dec_flags.to_bytes(4, "little", signed=True)
+        + bytes_to_feed.to_bytes(4, "little", signed=True)
+        + chars_to_skip.to_bytes(4, "little", signed=True)
+        + bytes([need_eof])
+        + bytes_to_skip.to_bytes(4, "little", signed=True)
+    )
+    return int.from_bytes(packed, "little")
+
+
+# A cookie names a position both in characters and in bytes, and everything
+# read back from it indexes what was decoded, so a position past the end is
+# refused rather than stored.
+for _bad in (
+    _text_cookie(bytes_to_feed=10, chars_to_skip=1000, bytes_to_skip=0),
+    _text_cookie(bytes_to_feed=10, chars_to_skip=100000, bytes_to_skip=3),
+    _text_cookie(bytes_to_feed=10, chars_to_skip=1, bytes_to_skip=1000),
+):
+    _textio = TextIOWrapper(BytesIO(b"hello world " * 20), encoding="utf-8")
+    _textio.read(1)
+    try:
+        _textio.seek(_bad)
+    except (OSError, OverflowError):
+        pass
+    else:
+        assert _textio.read(50) is not None
+        _textio.tell()
-    try:
-        _textio.seek(_bad)
-    except (OSError, OverflowError):
-        pass
-    else:
-        assert _textio.read(50) is not None
-        _textio.tell()
+    try:
+        _textio.seek(_bad)
+    except (OSError, OverflowError):
+        pass
+    else:
+        raise AssertionError("TextIOWrapper.seek accepted an invalid cookie")
-    try:
-        _textio.seek(_bad)
-    except (OSError, OverflowError):
-        pass
-    else:
-        assert _textio.read(50) is not None
-        _textio.tell()
+    try:
+        _textio.seek(_bad)
+    except (OSError, OverflowError):
+        pass
+    else:
+        raise AssertionError("TextIOWrapper.seek accepted an invalid cookie")
diff --git a/extra_tests/snippets/stdlib_io_bytesio.py b/extra_tests/snippets/stdlib_io_bytesio.py
@@ -106,3 +106,11 @@ def test_07():
     test_05()
     test_06()
     test_07()
+
+
+# Reading into a buffer that views this same object locks it twice unless the
+# read finishes first.
+_bio = BytesIO(b"x" * 60)
+assert _bio.readinto(_bio.getbuffer()) == 60
+_bio = BytesIO(b"x" * 60)
+assert _bio.readinto(memoryview(_bio.getbuffer())) == 60