Cleanups to ensure GIL-safety of Py<T> and PyObject methods

This commit is contained in:
David Hewitt 2020-06-10 08:46:39 +01:00
parent 090204c2b2
commit a85d157111
17 changed files with 153 additions and 102 deletions

View File

@ -14,6 +14,9 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
- Call `Py_Finalize` at exit to flush buffers, etc. [#943](https://github.com/PyO3/pyo3/pull/943) - Call `Py_Finalize` at exit to flush buffers, etc. [#943](https://github.com/PyO3/pyo3/pull/943)
- Add type parameter to PyBuffer. #[951](https://github.com/PyO3/pyo3/pull/951) - Add type parameter to PyBuffer. #[951](https://github.com/PyO3/pyo3/pull/951)
- Require `Send` bound for `#[pyclass]`. [#966](https://github.com/PyO3/pyo3/pull/966) - Require `Send` bound for `#[pyclass]`. [#966](https://github.com/PyO3/pyo3/pull/966)
- Add `Python` argument to most methods on `PyObject` and `Py<T>` to ensure GIL safety. [#970](https://github.com/PyO3/pyo3/pull/970)
- Change signature of `PyTypeObject::type_object()` - now takes `Python` argument and returns `&PyType`. [#970](https://github.com/PyO3/pyo3/pull/970)
- Change return type of `PyTuple::slice()` and `PyTuple::split_from()` from `Py<PyTuple>` to `&PyTuple`. [#970](https://github.com/PyO3/pyo3/pull/970)
- Change return type of `PyTuple::as_slice` to `&[&PyAny]`. [#971](https://github.com/PyO3/pyo3/pull/971) - Change return type of `PyTuple::as_slice` to `&[&PyAny]`. [#971](https://github.com/PyO3/pyo3/pull/971)
### Removed ### Removed

View File

@ -419,7 +419,7 @@ pub unsafe trait FromPyPointer<'p>: Sized {
unsafe fn from_owned_ptr_or_panic(py: Python<'p>, ptr: *mut ffi::PyObject) -> &'p Self { unsafe fn from_owned_ptr_or_panic(py: Python<'p>, ptr: *mut ffi::PyObject) -> &'p Self {
match Self::from_owned_ptr_or_opt(py, ptr) { match Self::from_owned_ptr_or_opt(py, ptr) {
Some(s) => s, Some(s) => s,
None => err::panic_after_error(), None => err::panic_after_error(py),
} }
} }
unsafe fn from_owned_ptr(py: Python<'p>, ptr: *mut ffi::PyObject) -> &'p Self { unsafe fn from_owned_ptr(py: Python<'p>, ptr: *mut ffi::PyObject) -> &'p Self {
@ -436,7 +436,7 @@ pub unsafe trait FromPyPointer<'p>: Sized {
unsafe fn from_borrowed_ptr_or_panic(py: Python<'p>, ptr: *mut ffi::PyObject) -> &'p Self { unsafe fn from_borrowed_ptr_or_panic(py: Python<'p>, ptr: *mut ffi::PyObject) -> &'p Self {
match Self::from_borrowed_ptr_or_opt(py, ptr) { match Self::from_borrowed_ptr_or_opt(py, ptr) {
Some(s) => s, Some(s) => s,
None => err::panic_after_error(), None => err::panic_after_error(py),
} }
} }
unsafe fn from_borrowed_ptr(py: Python<'p>, ptr: *mut ffi::PyObject) -> &'p Self { unsafe fn from_borrowed_ptr(py: Python<'p>, ptr: *mut ffi::PyObject) -> &'p Self {

View File

@ -1,12 +1,13 @@
// Copyright (c) 2017-present PyO3 Project and Contributors // Copyright (c) 2017-present PyO3 Project and Contributors
use crate::gil::ensure_gil;
use crate::panic::PanicException; use crate::panic::PanicException;
use crate::type_object::PyTypeObject; use crate::type_object::PyTypeObject;
use crate::types::PyType; use crate::types::PyType;
use crate::{exceptions, ffi}; use crate::{exceptions, ffi};
use crate::{ use crate::{
AsPyPointer, FromPy, FromPyPointer, IntoPy, IntoPyPointer, Py, PyAny, PyObject, Python, AsPyPointer, FromPy, FromPyPointer, IntoPy, IntoPyPointer, Py, PyAny, PyNativeType, PyObject,
ToBorrowedObject, ToPyObject, Python, ToBorrowedObject, ToPyObject,
}; };
use libc::c_int; use libc::c_int;
use std::ffi::CString; use std::ffi::CString;
@ -88,11 +89,14 @@ impl PyErr {
T: PyTypeObject, T: PyTypeObject,
V: ToPyObject + 'static, V: ToPyObject + 'static,
{ {
let ty = T::type_object(); let gil = ensure_gil();
let py = unsafe { gil.python() };
let ty = T::type_object(py);
assert_ne!(unsafe { ffi::PyExceptionClass_Check(ty.as_ptr()) }, 0); assert_ne!(unsafe { ffi::PyExceptionClass_Check(ty.as_ptr()) }, 0);
PyErr { PyErr {
ptype: ty, ptype: ty.into(),
pvalue: PyErrValue::ToObject(Box::new(value)), pvalue: PyErrValue::ToObject(Box::new(value)),
ptraceback: None, ptraceback: None,
} }
@ -103,12 +107,12 @@ impl PyErr {
/// `exc` is the exception type; usually one of the standard exceptions /// `exc` is the exception type; usually one of the standard exceptions
/// like `exceptions::RuntimeError`. /// like `exceptions::RuntimeError`.
/// `args` is the a tuple of arguments to pass to the exception constructor. /// `args` is the a tuple of arguments to pass to the exception constructor.
pub fn from_type<A>(exc: Py<PyType>, args: A) -> PyErr pub fn from_type<A>(exc: &PyType, args: A) -> PyErr
where where
A: ToPyObject + 'static, A: ToPyObject + 'static,
{ {
PyErr { PyErr {
ptype: exc, ptype: exc.into(),
pvalue: PyErrValue::ToObject(Box::new(args)), pvalue: PyErrValue::ToObject(Box::new(args)),
ptraceback: None, ptraceback: None,
} }
@ -119,11 +123,14 @@ impl PyErr {
where where
T: PyTypeObject, T: PyTypeObject,
{ {
let ty = T::type_object(); let gil = ensure_gil();
let py = unsafe { gil.python() };
let ty = T::type_object(py);
assert_ne!(unsafe { ffi::PyExceptionClass_Check(ty.as_ptr()) }, 0); assert_ne!(unsafe { ffi::PyExceptionClass_Check(ty.as_ptr()) }, 0);
PyErr { PyErr {
ptype: ty, ptype: ty.into(),
pvalue: value, pvalue: value,
ptraceback: None, ptraceback: None,
} }
@ -140,19 +147,21 @@ impl PyErr {
if unsafe { ffi::PyExceptionInstance_Check(ptr) } != 0 { if unsafe { ffi::PyExceptionInstance_Check(ptr) } != 0 {
PyErr { PyErr {
ptype: unsafe { Py::from_borrowed_ptr(ffi::PyExceptionInstance_Class(ptr)) }, ptype: unsafe {
Py::from_borrowed_ptr(obj.py(), ffi::PyExceptionInstance_Class(ptr))
},
pvalue: PyErrValue::Value(obj.into()), pvalue: PyErrValue::Value(obj.into()),
ptraceback: None, ptraceback: None,
} }
} else if unsafe { ffi::PyExceptionClass_Check(obj.as_ptr()) } != 0 { } else if unsafe { ffi::PyExceptionClass_Check(obj.as_ptr()) } != 0 {
PyErr { PyErr {
ptype: unsafe { Py::from_borrowed_ptr(ptr) }, ptype: unsafe { Py::from_borrowed_ptr(obj.py(), ptr) },
pvalue: PyErrValue::None, pvalue: PyErrValue::None,
ptraceback: None, ptraceback: None,
} }
} else { } else {
PyErr { PyErr {
ptype: exceptions::TypeError::type_object(), ptype: exceptions::TypeError::type_object(obj.py()).into(),
pvalue: PyErrValue::ToObject(Box::new("exceptions must derive from BaseException")), pvalue: PyErrValue::ToObject(Box::new("exceptions must derive from BaseException")),
ptraceback: None, ptraceback: None,
} }
@ -179,9 +188,9 @@ impl PyErr {
let mut ptraceback: *mut ffi::PyObject = std::ptr::null_mut(); let mut ptraceback: *mut ffi::PyObject = std::ptr::null_mut();
ffi::PyErr_Fetch(&mut ptype, &mut pvalue, &mut ptraceback); ffi::PyErr_Fetch(&mut ptype, &mut pvalue, &mut ptraceback);
let err = PyErr::new_from_ffi_tuple(ptype, pvalue, ptraceback); let err = PyErr::new_from_ffi_tuple(py, ptype, pvalue, ptraceback);
if ptype == PanicException::type_object().as_ptr() { if ptype == PanicException::type_object(py).as_ptr() {
let msg: String = PyAny::from_borrowed_ptr_or_opt(py, pvalue) let msg: String = PyAny::from_borrowed_ptr_or_opt(py, pvalue)
.and_then(|obj| obj.extract().ok()) .and_then(|obj| obj.extract().ok())
.unwrap_or_else(|| String::from("Unwrapped panic from Python code")); .unwrap_or_else(|| String::from("Unwrapped panic from Python code"));
@ -233,6 +242,7 @@ impl PyErr {
} }
unsafe fn new_from_ffi_tuple( unsafe fn new_from_ffi_tuple(
py: Python,
ptype: *mut ffi::PyObject, ptype: *mut ffi::PyObject,
pvalue: *mut ffi::PyObject, pvalue: *mut ffi::PyObject,
ptraceback: *mut ffi::PyObject, ptraceback: *mut ffi::PyObject,
@ -240,24 +250,22 @@ impl PyErr {
// Note: must not panic to ensure all owned pointers get acquired correctly, // Note: must not panic to ensure all owned pointers get acquired correctly,
// and because we mustn't panic in normalize(). // and because we mustn't panic in normalize().
let pvalue = if let Some(obj) = let pvalue = if let Some(obj) = PyObject::from_owned_ptr_or_opt(py, pvalue) {
PyObject::from_owned_ptr_or_opt(Python::assume_gil_acquired(), pvalue)
{
PyErrValue::Value(obj) PyErrValue::Value(obj)
} else { } else {
PyErrValue::None PyErrValue::None
}; };
let ptype = if ptype.is_null() { let ptype = if ptype.is_null() {
<exceptions::SystemError as PyTypeObject>::type_object() <exceptions::SystemError as PyTypeObject>::type_object(py).into()
} else { } else {
Py::from_owned_ptr(ptype) Py::from_owned_ptr(py, ptype)
}; };
PyErr { PyErr {
ptype, ptype,
pvalue, pvalue,
ptraceback: PyObject::from_owned_ptr_or_opt(Python::assume_gil_acquired(), ptraceback), ptraceback: PyObject::from_owned_ptr_or_opt(py, ptraceback),
} }
} }
@ -288,12 +296,12 @@ impl PyErr {
} }
/// Returns true if the current exception is instance of `T`. /// Returns true if the current exception is instance of `T`.
pub fn is_instance<T>(&self, _py: Python) -> bool pub fn is_instance<T>(&self, py: Python) -> bool
where where
T: PyTypeObject, T: PyTypeObject,
{ {
unsafe { unsafe {
ffi::PyErr_GivenExceptionMatches(self.ptype.as_ptr(), T::type_object().as_ptr()) != 0 ffi::PyErr_GivenExceptionMatches(self.ptype.as_ptr(), T::type_object(py).as_ptr()) != 0
} }
} }
@ -328,7 +336,7 @@ impl PyErr {
let mut ptraceback = ptraceback.into_ptr(); let mut ptraceback = ptraceback.into_ptr();
unsafe { unsafe {
ffi::PyErr_NormalizeException(&mut ptype, &mut pvalue, &mut ptraceback); ffi::PyErr_NormalizeException(&mut ptype, &mut pvalue, &mut ptraceback);
PyErr::new_from_ffi_tuple(ptype, pvalue, ptraceback) PyErr::new_from_ffi_tuple(py, ptype, pvalue, ptraceback)
} }
} }
@ -565,7 +573,7 @@ impl_to_pyerr!(std::string::FromUtf16Error, exceptions::UnicodeDecodeError);
impl_to_pyerr!(std::char::DecodeUtf16Error, exceptions::UnicodeDecodeError); impl_to_pyerr!(std::char::DecodeUtf16Error, exceptions::UnicodeDecodeError);
impl_to_pyerr!(std::net::AddrParseError, exceptions::ValueError); impl_to_pyerr!(std::net::AddrParseError, exceptions::ValueError);
pub fn panic_after_error() -> ! { pub fn panic_after_error(_py: Python) -> ! {
unsafe { unsafe {
ffi::PyErr_Print(); ffi::PyErr_Print();
} }

View File

@ -89,7 +89,7 @@ macro_rules! import_exception {
macro_rules! import_exception_type_object { macro_rules! import_exception_type_object {
($module: expr, $name: ident) => { ($module: expr, $name: ident) => {
unsafe impl $crate::type_object::PyTypeObject for $name { unsafe impl $crate::type_object::PyTypeObject for $name {
fn type_object() -> $crate::Py<$crate::types::PyType> { fn type_object(py: $crate::Python) -> &$crate::types::PyType {
use $crate::type_object::LazyHeapType; use $crate::type_object::LazyHeapType;
static TYPE_OBJECT: LazyHeapType = LazyHeapType::new(); static TYPE_OBJECT: LazyHeapType = LazyHeapType::new();
@ -111,7 +111,7 @@ macro_rules! import_exception_type_object {
} }
}); });
unsafe { $crate::Py::from_borrowed_ptr(ptr.as_ptr() as *mut $crate::ffi::PyObject) } unsafe { py.from_borrowed_ptr(ptr.as_ptr() as *mut $crate::ffi::PyObject) }
} }
} }
}; };
@ -173,7 +173,7 @@ macro_rules! create_exception {
macro_rules! create_exception_type_object { macro_rules! create_exception_type_object {
($module: ident, $name: ident, $base: ty) => { ($module: ident, $name: ident, $base: ty) => {
unsafe impl $crate::type_object::PyTypeObject for $name { unsafe impl $crate::type_object::PyTypeObject for $name {
fn type_object() -> $crate::Py<$crate::types::PyType> { fn type_object(py: $crate::Python) -> &$crate::types::PyType {
use $crate::type_object::LazyHeapType; use $crate::type_object::LazyHeapType;
static TYPE_OBJECT: LazyHeapType = LazyHeapType::new(); static TYPE_OBJECT: LazyHeapType = LazyHeapType::new();
@ -186,7 +186,7 @@ macro_rules! create_exception_type_object {
) )
}); });
unsafe { $crate::Py::from_borrowed_ptr(ptr.as_ptr() as *mut $crate::ffi::PyObject) } unsafe { py.from_borrowed_ptr(ptr.as_ptr() as *mut $crate::ffi::PyObject) }
} }
} }
}; };
@ -215,8 +215,8 @@ macro_rules! impl_native_exception (
} }
} }
unsafe impl PyTypeObject for $name { unsafe impl PyTypeObject for $name {
fn type_object() -> $crate::Py<$crate::types::PyType> { fn type_object(py: $crate::Python) -> &$crate::types::PyType {
unsafe { $crate::Py::from_borrowed_ptr(ffi::$exc_name) } unsafe { py.from_borrowed_ptr(ffi::$exc_name) }
} }
} }
); );

View File

@ -370,6 +370,35 @@ extern "C" fn finalize() {
} }
} }
/// Ensure the GIL is held, useful in implementation of APIs like PyErr::new where it's
/// inconvenient to force the user to acquire the GIL.
pub(crate) fn ensure_gil() -> EnsureGIL {
if gil_is_acquired() {
EnsureGIL(None)
} else {
EnsureGIL(Some(GILGuard::acquire()))
}
}
/// Struct used internally which avoids acquiring the GIL where it's not necessary.
pub(crate) struct EnsureGIL(Option<GILGuard>);
impl EnsureGIL {
/// Get the GIL token.
///
/// # Safety
/// If `self.0` is `None`, then this calls [Python::assume_gil_acquired].
/// Thus this method could be used to get access to a GIL token while the GIL is not held.
/// Care should be taken to only use the returned Python in contexts where it is certain the
/// GIL continues to be held.
pub unsafe fn python(&self) -> Python {
match &self.0 {
Some(gil) => gil.python(),
None => Python::assume_gil_acquired(),
}
}
}
#[cfg(test)] #[cfg(test)]
mod test { mod test {
use super::{gil_is_acquired, GILPool, GIL_COUNT, OWNED_OBJECTS, POOL}; use super::{gil_is_acquired, GILPool, GIL_COUNT, OWNED_OBJECTS, POOL};
@ -569,13 +598,15 @@ mod test {
#[test] #[test]
fn test_clone_with_gil() { fn test_clone_with_gil() {
let gil = Python::acquire_gil(); let gil = Python::acquire_gil();
let obj = get_object(gil.python()); let py = gil.python();
let count = obj.get_refcnt();
let obj = get_object(py);
let count = obj.get_refcnt(py);
// Cloning with the GIL should increase reference count immediately // Cloning with the GIL should increase reference count immediately
#[allow(clippy::redundant_clone)] #[allow(clippy::redundant_clone)]
let c = obj.clone(); let c = obj.clone();
assert_eq!(count + 1, c.get_refcnt()); assert_eq!(count + 1, c.get_refcnt(py));
} }
#[test] #[test]
@ -583,39 +614,46 @@ mod test {
let gil = Python::acquire_gil(); let gil = Python::acquire_gil();
let py = gil.python(); let py = gil.python();
let obj = get_object(py); let obj = get_object(py);
let count = obj.get_refcnt(); let count = obj.get_refcnt(py);
// Cloning without GIL should not update reference count // Cloning without GIL should not update reference count
drop(gil); drop(gil);
let c = obj.clone(); let c = obj.clone();
assert_eq!(count, obj.get_refcnt()); assert_eq!(
count,
obj.get_refcnt(unsafe { Python::assume_gil_acquired() })
);
// Acquring GIL will clear this pending change // Acquring GIL will clear this pending change
let gil = Python::acquire_gil(); let gil = Python::acquire_gil();
let py = gil.python();
// Total reference count should be one higher // Total reference count should be one higher
assert_eq!(count + 1, obj.get_refcnt()); assert_eq!(count + 1, obj.get_refcnt(py));
// Clone dropped then GIL released // Clone dropped
drop(c); drop(c);
drop(gil);
// Overall count is now back to the original, and should be no pending change // Overall count is now back to the original, and should be no pending change
assert_eq!(count, obj.get_refcnt()); assert_eq!(count, obj.get_refcnt(py));
} }
#[test] #[test]
fn test_clone_in_other_thread() { fn test_clone_in_other_thread() {
let gil = Python::acquire_gil(); let gil = Python::acquire_gil();
let obj = get_object(gil.python()); let py = gil.python();
let count = obj.get_refcnt(); let obj = get_object(py);
let count = obj.get_refcnt(py);
// Move obj to a thread which does not have the GIL, and clone it // Move obj to a thread which does not have the GIL, and clone it
let t = std::thread::spawn(move || { let t = std::thread::spawn(move || {
// Cloning without GIL should not update reference count // Cloning without GIL should not update reference count
#[allow(clippy::redundant_clone)] #[allow(clippy::redundant_clone)]
let _ = obj.clone(); let _ = obj.clone();
assert_eq!(count, obj.get_refcnt()); assert_eq!(
count,
obj.get_refcnt(unsafe { Python::assume_gil_acquired() })
);
// Return obj so original thread can continue to use // Return obj so original thread can continue to use
obj obj
@ -631,13 +669,13 @@ mod test {
// Re-acquring GIL will clear these pending changes // Re-acquring GIL will clear these pending changes
drop(gil); drop(gil);
let _gil = Python::acquire_gil(); let gil = Python::acquire_gil();
assert!(POOL.pointers_to_incref.lock().is_empty()); assert!(POOL.pointers_to_incref.lock().is_empty());
assert!(POOL.pointers_to_decref.lock().is_empty()); assert!(POOL.pointers_to_decref.lock().is_empty());
// Overall count is still unchanged // Overall count is still unchanged
assert_eq!(count, obj.get_refcnt()); assert_eq!(count, obj.get_refcnt(gil.python()));
} }
#[test] #[test]

View File

@ -61,7 +61,7 @@ impl<T> Py<T> {
{ {
let initializer = value.into(); let initializer = value.into();
let obj = unsafe { initializer.create_cell(py)? }; let obj = unsafe { initializer.create_cell(py)? };
let ob = unsafe { Py::from_owned_ptr(obj as _) }; let ob = unsafe { Py::from_owned_ptr(py, obj as _) };
Ok(ob) Ok(ob)
} }
@ -70,7 +70,7 @@ impl<T> Py<T> {
/// This moves ownership over the pointer into the `Py<T>`. /// This moves ownership over the pointer into the `Py<T>`.
/// Undefined behavior if the pointer is NULL or invalid. /// Undefined behavior if the pointer is NULL or invalid.
#[inline] #[inline]
pub unsafe fn from_owned_ptr(ptr: *mut ffi::PyObject) -> Py<T> { pub unsafe fn from_owned_ptr(_py: Python, ptr: *mut ffi::PyObject) -> Py<T> {
debug_assert!( debug_assert!(
!ptr.is_null() && ffi::Py_REFCNT(ptr) > 0, !ptr.is_null() && ffi::Py_REFCNT(ptr) > 0,
format!("REFCNT: {:?} - {:?}", ptr, ffi::Py_REFCNT(ptr)) format!("REFCNT: {:?} - {:?}", ptr, ffi::Py_REFCNT(ptr))
@ -83,11 +83,11 @@ impl<T> Py<T> {
/// Panics if the pointer is NULL. /// Panics if the pointer is NULL.
/// Undefined behavior if the pointer is invalid. /// Undefined behavior if the pointer is invalid.
#[inline] #[inline]
pub unsafe fn from_owned_ptr_or_panic(ptr: *mut ffi::PyObject) -> Py<T> { pub unsafe fn from_owned_ptr_or_panic(_py: Python, ptr: *mut ffi::PyObject) -> Py<T> {
match NonNull::new(ptr) { match NonNull::new(ptr) {
Some(nonnull_ptr) => Py(nonnull_ptr, PhantomData), Some(nonnull_ptr) => Py(nonnull_ptr, PhantomData),
None => { None => {
crate::err::panic_after_error(); crate::err::panic_after_error(_py);
} }
} }
} }
@ -109,7 +109,7 @@ impl<T> Py<T> {
/// Calls `Py_INCREF()` on the ptr. /// Calls `Py_INCREF()` on the ptr.
/// Undefined behavior if the pointer is NULL or invalid. /// Undefined behavior if the pointer is NULL or invalid.
#[inline] #[inline]
pub unsafe fn from_borrowed_ptr(ptr: *mut ffi::PyObject) -> Py<T> { pub unsafe fn from_borrowed_ptr(_py: Python, ptr: *mut ffi::PyObject) -> Py<T> {
debug_assert!( debug_assert!(
!ptr.is_null() && ffi::Py_REFCNT(ptr) > 0, !ptr.is_null() && ffi::Py_REFCNT(ptr) > 0,
format!("REFCNT: {:?} - {:?}", ptr, ffi::Py_REFCNT(ptr)) format!("REFCNT: {:?} - {:?}", ptr, ffi::Py_REFCNT(ptr))
@ -120,14 +120,14 @@ impl<T> Py<T> {
/// Gets the reference count of the `ffi::PyObject` pointer. /// Gets the reference count of the `ffi::PyObject` pointer.
#[inline] #[inline]
pub fn get_refcnt(&self) -> isize { pub fn get_refcnt(&self, _py: Python) -> isize {
unsafe { ffi::Py_REFCNT(self.0.as_ptr()) } unsafe { ffi::Py_REFCNT(self.0.as_ptr()) }
} }
/// Clones self by calling `Py_INCREF()` on the ptr. /// Clones self by calling `Py_INCREF()` on the ptr.
#[inline] #[inline]
pub fn clone_ref(&self, _py: Python) -> Py<T> { pub fn clone_ref(&self, py: Python) -> Py<T> {
unsafe { Py::from_borrowed_ptr(self.0.as_ptr()) } unsafe { Py::from_borrowed_ptr(py, self.0.as_ptr()) }
} }
/// Returns the inner pointer without decreasing the refcount. /// Returns the inner pointer without decreasing the refcount.
@ -225,7 +225,7 @@ where
T: AsPyPointer + PyNativeType, T: AsPyPointer + PyNativeType,
{ {
fn from(obj: &'a T) -> Self { fn from(obj: &'a T) -> Self {
unsafe { Py::from_borrowed_ptr(obj.as_ptr()) } unsafe { Py::from_borrowed_ptr(obj.py(), obj.as_ptr()) }
} }
} }
@ -235,7 +235,7 @@ where
T: PyClass, T: PyClass,
{ {
fn from(cell: &PyCell<T>) -> Self { fn from(cell: &PyCell<T>) -> Self {
unsafe { Py::from_borrowed_ptr(cell.as_ptr()) } unsafe { Py::from_borrowed_ptr(cell.py(), cell.as_ptr()) }
} }
} }
@ -244,7 +244,7 @@ where
T: PyClass, T: PyClass,
{ {
fn from(pyref: PyRef<'a, T>) -> Self { fn from(pyref: PyRef<'a, T>) -> Self {
unsafe { Py::from_borrowed_ptr(pyref.as_ptr()) } unsafe { Py::from_borrowed_ptr(pyref.py(), pyref.as_ptr()) }
} }
} }
@ -253,7 +253,7 @@ where
T: PyClass, T: PyClass,
{ {
fn from(pyref: PyRefMut<'a, T>) -> Self { fn from(pyref: PyRefMut<'a, T>) -> Self {
unsafe { Py::from_borrowed_ptr(pyref.as_ptr()) } unsafe { Py::from_borrowed_ptr(pyref.py(), pyref.as_ptr()) }
} }
} }
@ -291,19 +291,19 @@ impl<T> std::convert::From<Py<T>> for PyObject {
impl<'a, T> std::convert::From<&'a T> for PyObject impl<'a, T> std::convert::From<&'a T> for PyObject
where where
T: AsPyPointer, T: AsPyPointer + PyNativeType,
{ {
fn from(ob: &'a T) -> Self { fn from(ob: &'a T) -> Self {
unsafe { Py::<T>::from_borrowed_ptr(ob.as_ptr()) }.into() unsafe { Py::<T>::from_borrowed_ptr(ob.py(), ob.as_ptr()) }.into()
} }
} }
impl<'a, T> std::convert::From<&'a mut T> for PyObject impl<'a, T> std::convert::From<&'a mut T> for PyObject
where where
T: AsPyPointer, T: AsPyPointer + PyNativeType,
{ {
fn from(ob: &'a mut T) -> Self { fn from(ob: &'a mut T) -> Self {
unsafe { Py::<T>::from_borrowed_ptr(ob.as_ptr()) }.into() unsafe { Py::<T>::from_borrowed_ptr(ob.py(), ob.as_ptr()) }.into()
} }
} }
@ -317,7 +317,7 @@ where
fn extract(ob: &'a PyAny) -> PyResult<Self> { fn extract(ob: &'a PyAny) -> PyResult<Self> {
unsafe { unsafe {
ob.extract::<&T::AsRefTarget>() ob.extract::<&T::AsRefTarget>()
.map(|val| Py::from_borrowed_ptr(val.as_ptr())) .map(|val| Py::from_borrowed_ptr(ob.py(), val.as_ptr()))
} }
} }
} }

View File

@ -48,11 +48,11 @@ impl PyObject {
/// Panics if the pointer is NULL. /// Panics if the pointer is NULL.
/// Undefined behavior if the pointer is invalid. /// Undefined behavior if the pointer is invalid.
#[inline] #[inline]
pub unsafe fn from_owned_ptr_or_panic(_py: Python, ptr: *mut ffi::PyObject) -> PyObject { pub unsafe fn from_owned_ptr_or_panic(py: Python, ptr: *mut ffi::PyObject) -> PyObject {
match NonNull::new(ptr) { match NonNull::new(ptr) {
Some(nonnull_ptr) => PyObject(nonnull_ptr), Some(nonnull_ptr) => PyObject(nonnull_ptr),
None => { None => {
crate::err::panic_after_error(); crate::err::panic_after_error(py);
} }
} }
} }
@ -119,7 +119,7 @@ impl PyObject {
} }
/// Gets the reference count of the ffi::PyObject pointer. /// Gets the reference count of the ffi::PyObject pointer.
pub fn get_refcnt(&self) -> isize { pub fn get_refcnt(&self, _py: Python) -> isize {
unsafe { ffi::Py_REFCNT(self.0.as_ptr()) } unsafe { ffi::Py_REFCNT(self.0.as_ptr()) }
} }
@ -131,7 +131,7 @@ impl PyObject {
/// Returns whether the object is considered to be None. /// Returns whether the object is considered to be None.
/// ///
/// This is equivalent to the Python expression `self is None`. /// This is equivalent to the Python expression `self is None`.
pub fn is_none(&self) -> bool { pub fn is_none(&self, _py: Python) -> bool {
unsafe { ffi::Py_None() == self.as_ptr() } unsafe { ffi::Py_None() == self.as_ptr() }
} }

View File

@ -6,9 +6,7 @@ use crate::err::{PyDowncastError, PyErr, PyResult};
use crate::gil::{self, GILGuard, GILPool}; use crate::gil::{self, GILGuard, GILPool};
use crate::type_object::{PyTypeInfo, PyTypeObject}; use crate::type_object::{PyTypeInfo, PyTypeObject};
use crate::types::{PyAny, PyDict, PyModule, PyType}; use crate::types::{PyAny, PyDict, PyModule, PyType};
use crate::{ use crate::{ffi, AsPyPointer, FromPyPointer, IntoPyPointer, PyNativeType, PyObject, PyTryFrom};
ffi, AsPyPointer, AsPyRef, FromPyPointer, IntoPyPointer, PyNativeType, PyObject, PyTryFrom,
};
use std::ffi::CString; use std::ffi::CString;
use std::marker::PhantomData; use std::marker::PhantomData;
use std::os::raw::c_int; use std::os::raw::c_int;
@ -253,7 +251,7 @@ impl<'p> Python<'p> {
where where
T: PyTypeObject, T: PyTypeObject,
{ {
unsafe { self.from_borrowed_ptr(T::type_object().into_ptr()) } T::type_object(self)
} }
/// Imports the Python module with the specified name. /// Imports the Python module with the specified name.
@ -265,7 +263,7 @@ impl<'p> Python<'p> {
/// ///
/// This is equivalent to the Python `isinstance` function. /// This is equivalent to the Python `isinstance` function.
pub fn is_instance<T: PyTypeObject, V: AsPyPointer>(self, obj: &V) -> PyResult<bool> { pub fn is_instance<T: PyTypeObject, V: AsPyPointer>(self, obj: &V) -> PyResult<bool> {
T::type_object().as_ref(self).is_instance(obj) T::type_object(self).is_instance(obj)
} }
/// Checks whether type `T` is subclass of type `U`. /// Checks whether type `T` is subclass of type `U`.
@ -276,7 +274,7 @@ impl<'p> Python<'p> {
T: PyTypeObject, T: PyTypeObject,
U: PyTypeObject, U: PyTypeObject,
{ {
T::type_object().as_ref(self).is_subclass::<U>() T::type_object(self).is_subclass::<U>()
} }
/// Gets the Python builtin value `None`. /// Gets the Python builtin value `None`.

View File

@ -4,7 +4,7 @@
use crate::pyclass::{initialize_type_object, PyClass}; use crate::pyclass::{initialize_type_object, PyClass};
use crate::pyclass_init::PyObjectInit; use crate::pyclass_init::PyObjectInit;
use crate::types::{PyAny, PyType}; use crate::types::{PyAny, PyType};
use crate::{ffi, AsPyPointer, Py, Python}; use crate::{ffi, AsPyPointer, Python};
use std::cell::UnsafeCell; use std::cell::UnsafeCell;
use std::ptr::NonNull; use std::ptr::NonNull;
use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::atomic::{AtomicBool, Ordering};
@ -124,15 +124,15 @@ pub unsafe trait PyTypeInfo: Sized {
/// See [PyTypeInfo::type_object] /// See [PyTypeInfo::type_object]
pub unsafe trait PyTypeObject { pub unsafe trait PyTypeObject {
/// Returns the safe abstraction over the type object. /// Returns the safe abstraction over the type object.
fn type_object() -> Py<PyType>; fn type_object(py: Python) -> &PyType;
} }
unsafe impl<T> PyTypeObject for T unsafe impl<T> PyTypeObject for T
where where
T: PyTypeInfo, T: PyTypeInfo,
{ {
fn type_object() -> Py<PyType> { fn type_object(py: Python) -> &PyType {
unsafe { Py::from_borrowed_ptr(<Self as PyTypeInfo>::type_object() as *const _ as _) } unsafe { py.from_borrowed_ptr(<Self as PyTypeInfo>::type_object() as *const _ as _) }
} }
} }

View File

@ -479,11 +479,11 @@ mod test {
{ {
let _pool = unsafe { crate::GILPool::new() }; let _pool = unsafe { crate::GILPool::new() };
let none = py.None(); let none = py.None();
cnt = none.get_refcnt(); cnt = none.get_refcnt(py);
let _dict = [(10, none)].into_py_dict(py); let _dict = [(10, none)].into_py_dict(py);
} }
{ {
assert_eq!(cnt, py.None().get_refcnt()); assert_eq!(cnt, py.None().get_refcnt(py));
} }
} }

View File

@ -118,7 +118,7 @@ mod tests {
let gil_guard = Python::acquire_gil(); let gil_guard = Python::acquire_gil();
let py = gil_guard.python(); let py = gil_guard.python();
obj = vec![10, 20].to_object(py); obj = vec![10, 20].to_object(py);
count = obj.get_refcnt(); count = obj.get_refcnt(py);
} }
{ {
@ -129,7 +129,7 @@ mod tests {
assert_eq!(10, it.next().unwrap().unwrap().extract().unwrap()); assert_eq!(10, it.next().unwrap().unwrap().extract().unwrap());
} }
assert_eq!(count, obj.get_refcnt()); assert_eq!(count, obj.get_refcnt(Python::acquire_gil().python()));
} }
#[test] #[test]
@ -146,7 +146,7 @@ mod tests {
none = py.None(); none = py.None();
l.append(10).unwrap(); l.append(10).unwrap();
l.append(&none).unwrap(); l.append(&none).unwrap();
count = none.get_refcnt(); count = none.get_refcnt(py);
obj = l.to_object(py); obj = l.to_object(py);
} }
@ -158,7 +158,7 @@ mod tests {
assert_eq!(10, it.next().unwrap().unwrap().extract().unwrap()); assert_eq!(10, it.next().unwrap().unwrap().extract().unwrap());
assert!(it.next().unwrap().unwrap().is_none()); assert!(it.next().unwrap().unwrap().is_none());
} }
assert_eq!(count, none.get_refcnt()); assert_eq!(count, none.get_refcnt(py));
} }
#[test] #[test]

View File

@ -303,11 +303,11 @@ mod test {
let ob = v.to_object(py); let ob = v.to_object(py);
let list = <PyList as PyTryFrom>::try_from(ob.as_ref(py)).unwrap(); let list = <PyList as PyTryFrom>::try_from(ob.as_ref(py)).unwrap();
let none = py.None(); let none = py.None();
cnt = none.get_refcnt(); cnt = none.get_refcnt(py);
list.set_item(0, none).unwrap(); list.set_item(0, none).unwrap();
} }
assert_eq!(cnt, py.None().get_refcnt()); assert_eq!(cnt, py.None().get_refcnt(py));
} }
#[test] #[test]
@ -336,11 +336,11 @@ mod test {
let _pool = unsafe { crate::GILPool::new() }; let _pool = unsafe { crate::GILPool::new() };
let list = PyList::empty(py); let list = PyList::empty(py);
let none = py.None(); let none = py.None();
cnt = none.get_refcnt(); cnt = none.get_refcnt(py);
list.insert(0, none).unwrap(); list.insert(0, none).unwrap();
} }
assert_eq!(cnt, py.None().get_refcnt()); assert_eq!(cnt, py.None().get_refcnt(py));
} }
#[test] #[test]
@ -365,10 +365,10 @@ mod test {
let _pool = unsafe { crate::GILPool::new() }; let _pool = unsafe { crate::GILPool::new() };
let list = PyList::empty(py); let list = PyList::empty(py);
let none = py.None(); let none = py.None();
cnt = none.get_refcnt(); cnt = none.get_refcnt(py);
list.append(none).unwrap(); list.append(none).unwrap();
} }
assert_eq!(cnt, py.None().get_refcnt()); assert_eq!(cnt, py.None().get_refcnt(py));
} }
#[test] #[test]

View File

@ -176,7 +176,7 @@ impl PyModule {
where where
T: PyClass, T: PyClass,
{ {
self.add(T::NAME, <T as PyTypeObject>::type_object()) self.add(T::NAME, <T as PyTypeObject>::type_object(self.py()))
} }
/// Adds a function or a (sub)module to a module, using the functions __name__ as name. /// Adds a function or a (sub)module to a module, using the functions __name__ as name.

View File

@ -521,8 +521,8 @@ mod test {
} }
{ {
let gil = Python::acquire_gil(); let gil = Python::acquire_gil();
let _py = gil.python(); let py = gil.python();
assert_eq!(1, obj.get_refcnt()); assert_eq!(1, obj.get_refcnt(py));
} }
} }

View File

@ -52,16 +52,19 @@ impl PyTuple {
} }
/// Takes a slice of the tuple pointed from `low` to `high` and returns it as a new tuple. /// Takes a slice of the tuple pointed from `low` to `high` and returns it as a new tuple.
pub fn slice(&self, low: isize, high: isize) -> Py<PyTuple> { pub fn slice(&self, low: isize, high: isize) -> &PyTuple {
unsafe { Py::from_owned_ptr_or_panic(ffi::PyTuple_GetSlice(self.as_ptr(), low, high)) } unsafe {
self.py()
.from_owned_ptr(ffi::PyTuple_GetSlice(self.as_ptr(), low, high))
}
} }
/// Takes a slice of the tuple from `low` to the end and returns it as a new tuple. /// Takes a slice of the tuple from `low` to the end and returns it as a new tuple.
pub fn split_from(&self, low: isize) -> Py<PyTuple> { pub fn split_from(&self, low: isize) -> &PyTuple {
unsafe { unsafe {
let ptr = let ptr =
ffi::PyTuple_GetSlice(self.as_ptr(), low, ffi::PyTuple_GET_SIZE(self.as_ptr())); ffi::PyTuple_GetSlice(self.as_ptr(), low, ffi::PyTuple_GET_SIZE(self.as_ptr()));
Py::from_owned_ptr_or_panic(ptr) self.py().from_owned_ptr(ptr)
} }
} }
@ -128,7 +131,7 @@ impl<'a> IntoIterator for &'a PyTuple {
impl<'a> FromPy<&'a PyTuple> for Py<PyTuple> { impl<'a> FromPy<&'a PyTuple> for Py<PyTuple> {
fn from_py(tuple: &'a PyTuple, _py: Python) -> Py<PyTuple> { fn from_py(tuple: &'a PyTuple, _py: Python) -> Py<PyTuple> {
unsafe { Py::from_borrowed_ptr(tuple.as_ptr()) } unsafe { Py::from_borrowed_ptr(tuple.py(), tuple.as_ptr()) }
} }
} }
@ -166,7 +169,7 @@ macro_rules! tuple_conversion ({$length:expr,$(($refN:ident, $n:tt, $T:ident)),+
unsafe { unsafe {
let ptr = ffi::PyTuple_New($length); let ptr = ffi::PyTuple_New($length);
$(ffi::PyTuple_SetItem(ptr, $n, self.$n.into_py(py).into_ptr());)+ $(ffi::PyTuple_SetItem(ptr, $n, self.$n.into_py(py).into_ptr());)+
Py::from_owned_ptr_or_panic(ptr) Py::from_owned_ptr_or_panic(py, ptr)
} }
} }
} }

View File

@ -3,7 +3,7 @@
// based on Daniel Grunwald's https://github.com/dgrunwald/rust-cpython // based on Daniel Grunwald's https://github.com/dgrunwald/rust-cpython
use crate::err::{PyErr, PyResult}; use crate::err::{PyErr, PyResult};
use crate::instance::{Py, PyNativeType}; use crate::instance::PyNativeType;
use crate::type_object::PyTypeObject; use crate::type_object::PyTypeObject;
use crate::{ffi, AsPyPointer, PyAny, Python}; use crate::{ffi, AsPyPointer, PyAny, Python};
use std::borrow::Cow; use std::borrow::Cow;
@ -18,8 +18,8 @@ pyobject_native_var_type!(PyType, ffi::PyType_Type, ffi::PyType_Check);
impl PyType { impl PyType {
/// Creates a new type object. /// Creates a new type object.
#[inline] #[inline]
pub fn new<T: PyTypeObject>() -> Py<PyType> { pub fn new<T: PyTypeObject>(py: Python) -> &PyType {
T::type_object() T::type_object(py)
} }
/// Retrieves the underlying FFI pointer associated with this Python object. /// Retrieves the underlying FFI pointer associated with this Python object.
@ -48,7 +48,8 @@ impl PyType {
where where
T: PyTypeObject, T: PyTypeObject,
{ {
let result = unsafe { ffi::PyObject_IsSubclass(self.as_ptr(), T::type_object().as_ptr()) }; let result =
unsafe { ffi::PyObject_IsSubclass(self.as_ptr(), T::type_object(self.py()).as_ptr()) };
if result == -1 { if result == -1 {
Err(PyErr::fetch(self.py())) Err(PyErr::fetch(self.py()))
} else if result == 1 { } else if result == 1 {

View File

@ -284,7 +284,7 @@ fn gc_during_borrow() {
} }
// get the traverse function // get the traverse function
let ty = TraversableClass::type_object().as_ref(py).as_type_ptr(); let ty = TraversableClass::type_object(py).as_type_ptr();
let traverse = (*ty).tp_traverse.unwrap(); let traverse = (*ty).tp_traverse.unwrap();
// create an object and check that traversing it works normally // create an object and check that traversing it works normally