Skip to content

Added obj() getter method to PyUntypedBuffer #5870

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
IntrepidT wants to merge 4 commits into
base: main
Choose a base branch
from
+42 −0
Open

Added obj() getter method to PyUntypedBuffer #5870

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c47bae9
Add obj() getter method to PyUntypedBuffer
IntrepidT Mar 8, 2026
e93c9ef
Add changelog entry
IntrepidT Mar 8, 2026
717bf14
Address Copilot review feedback: null check, doc fixes, test improvement
IntrepidT Mar 8, 2026
43d470e
Apply suggestion from @Copilot
davidhewitt Mar 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions newsfragments/5870.added.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add `obj()` getter to `PyUntypedBuffer` to retrieve the Python object owning the buffer.
24 changes: 24 additions & 0 deletions src/buffer.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,18 @@ impl PyUntypedBuffer {
self.raw().buf
}

///Returns the Python object that owns the buffer data.
///
///This is the object passed to [`PyUntypedBuffer::get()`]
///Calling this before [`release()`][Self::release] allows you to clone an owned reference and
///keeps the object alive after the buffer is released.
pub fn obj<'py>(&self, py: Python<'py>) -> Bound<'py, PyAny> {
// Safety: `PyObject_GetBuffer` increments the reference count of `obj` automatically
// and `PyBuffer_Release` decrements it on drop. The `obj` is guaranteed to be valid
// and non-null for the entire lifetime of `self`.
unsafe { Bound::from_borrowed_ptr(py, self.raw().obj) }
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

obj() uses Bound::from_borrowed_ptr(py, self.raw().obj) but there’s no invariant in PyUntypedBuffer::get() that raw.obj is non-null. Since ffi::Py_buffer::obj is just a raw pointer (and is initialized as null in Py_buffer::new()), a misbehaving exporter could yield a null obj, making this safe method invoke UB. Consider either validating raw.obj is non-null when constructing the buffer (returning a PyBufferError if it is), or changing obj() to return Option<Bound<'py, PyAny>>/PyResult<...> and handling the null case explicitly.

Suggested change
pub fn obj<'py>(&self, py: Python<'py>) -> Bound<'py, PyAny> {
// Safety: `PyObject_GetBuffer` increments the reference count of `obj` automatically
// and `PyBuffer_Release` decrements it on drop. The `obj` is guaranteed to be valid
// and non-null for the entire lifetime of `self`.
unsafe { Bound::from_borrowed_ptr(py, self.raw().obj) }
pub fn obj<'py>(&self, py: Python<'py>) -> PyResult<Bound<'py, PyAny>> {
let obj_ptr = self.raw().obj;
if obj_ptr.is_null() {
// A misbehaving exporter returned a null obj pointer, which would be UB to use.
return Err(PyBufferError::new_err(
"buffer exporter returned a null obj pointer in Py_buffer",
));
}
// Safety: `PyObject_GetBuffer` increments the reference count of `obj` automatically
// and `PyBuffer_Release` decrements it on drop. We additionally checked that `obj_ptr`
// is non-null before converting it to a `Bound`.
unsafe { Ok(Bound::from_borrowed_ptr(py, obj_ptr)) }

Copilot uses AI. Check for mistakes.
}

/// Gets a pointer to the specified item.
///
/// If `indices.len() < self.dimensions()`, returns the start address of the sub-array at the specified dimension.
Expand Down Expand Up @@ -1044,4 +1056,16 @@ mod tests {
assert_eq!(typed.shape(), [5]);
});
}

#[test]
fn test_obj_getter() {
Python::attach(|py| {
let bytes = py.eval(ffi::c_str!("b'hello'"), None, None).unwrap();
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new test constructs the bytes object via py.eval(...). For consistency with the other buffer tests in this module (which use PyBytes::new) and to avoid relying on Python code execution/parsing in a unit test, consider creating the bytes object directly with PyBytes::new(py, b"hello").

Suggested change
let bytes = py.eval(ffi::c_str!("b'hello'"), None, None).unwrap();
let bytes = PyBytes::new(py, b"hello");

Copilot uses AI. Check for mistakes.
let buf = PyUntypedBuffer::get(&bytes).unwrap();
let owner = buf.obj(py);
assert!(owner.is_instance_of::<crate::types::PyBytes>());
//owner and bytes should point to the same object
assert!(owner.is(&bytes));
Copy link

Copilot AI Mar 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

test_obj_getter only checks that obj() returns the same object, but it doesn’t cover the key behavior described in the docs/PR rationale: that you can keep the owning object alive after releasing the buffer (e.g., by cloning/unbinding the result of obj(), releasing/dropping the buffer, dropping the original bytes, and asserting the cloned reference still resolves). Adding an assertion around release() would protect against regressions in refcount handling.

Suggested change
let buf = PyUntypedBuffer::get(&bytes).unwrap();
let owner = buf.obj(py);
assert!(owner.is_instance_of::<crate::types::PyBytes>());
//owner and bytes should point to the same object
assert!(owner.is(&bytes));
let mut buf = PyUntypedBuffer::get(&bytes).unwrap();
let owner = buf.obj(py);
assert!(owner.is_instance_of::<crate::types::PyBytes>());
//owner and bytes should point to the same object
assert!(owner.is(&bytes));
// Clone/unbind the owner so it can outlive the buffer and original bytes
let owner_handle = owner.unbind();
// Explicitly release the buffer and drop it, then drop the original bytes
buf.release(py);
drop(buf);
drop(bytes);
// The unbound handle should still keep the owning object alive and usable
let bound_owner = owner_handle.bind(py);
assert!(bound_owner.is_instance_of::<crate::types::PyBytes>());

Copilot uses AI. Check for mistakes.
})
}
}
Loading