-
Notifications
You must be signed in to change notification settings - Fork 950
Closed
Labels
Description
EDIT (Mistakenly posted the issue template, sorry if it looks weird in email notifications)
While digging around with lifetimes and borrowed objects just now I realised that using py::from_borrowed_ptr() for PyDict::get_item is fundamentally unsound.
If a user deletes the item from the dict after getting it, the user has easy access in safe code to a dangling PyObject.
This can easily be observed by the following code:
#[test]
fn test_dict_borrowed_object() {
use crate::{ffi, AsPyPointer};
let gil = Python::acquire_gil();
let py = gil.python();
let arr = [("a", 1234567)];
let py_map = arr.into_py_dict(py);
let item = py_map.get_item("a").unwrap();
// The value has a ref count of 1, because only the dict owns it still
assert_eq!(unsafe { ffi::Py_REFCNT(item.as_ptr()) }, 1);
// Deleting it means the refcount goes to 0 and the borrowed object `item` is now dangling
py_map.del_item("a").unwrap();
// I know that Py_REFCNT will never return 0; this is more for illustration.
// On my machine Py_REFCNT returns 2468790368784 here, but I don't know if it's deterministic.
assert_eq!(unsafe { ffi::Py_REFCNT(item.as_ptr()) }, 0);
// User could still use `item` and cause a segfault...
}My feeling from this is that this means that returning borrowed objects from our Py* Rust APIs is probably almost always unsound for similar reasons.
We might want to convert these functions to return owned pointers.
Reactions are currently unavailable