diff --git a/crates/vm/src/builtins/memory.rs b/crates/vm/src/builtins/memory.rs index 6dd4d71fa3a..24edde5e4b2 100644 --- a/crates/vm/src/builtins/memory.rs +++ b/crates/vm/src/builtins/memory.rs @@ -661,17 +661,18 @@ impl Py { // copy_buffer reads the source as it stood before the copy began, which an // overlapping assignment depends on and which also keeps the two borrows // below off the same storage. - let src = if root_exporter(&src).is(&root_exporter(&dest.buffer)) { - let owned = src.to_contiguous(vm); - drop(src); - owned - } else { - src + let may_overlap = match (root_exporter(&src), root_exporter(&dest.buffer)) { + (Some(src), Some(dest)) => src.is(&dest), + _ => true, }; + // Keep the source acquisition alive until after the copy and its byte + // guards: releasing it can run Python that releases or resizes dest. + let snapshot = may_overlap.then(|| src.to_contiguous(vm)); + let copy_src = snapshot.as_ref().unwrap_or(&src); let mut bytes_mut = dest.buffer.obj_bytes_mut(); - let src_bytes = src.obj_bytes(); - dest.desc.zip_eq(&src.desc, true, |a_range, b_range| { + let src_bytes = copy_src.obj_bytes(); + dest.desc.zip_eq(©_src.desc, true, |a_range, b_range| { let a_range = a_range.start as usize..a_range.end as usize; let b_range = b_range.start as usize..b_range.end as usize; bytes_mut[a_range].copy_from_slice(&src_bytes[b_range]); @@ -1547,8 +1548,11 @@ static BUFFER_WINDOW_METHODS: BufferMethods = BufferMethods { /// /// Two buffers that resolve to the same object address the same storage, so /// borrowing one for writing while the other is borrowed for reading would -/// deadlock on it. -fn root_exporter(buffer: &PyBuffer) -> PyObjectRef { +/// deadlock on a shared lock or create overlapping references with separate locks. +/// Borrowed ctypes storage has no reliable exporter identity: a field, pointer, +/// or from_address view can alias another object without sharing its owner. +/// Return None for that storage so either operand forces a source snapshot. +fn root_exporter(buffer: &PyBuffer) -> Option { let mut obj = buffer.obj.clone(); loop { let next = if let Some(view) = obj.downcast_ref::() { @@ -1557,8 +1561,10 @@ fn root_exporter(buffer: &PyBuffer) -> PyObjectRef { wrapper.view.obj.clone() } else if let Some(window) = obj.downcast_ref::() { window.source.obj.clone() + } else if crate::stdlib::is_borrowed_ctypes_buffer(&obj) { + return None; } else { - return obj; + return Some(obj); }; obj = next; } diff --git a/crates/vm/src/stdlib/_ctypes.rs b/crates/vm/src/stdlib/_ctypes.rs index 780b2bc0bd2..5deb9b46b54 100644 --- a/crates/vm/src/stdlib/_ctypes.rs +++ b/crates/vm/src/stdlib/_ctypes.rs @@ -22,6 +22,12 @@ pub(super) use simple::{PyCSimple, PyCSimpleType}; pub(super) use structure::PyCStructure; pub(super) use union::PyCUnion; +pub(super) fn is_borrowed_buffer(obj: &crate::PyObject) -> bool { + obj.downcast_ref::() + .is_some_and(|cdata| cdata.is_borrowed()) + || obj.downcast_ref::().is_some() +} + /// Extension for PyType to get StgInfo /// PyStgInfo_FromType impl Py { @@ -769,6 +775,9 @@ pub(crate) mod _ctypes { // 5. Resize the buffer let new_size = size as usize; let mut buffer = cdata.buffer.write(); + if new_size == buffer.len() { + return Ok(()); + } let old_data = buffer.to_vec(); *buffer = Cow::Owned(rustpython_host_env::ctypes::resize_owned_bytes( &old_data, new_size, diff --git a/crates/vm/src/stdlib/_ctypes/base.rs b/crates/vm/src/stdlib/_ctypes/base.rs index 2b697ef5558..916f621a15b 100644 --- a/crates/vm/src/stdlib/_ctypes/base.rs +++ b/crates/vm/src/stdlib/_ctypes/base.rs @@ -372,7 +372,16 @@ pub(super) static CDATA_BUFFER_METHODS: BufferMethods = BufferMethods { obj_bytes_mut: |buffer| { rustpython_common::lock::PyRwLockWriteGuard::map( buffer.obj_as::().buffer.write(), - |x| x.to_mut().as_mut_slice(), + |x| match x { + // ctypes buffers are writable, including views into their base object. + // SAFETY: requires valid writable storage and exclusive access. + // This guard only locks this view, not its owner or sibling views; + // ctypes does not enforce these requirements across aliases. + Cow::Borrowed(slice) => unsafe { + rustpython_host_env::ctypes::borrowed_slice_as_mut(slice) + }, + Cow::Owned(vec) => vec.as_mut_slice(), + }, ) .into() }, @@ -409,7 +418,9 @@ pub struct PyCData { /// /// SAFETY: Borrowed variant's 'static lifetime is not actually static. /// When created via from_address or from_base_obj, only valid for the lifetime of the source memory. - /// Same behavior as CPython's b_ptr (user responsibility, kept alive via b_base). + /// A base reference keeps the owner alive but does not pin its allocation. + /// Resizing the owner may invalidate the slice; locks on distinct views + /// do not synchronize access to their shared storage. pub buffer: PyRwLock>, /// pointer to base object or None (b_base) pub base: PyRwLock>, @@ -493,36 +504,15 @@ impl PyCData { } } - /// Create from base object with offset and data copy - /// - /// Similar to from_base_with_offset, but also stores a copy of the data. - /// This is used for arrays where we need our own buffer for the buffer protocol, - /// but still maintain the base reference for KeepRef and tracking. - pub fn from_base_with_data( - base_obj: PyObjectRef, - offset: usize, - idx: usize, - length: usize, - data: Vec, - ) -> Self { - Self { - buffer: PyRwLock::new(Cow::Owned(data)), // Has its own buffer copy - base: PyRwLock::new(Some(base_obj)), // But still tracks base - base_offset: AtomicCell::new(offset), // And offset for writes - index: AtomicCell::new(idx), - objects: PyRwLock::new(None), - length: AtomicCell::new(length), - kept_refs: PyRwLock::new(std::collections::HashMap::new()), - } - } - /// Create from base object's buffer /// /// This creates a borrowed view into the base's buffer at the given address. - /// The base object is stored in b_base to keep the memory alive. + /// The base object is stored in b_base to keep the owner alive, not to pin its allocation. /// /// # Safety - /// ptr must point into base_obj's buffer and remain valid as long as base_obj is alive. + /// `ptr` must cover `size` valid bytes for the lifetime of the returned view. + /// Keeping `base_obj` alive does not prevent reallocation or incompatible + /// access through another alias; the caller must account for both. pub unsafe fn from_base_obj( ptr: *mut u8, size: usize, @@ -530,7 +520,7 @@ impl PyCData { idx: usize, ) -> Self { // = PyCData_FromBaseObj - // SAFETY: ptr points into base_obj's buffer, kept alive via base reference + // SAFETY: the caller must uphold the storage validity and aliasing requirements above. let slice = unsafe { rustpython_host_env::ctypes::borrow_memory(ptr, size) }; Self { buffer: PyRwLock::new(Cow::Borrowed(slice)), @@ -1051,13 +1041,21 @@ impl PyCData { return Ok(vm.ctx.new_str(wchar_array_field_value(data)).into()); } - // Other array types - create array with a copy of data from the base's buffer - // The array also keeps a reference to the base for keeping it alive and for writes - let array_data = buffer[offset..offset + size].to_vec(); + // Array fields share their storage with the containing object. + // SAFETY: the field lies within the currently locked base buffer. + // The base reference retains the owner, not the allocation address; + // a reallocating resize can invalidate this view, as with other ctypes views. + let cdata_obj = unsafe { + Self::from_base_obj( + buffer.as_ptr().add(offset).cast_mut(), + size, + base_obj, + index, + ) + }; + cdata_obj.base_offset.store(offset); + cdata_obj.length.store(stg.length); drop(buffer); - - let cdata_obj = - Self::from_base_with_data(base_obj, offset, index, stg.length, array_data); let array_type: PyTypeRef = proto .to_owned() .downcast() diff --git a/crates/vm/src/stdlib/mod.rs b/crates/vm/src/stdlib/mod.rs index b1442645b7f..ee66ed3227b 100644 --- a/crates/vm/src/stdlib/mod.rs +++ b/crates/vm/src/stdlib/mod.rs @@ -83,6 +83,24 @@ mod winsound; use crate::{Context, builtins::PyModuleDef}; +// Keep ctypes-specific alias detection available to memoryview on all targets. +pub(crate) fn is_borrowed_ctypes_buffer(_obj: &crate::PyObject) -> bool { + #[cfg(all( + feature = "host_env", + any( + target_os = "linux", + target_os = "macos", + target_os = "windows", + target_os = "android" + ), + not(any(target_env = "musl", target_env = "sgx")) + ))] + if _ctypes::is_borrowed_buffer(_obj) { + return true; + } + false +} + /// Returns module definitions for multi-phase init modules. /// /// These modules use multi-phase initialization pattern: diff --git a/extra_tests/snippets/stdlib_ctypes.py b/extra_tests/snippets/stdlib_ctypes.py index 4b5dc677031..c4bdf0d939a 100644 --- a/extra_tests/snippets/stdlib_ctypes.py +++ b/extra_tests/snippets/stdlib_ctypes.py @@ -469,6 +469,77 @@ def get_win_folder_via_ctypes(csidl_name: str) -> str: gc.collect() assert roundtrip(41) == 42 + +class ArrayField(ctypes.Structure): + _fields_ = [("prefix", ctypes.c_int), ("values", ctypes.c_int * 3)] + + +owner = ArrayField(99, (1, 2, 3)) +view = owner.values +assert view._b_base_ is owner +assert ctypes.addressof(view) == ctypes.addressof(owner) + ArrayField.values.offset +ctypes.resize(owner, ctypes.sizeof(owner)) +assert ctypes.addressof(view) == ctypes.addressof(owner) + ArrayField.values.offset +pointer = ctypes.cast(view, ctypes.POINTER(ctypes.c_int)) +pointer[1] = 42 +assert list(owner.values) == [1, 42, 3] +owner.values[0] = 7 +assert bytes(view) == bytes(owner.values) +memoryview(view).cast("B")[:] = bytes((ctypes.c_int * 3)(7, 42, 9)) +assert list(owner.values) == [7, 42, 9] +del owner +assert list(view) == [7, 42, 9] + + +class OverlappingFields(ctypes.Union): + _fields_ = [("a", ctypes.c_ubyte * 8), ("b", ctypes.c_ubyte * 8)] + + +overlap = OverlappingFields() +overlap.a[:] = range(8) +memoryview(overlap.a).cast("B")[::2] = memoryview(overlap.b).cast("B")[:4] +assert list(overlap.a) == [0, 1, 1, 3, 2, 5, 3, 7] + +backing = bytearray(range(8)) +view = (ctypes.c_ubyte * 7).from_buffer(backing, 1) +view._objects.clear() # Alias detection must not depend on mutable retention metadata. +memoryview(view).cast("B")[::2] = memoryview(backing)[:4] +assert list(backing) == [0, 0, 2, 1, 4, 2, 6, 3] + +raw_owner = (ctypes.c_ubyte * 8)() +raw_views = [type(raw_owner).from_address(ctypes.addressof(raw_owner))] +if _sys.version_info >= (3, 14): + raw_views.append( + ctypes.memoryview_at(ctypes.addressof(raw_owner), ctypes.sizeof(raw_owner)) + ) +for raw_view in raw_views: + for source, dest in ((raw_owner, raw_view), (raw_view, raw_owner)): + raw_owner[:] = range(8) + memoryview(dest).cast("B")[::2] = memoryview(source).cast("B")[:4] + assert list(raw_owner) == [0, 1, 1, 3, 2, 5, 3, 7] + +# A snapshotted source's release hook must run after the destination write. +raw_owner[:] = range(8) +destination_buffer = bytearray(b"abcdefgh") +destination_view = memoryview(destination_buffer) +release_events = [] + + +class ReleasingBuffer: + def __buffer__(self, flags): + return memoryview(raw_views[0]).cast("B") + + def __release_buffer__(self, view): + release_events.append(bytes(destination_buffer)) + destination_view.release() + destination_buffer.clear() + + +destination_view[:] = ReleasingBuffer() +assert release_events == [bytes(raw_owner)] +assert destination_buffer == b"" + + # byref carries an address, including its byte offset, rather than the value # stored in the referenced object. pair = (ctypes.c_int * 2)(42, 99)