Skip to content

Commit

Permalink
Adjust tests to be compatible with disable-reference-pool feature
Browse files Browse the repository at this point in the history
  • Loading branch information
adamreichold committed Apr 21, 2024
1 parent 3e752c4 commit aebcd78
Show file tree
Hide file tree
Showing 11 changed files with 36 additions and 21 deletions.
4 changes: 3 additions & 1 deletion guide/src/class.md
Original file line number Diff line number Diff line change
Expand Up @@ -249,7 +249,7 @@ fn return_myclass() -> Py<MyClass> {

let obj = return_myclass();

Python::with_gil(|py| {
Python::with_gil(move |py| {
let bound = obj.bind(py); // Py<MyClass>::bind returns &Bound<'py, MyClass>
let obj_ref = bound.borrow(); // Get PyRef<T>
assert_eq!(obj_ref.num, 1);
Expand Down Expand Up @@ -280,6 +280,8 @@ let py_counter: Py<FrozenCounter> = Python::with_gil(|py| {
});

py_counter.get().value.fetch_add(1, Ordering::Relaxed);

Python::with_gil(move |_py| drop(py_counter));
```

Frozen classes are likely to become the default thereby guiding the PyO3 ecosystem towards a more deliberate application of interior mutability. Eventually, this should enable further optimizations of PyO3's internals and avoid downstream code paying the cost of interior mutability when it is not actually required.
Expand Down
7 changes: 5 additions & 2 deletions guide/src/memory.md
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,7 @@ This example wasn't very interesting. We could have just used a GIL-bound
we are *not* holding the GIL?

```rust
# #[cfg(not(feature = "disable-reference-pool"))] {
# use pyo3::prelude::*;
# use pyo3::types::PyString;
# fn main() -> PyResult<()> {
Expand All @@ -226,12 +227,14 @@ Python::with_gil(|py|
);
# Ok(())
# }
# }
```

When `hello` is dropped *nothing* happens to the pointed-to memory on Python's
heap because nothing _can_ happen if we're not holding the GIL. Fortunately,
the memory isn't leaked. PyO3 keeps track of the memory internally and will
release it the next time we acquire the GIL.
the memory isn't leaked. If the `disable-reference-pool` feature is not enabled,
PyO3 keeps track of the memory internally and will release it the next time
we acquire the GIL.

We can avoid the delay in releasing memory if we are careful to drop the
`Py<Any>` while the GIL is held.
Expand Down
4 changes: 2 additions & 2 deletions guide/src/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ drop(second);

The replacement is [`Python::with_gil`](https://docs.rs/pyo3/0.18.3/pyo3/marker/struct.Python.html#method.with_gil) which is more cumbersome but enforces the proper nesting by design, e.g.

```rust
```rust,ignore
# #![allow(dead_code)]
# use pyo3::prelude::*;
Expand All @@ -677,7 +677,7 @@ let second = Python::with_gil(|py| Object::new(py));
drop(first);
drop(second);
// Or it ensure releasing the inner lock before the outer one.
// Or it ensures releasing the inner lock before the outer one.
Python::with_gil(|py| {
let first = Object::new(py);
let second = Python::with_gil(|py| Object::new(py));
Expand Down
11 changes: 8 additions & 3 deletions src/instance.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,11 @@ where
/// struct Foo {/* fields omitted */}
///
/// # fn main() -> PyResult<()> {
/// Python::with_gil(|py| -> PyResult<Py<Foo>> {
/// let foo: Py<Foo> = Python::with_gil(|py| -> PyResult<_> {
/// let foo: Bound<'_, Foo> = Bound::new(py, Foo {})?;
/// Ok(foo.into())
/// })?;
/// # Python::with_gil(move |_py| drop(foo));
/// # Ok(())
/// # }
/// ```
Expand Down Expand Up @@ -932,10 +933,11 @@ where
/// struct Foo {/* fields omitted */}
///
/// # fn main() -> PyResult<()> {
/// Python::with_gil(|py| -> PyResult<Py<Foo>> {
/// let foo = Python::with_gil(|py| -> PyResult<_> {
/// let foo: Py<Foo> = Py::new(py, Foo {})?;
/// Ok(foo)
/// })?;
/// # Python::with_gil(move |_py| drop(foo));
/// # Ok(())
/// # }
/// ```
Expand Down Expand Up @@ -1245,6 +1247,7 @@ where
/// });
///
/// cell.get().value.fetch_add(1, Ordering::Relaxed);
/// # Python::with_gil(move |_py| drop(cell));
/// ```
#[inline]
pub fn get(&self) -> &T
Expand Down Expand Up @@ -2054,7 +2057,9 @@ mod tests {
Py::from(native)
});

assert_eq!(Python::with_gil(|py| dict.get_refcnt(py)), 1);
Python::with_gil(move |py| {
assert_eq!(dict.get_refcnt(py), 1);
});
}

#[test]
Expand Down
4 changes: 2 additions & 2 deletions src/marker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,6 @@ impl<'unbound> Python<'unbound> {
mod tests {
use super::*;
use crate::types::{IntoPyDict, PyList};
use std::sync::Arc;

#[test]
fn test_eval() {
Expand Down Expand Up @@ -1314,11 +1313,12 @@ mod tests {
});
}

#[cfg(not(feature = "disable-reference-pool"))]
#[test]
fn test_allow_threads_pass_stuff_in() {
let list = Python::with_gil(|py| PyList::new_bound(py, vec!["foo", "bar"]).unbind());
let mut v = vec![1, 2, 3];
let a = Arc::new(String::from("foo"));
let a = std::sync::Arc::new(String::from("foo"));

Python::with_gil(|py| {
py.allow_threads(|| {
Expand Down
8 changes: 4 additions & 4 deletions src/types/capsule.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,7 +492,7 @@ mod tests {
cap.into()
});

Python::with_gil(|py| {
Python::with_gil(move |py| {
let f = unsafe { cap.bind(py).reference::<fn(u32) -> u32>() };
assert_eq!(f(123), 123);
});
Expand Down Expand Up @@ -556,7 +556,7 @@ mod tests {
cap.into()
});

Python::with_gil(|py| {
Python::with_gil(move |py| {
let ctx: &Vec<u8> = unsafe { cap.bind(py).reference() };
assert_eq!(ctx, &[1, 2, 3, 4]);
})
Expand All @@ -575,7 +575,7 @@ mod tests {
cap.into()
});

Python::with_gil(|py| {
Python::with_gil(move |py| {
let ctx_ptr: *mut c_void = cap.bind(py).context().unwrap();
let ctx = unsafe { *Box::from_raw(ctx_ptr.cast::<&Vec<u8>>()) };
assert_eq!(ctx, &vec![1_u8, 2, 3, 4]);
Expand All @@ -592,7 +592,7 @@ mod tests {
context.send(true).unwrap();
}

Python::with_gil(|py| {
Python::with_gil(move |py| {
let name = CString::new("foo").unwrap();
let cap = PyCapsule::new_bound_with_destructor(py, 0, Some(name), destructor).unwrap();
cap.set_context(Box::into_raw(Box::new(tx)).cast()).unwrap();
Expand Down
2 changes: 1 addition & 1 deletion src/types/iterator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ mod tests {
);
});

Python::with_gil(|py| {
Python::with_gil(move |py| {
assert_eq!(count, obj.get_refcnt(py));
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/types/sequence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -801,7 +801,7 @@ mod tests {
assert!(seq[1].as_ptr() == obj.as_ptr());
});

Python::with_gil(|py| {
Python::with_gil(move |py| {
assert_eq!(1, obj.get_refcnt(py));
});
}
Expand Down
2 changes: 2 additions & 0 deletions tests/test_bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,6 @@ fn test_py_as_bytes() {
let data = Python::with_gil(|py| pyobj.as_bytes(py));

assert_eq!(data, b"abc");

Python::with_gil(move |_py| drop(pyobj));
}
11 changes: 6 additions & 5 deletions tests/test_class_basics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ impl UnsendableChild {
}

fn test_unsendable<T: PyClass + 'static>() -> PyResult<()> {
let obj = Python::with_gil(|py| -> PyResult<_> {
let (keep_obj_here, obj) = Python::with_gil(|py| -> PyResult<_> {
let obj: Py<T> = PyType::new_bound::<T>(py).call1((5,))?.extract()?;

// Accessing the value inside this thread should not panic
Expand All @@ -241,14 +241,13 @@ fn test_unsendable<T: PyClass + 'static>() -> PyResult<()> {
.is_err();

assert!(!caught_panic);
Ok(obj)
})?;

let keep_obj_here = obj.clone();
Ok((obj.clone(), obj))
})?;

let caught_panic = std::thread::spawn(move || {
// This access must panic
Python::with_gil(|py| {
Python::with_gil(move |py| {
obj.borrow(py);
});
})
Expand Down Expand Up @@ -551,6 +550,8 @@ fn access_frozen_class_without_gil() {
});

assert_eq!(py_counter.get().value.load(Ordering::Relaxed), 1);

Python::with_gil(move |_py| drop(py_counter));
}

#[test]
Expand Down
2 changes: 2 additions & 0 deletions tests/test_gc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -445,6 +445,7 @@ impl DropDuringTraversal {
}
}

#[cfg(not(feature = "disable-reference-pool"))]
#[test]
fn drop_during_traversal_with_gil() {
let drop_called = Arc::new(AtomicBool::new(false));
Expand Down Expand Up @@ -476,6 +477,7 @@ fn drop_during_traversal_with_gil() {
assert!(drop_called.load(Ordering::Relaxed));
}

#[cfg(not(feature = "disable-reference-pool"))]
#[test]
fn drop_during_traversal_without_gil() {
let drop_called = Arc::new(AtomicBool::new(false));
Expand Down

0 comments on commit aebcd78

Please sign in to comment.