diff --git a/CHANGELOG.md b/CHANGELOG.md index 0fb01598..aaebe7ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) - 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) +- Add `Python` argument to most methods on `PyObject` and `Py` 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` 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) ### Removed diff --git a/src/conversion.rs b/src/conversion.rs index 051d0b8d..52e68439 100644 --- a/src/conversion.rs +++ b/src/conversion.rs @@ -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 { match Self::from_owned_ptr_or_opt(py, ptr) { 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 { @@ -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 { match Self::from_borrowed_ptr_or_opt(py, ptr) { 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 { diff --git a/src/err.rs b/src/err.rs index 43339628..fde89295 100644 --- a/src/err.rs +++ b/src/err.rs @@ -1,12 +1,13 @@ // Copyright (c) 2017-present PyO3 Project and Contributors +use crate::gil::ensure_gil; use crate::panic::PanicException; use crate::type_object::PyTypeObject; use crate::types::PyType; use crate::{exceptions, ffi}; use crate::{ - AsPyPointer, FromPy, FromPyPointer, IntoPy, IntoPyPointer, Py, PyAny, PyObject, Python, - ToBorrowedObject, ToPyObject, + AsPyPointer, FromPy, FromPyPointer, IntoPy, IntoPyPointer, Py, PyAny, PyNativeType, PyObject, + Python, ToBorrowedObject, ToPyObject, }; use libc::c_int; use std::ffi::CString; @@ -88,11 +89,14 @@ impl PyErr { T: PyTypeObject, 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); PyErr { - ptype: ty, + ptype: ty.into(), pvalue: PyErrValue::ToObject(Box::new(value)), ptraceback: None, } @@ -103,12 +107,12 @@ impl PyErr { /// `exc` is the exception type; usually one of the standard exceptions /// like `exceptions::RuntimeError`. /// `args` is the a tuple of arguments to pass to the exception constructor. - pub fn from_type(exc: Py, args: A) -> PyErr + pub fn from_type(exc: &PyType, args: A) -> PyErr where A: ToPyObject + 'static, { PyErr { - ptype: exc, + ptype: exc.into(), pvalue: PyErrValue::ToObject(Box::new(args)), ptraceback: None, } @@ -119,11 +123,14 @@ impl PyErr { where 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); PyErr { - ptype: ty, + ptype: ty.into(), pvalue: value, ptraceback: None, } @@ -140,19 +147,21 @@ impl PyErr { if unsafe { ffi::PyExceptionInstance_Check(ptr) } != 0 { 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()), ptraceback: None, } } else if unsafe { ffi::PyExceptionClass_Check(obj.as_ptr()) } != 0 { PyErr { - ptype: unsafe { Py::from_borrowed_ptr(ptr) }, + ptype: unsafe { Py::from_borrowed_ptr(obj.py(), ptr) }, pvalue: PyErrValue::None, ptraceback: None, } } else { PyErr { - ptype: exceptions::TypeError::type_object(), + ptype: exceptions::TypeError::type_object(obj.py()).into(), pvalue: PyErrValue::ToObject(Box::new("exceptions must derive from BaseException")), ptraceback: None, } @@ -179,9 +188,9 @@ impl PyErr { let mut ptraceback: *mut ffi::PyObject = std::ptr::null_mut(); 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) .and_then(|obj| obj.extract().ok()) .unwrap_or_else(|| String::from("Unwrapped panic from Python code")); @@ -233,6 +242,7 @@ impl PyErr { } unsafe fn new_from_ffi_tuple( + py: Python, ptype: *mut ffi::PyObject, pvalue: *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, // and because we mustn't panic in normalize(). - let pvalue = if let Some(obj) = - PyObject::from_owned_ptr_or_opt(Python::assume_gil_acquired(), pvalue) - { + let pvalue = if let Some(obj) = PyObject::from_owned_ptr_or_opt(py, pvalue) { PyErrValue::Value(obj) } else { PyErrValue::None }; let ptype = if ptype.is_null() { - ::type_object() + ::type_object(py).into() } else { - Py::from_owned_ptr(ptype) + Py::from_owned_ptr(py, ptype) }; PyErr { ptype, 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`. - pub fn is_instance(&self, _py: Python) -> bool + pub fn is_instance(&self, py: Python) -> bool where T: PyTypeObject, { 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(); unsafe { 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::net::AddrParseError, exceptions::ValueError); -pub fn panic_after_error() -> ! { +pub fn panic_after_error(_py: Python) -> ! { unsafe { ffi::PyErr_Print(); } diff --git a/src/exceptions.rs b/src/exceptions.rs index 85469614..4237287a 100644 --- a/src/exceptions.rs +++ b/src/exceptions.rs @@ -89,7 +89,7 @@ macro_rules! import_exception { macro_rules! import_exception_type_object { ($module: expr, $name: ident) => { 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; 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 { ($module: ident, $name: ident, $base: ty) => { 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; 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 { - fn type_object() -> $crate::Py<$crate::types::PyType> { - unsafe { $crate::Py::from_borrowed_ptr(ffi::$exc_name) } + fn type_object(py: $crate::Python) -> &$crate::types::PyType { + unsafe { py.from_borrowed_ptr(ffi::$exc_name) } } } ); diff --git a/src/gil.rs b/src/gil.rs index bcff6258..fb7c409c 100644 --- a/src/gil.rs +++ b/src/gil.rs @@ -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); + +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)] mod test { use super::{gil_is_acquired, GILPool, GIL_COUNT, OWNED_OBJECTS, POOL}; @@ -569,13 +598,15 @@ mod test { #[test] fn test_clone_with_gil() { let gil = Python::acquire_gil(); - let obj = get_object(gil.python()); - let count = obj.get_refcnt(); + let py = gil.python(); + + let obj = get_object(py); + let count = obj.get_refcnt(py); // Cloning with the GIL should increase reference count immediately #[allow(clippy::redundant_clone)] let c = obj.clone(); - assert_eq!(count + 1, c.get_refcnt()); + assert_eq!(count + 1, c.get_refcnt(py)); } #[test] @@ -583,39 +614,46 @@ mod test { let gil = Python::acquire_gil(); let py = gil.python(); let obj = get_object(py); - let count = obj.get_refcnt(); + let count = obj.get_refcnt(py); // Cloning without GIL should not update reference count drop(gil); 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 let gil = Python::acquire_gil(); + let py = gil.python(); // 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(gil); // 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] fn test_clone_in_other_thread() { let gil = Python::acquire_gil(); - let obj = get_object(gil.python()); - let count = obj.get_refcnt(); + let py = gil.python(); + 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 let t = std::thread::spawn(move || { // Cloning without GIL should not update reference count #[allow(clippy::redundant_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 obj @@ -631,13 +669,13 @@ mod test { // Re-acquring GIL will clear these pending changes drop(gil); - let _gil = Python::acquire_gil(); + let gil = Python::acquire_gil(); assert!(POOL.pointers_to_incref.lock().is_empty()); assert!(POOL.pointers_to_decref.lock().is_empty()); // Overall count is still unchanged - assert_eq!(count, obj.get_refcnt()); + assert_eq!(count, obj.get_refcnt(gil.python())); } #[test] diff --git a/src/instance.rs b/src/instance.rs index 351d0082..42403ba1 100644 --- a/src/instance.rs +++ b/src/instance.rs @@ -61,7 +61,7 @@ impl Py { { let initializer = value.into(); 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) } @@ -70,7 +70,7 @@ impl Py { /// This moves ownership over the pointer into the `Py`. /// Undefined behavior if the pointer is NULL or invalid. #[inline] - pub unsafe fn from_owned_ptr(ptr: *mut ffi::PyObject) -> Py { + pub unsafe fn from_owned_ptr(_py: Python, ptr: *mut ffi::PyObject) -> Py { debug_assert!( !ptr.is_null() && ffi::Py_REFCNT(ptr) > 0, format!("REFCNT: {:?} - {:?}", ptr, ffi::Py_REFCNT(ptr)) @@ -83,11 +83,11 @@ impl Py { /// Panics if the pointer is NULL. /// Undefined behavior if the pointer is invalid. #[inline] - pub unsafe fn from_owned_ptr_or_panic(ptr: *mut ffi::PyObject) -> Py { + pub unsafe fn from_owned_ptr_or_panic(_py: Python, ptr: *mut ffi::PyObject) -> Py { match NonNull::new(ptr) { Some(nonnull_ptr) => Py(nonnull_ptr, PhantomData), None => { - crate::err::panic_after_error(); + crate::err::panic_after_error(_py); } } } @@ -109,7 +109,7 @@ impl Py { /// Calls `Py_INCREF()` on the ptr. /// Undefined behavior if the pointer is NULL or invalid. #[inline] - pub unsafe fn from_borrowed_ptr(ptr: *mut ffi::PyObject) -> Py { + pub unsafe fn from_borrowed_ptr(_py: Python, ptr: *mut ffi::PyObject) -> Py { debug_assert!( !ptr.is_null() && ffi::Py_REFCNT(ptr) > 0, format!("REFCNT: {:?} - {:?}", ptr, ffi::Py_REFCNT(ptr)) @@ -120,14 +120,14 @@ impl Py { /// Gets the reference count of the `ffi::PyObject` pointer. #[inline] - pub fn get_refcnt(&self) -> isize { + pub fn get_refcnt(&self, _py: Python) -> isize { unsafe { ffi::Py_REFCNT(self.0.as_ptr()) } } /// Clones self by calling `Py_INCREF()` on the ptr. #[inline] - pub fn clone_ref(&self, _py: Python) -> Py { - unsafe { Py::from_borrowed_ptr(self.0.as_ptr()) } + pub fn clone_ref(&self, py: Python) -> Py { + unsafe { Py::from_borrowed_ptr(py, self.0.as_ptr()) } } /// Returns the inner pointer without decreasing the refcount. @@ -225,7 +225,7 @@ where T: AsPyPointer + PyNativeType, { 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, { fn from(cell: &PyCell) -> 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, { 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, { 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 std::convert::From> for PyObject { impl<'a, T> std::convert::From<&'a T> for PyObject where - T: AsPyPointer, + T: AsPyPointer + PyNativeType, { fn from(ob: &'a T) -> Self { - unsafe { Py::::from_borrowed_ptr(ob.as_ptr()) }.into() + unsafe { Py::::from_borrowed_ptr(ob.py(), ob.as_ptr()) }.into() } } impl<'a, T> std::convert::From<&'a mut T> for PyObject where - T: AsPyPointer, + T: AsPyPointer + PyNativeType, { fn from(ob: &'a mut T) -> Self { - unsafe { Py::::from_borrowed_ptr(ob.as_ptr()) }.into() + unsafe { Py::::from_borrowed_ptr(ob.py(), ob.as_ptr()) }.into() } } @@ -317,7 +317,7 @@ where fn extract(ob: &'a PyAny) -> PyResult { unsafe { ob.extract::<&T::AsRefTarget>() - .map(|val| Py::from_borrowed_ptr(val.as_ptr())) + .map(|val| Py::from_borrowed_ptr(ob.py(), val.as_ptr())) } } } diff --git a/src/object.rs b/src/object.rs index d3610dd4..a30359e9 100644 --- a/src/object.rs +++ b/src/object.rs @@ -48,11 +48,11 @@ impl PyObject { /// Panics if the pointer is NULL. /// Undefined behavior if the pointer is invalid. #[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) { Some(nonnull_ptr) => PyObject(nonnull_ptr), 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. - pub fn get_refcnt(&self) -> isize { + pub fn get_refcnt(&self, _py: Python) -> isize { unsafe { ffi::Py_REFCNT(self.0.as_ptr()) } } @@ -131,7 +131,7 @@ impl PyObject { /// Returns whether the object is considered to be 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() } } diff --git a/src/python.rs b/src/python.rs index 28b673f5..fb9e29fc 100644 --- a/src/python.rs +++ b/src/python.rs @@ -6,9 +6,7 @@ use crate::err::{PyDowncastError, PyErr, PyResult}; use crate::gil::{self, GILGuard, GILPool}; use crate::type_object::{PyTypeInfo, PyTypeObject}; use crate::types::{PyAny, PyDict, PyModule, PyType}; -use crate::{ - ffi, AsPyPointer, AsPyRef, FromPyPointer, IntoPyPointer, PyNativeType, PyObject, PyTryFrom, -}; +use crate::{ffi, AsPyPointer, FromPyPointer, IntoPyPointer, PyNativeType, PyObject, PyTryFrom}; use std::ffi::CString; use std::marker::PhantomData; use std::os::raw::c_int; @@ -253,7 +251,7 @@ impl<'p> Python<'p> { where T: PyTypeObject, { - unsafe { self.from_borrowed_ptr(T::type_object().into_ptr()) } + T::type_object(self) } /// Imports the Python module with the specified name. @@ -265,7 +263,7 @@ impl<'p> Python<'p> { /// /// This is equivalent to the Python `isinstance` function. pub fn is_instance(self, obj: &V) -> PyResult { - 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`. @@ -276,7 +274,7 @@ impl<'p> Python<'p> { T: PyTypeObject, U: PyTypeObject, { - T::type_object().as_ref(self).is_subclass::() + T::type_object(self).is_subclass::() } /// Gets the Python builtin value `None`. diff --git a/src/type_object.rs b/src/type_object.rs index 2e1a47f2..19d2b8d2 100644 --- a/src/type_object.rs +++ b/src/type_object.rs @@ -4,7 +4,7 @@ use crate::pyclass::{initialize_type_object, PyClass}; use crate::pyclass_init::PyObjectInit; use crate::types::{PyAny, PyType}; -use crate::{ffi, AsPyPointer, Py, Python}; +use crate::{ffi, AsPyPointer, Python}; use std::cell::UnsafeCell; use std::ptr::NonNull; use std::sync::atomic::{AtomicBool, Ordering}; @@ -124,15 +124,15 @@ pub unsafe trait PyTypeInfo: Sized { /// See [PyTypeInfo::type_object] pub unsafe trait PyTypeObject { /// Returns the safe abstraction over the type object. - fn type_object() -> Py; + fn type_object(py: Python) -> &PyType; } unsafe impl PyTypeObject for T where T: PyTypeInfo, { - fn type_object() -> Py { - unsafe { Py::from_borrowed_ptr(::type_object() as *const _ as _) } + fn type_object(py: Python) -> &PyType { + unsafe { py.from_borrowed_ptr(::type_object() as *const _ as _) } } } diff --git a/src/types/dict.rs b/src/types/dict.rs index 4e4cca72..02d59fa6 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -479,11 +479,11 @@ mod test { { let _pool = unsafe { crate::GILPool::new() }; let none = py.None(); - cnt = none.get_refcnt(); + cnt = none.get_refcnt(py); let _dict = [(10, none)].into_py_dict(py); } { - assert_eq!(cnt, py.None().get_refcnt()); + assert_eq!(cnt, py.None().get_refcnt(py)); } } diff --git a/src/types/iterator.rs b/src/types/iterator.rs index 86e17f06..2c192488 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -118,7 +118,7 @@ mod tests { let gil_guard = Python::acquire_gil(); let py = gil_guard.python(); 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!(count, obj.get_refcnt()); + assert_eq!(count, obj.get_refcnt(Python::acquire_gil().python())); } #[test] @@ -146,7 +146,7 @@ mod tests { none = py.None(); l.append(10).unwrap(); l.append(&none).unwrap(); - count = none.get_refcnt(); + count = none.get_refcnt(py); obj = l.to_object(py); } @@ -158,7 +158,7 @@ mod tests { assert_eq!(10, it.next().unwrap().unwrap().extract().unwrap()); assert!(it.next().unwrap().unwrap().is_none()); } - assert_eq!(count, none.get_refcnt()); + assert_eq!(count, none.get_refcnt(py)); } #[test] diff --git a/src/types/list.rs b/src/types/list.rs index 50129e89..4191a466 100644 --- a/src/types/list.rs +++ b/src/types/list.rs @@ -303,11 +303,11 @@ mod test { let ob = v.to_object(py); let list = ::try_from(ob.as_ref(py)).unwrap(); let none = py.None(); - cnt = none.get_refcnt(); + cnt = none.get_refcnt(py); list.set_item(0, none).unwrap(); } - assert_eq!(cnt, py.None().get_refcnt()); + assert_eq!(cnt, py.None().get_refcnt(py)); } #[test] @@ -336,11 +336,11 @@ mod test { let _pool = unsafe { crate::GILPool::new() }; let list = PyList::empty(py); let none = py.None(); - cnt = none.get_refcnt(); + cnt = none.get_refcnt(py); list.insert(0, none).unwrap(); } - assert_eq!(cnt, py.None().get_refcnt()); + assert_eq!(cnt, py.None().get_refcnt(py)); } #[test] @@ -365,10 +365,10 @@ mod test { let _pool = unsafe { crate::GILPool::new() }; let list = PyList::empty(py); let none = py.None(); - cnt = none.get_refcnt(); + cnt = none.get_refcnt(py); list.append(none).unwrap(); } - assert_eq!(cnt, py.None().get_refcnt()); + assert_eq!(cnt, py.None().get_refcnt(py)); } #[test] diff --git a/src/types/module.rs b/src/types/module.rs index db15bdd9..f586ee22 100644 --- a/src/types/module.rs +++ b/src/types/module.rs @@ -176,7 +176,7 @@ impl PyModule { where T: PyClass, { - self.add(T::NAME, ::type_object()) + self.add(T::NAME, ::type_object(self.py())) } /// Adds a function or a (sub)module to a module, using the functions __name__ as name. diff --git a/src/types/sequence.rs b/src/types/sequence.rs index 98d667ea..33a84a92 100644 --- a/src/types/sequence.rs +++ b/src/types/sequence.rs @@ -521,8 +521,8 @@ mod test { } { let gil = Python::acquire_gil(); - let _py = gil.python(); - assert_eq!(1, obj.get_refcnt()); + let py = gil.python(); + assert_eq!(1, obj.get_refcnt(py)); } } diff --git a/src/types/tuple.rs b/src/types/tuple.rs index 9549ce0f..39c7aeca 100644 --- a/src/types/tuple.rs +++ b/src/types/tuple.rs @@ -52,16 +52,19 @@ impl PyTuple { } /// 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 { - unsafe { Py::from_owned_ptr_or_panic(ffi::PyTuple_GetSlice(self.as_ptr(), low, high)) } + pub fn slice(&self, low: isize, high: isize) -> &PyTuple { + 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. - pub fn split_from(&self, low: isize) -> Py { + pub fn split_from(&self, low: isize) -> &PyTuple { unsafe { let 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 { fn from_py(tuple: &'a PyTuple, _py: Python) -> Py { - 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 { let ptr = ffi::PyTuple_New($length); $(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) } } } diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index a3cfc26d..5b80d39d 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -3,7 +3,7 @@ // based on Daniel Grunwald's https://github.com/dgrunwald/rust-cpython use crate::err::{PyErr, PyResult}; -use crate::instance::{Py, PyNativeType}; +use crate::instance::PyNativeType; use crate::type_object::PyTypeObject; use crate::{ffi, AsPyPointer, PyAny, Python}; use std::borrow::Cow; @@ -18,8 +18,8 @@ pyobject_native_var_type!(PyType, ffi::PyType_Type, ffi::PyType_Check); impl PyType { /// Creates a new type object. #[inline] - pub fn new() -> Py { - T::type_object() + pub fn new(py: Python) -> &PyType { + T::type_object(py) } /// Retrieves the underlying FFI pointer associated with this Python object. @@ -48,7 +48,8 @@ impl PyType { where 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 { Err(PyErr::fetch(self.py())) } else if result == 1 { diff --git a/tests/test_gc.rs b/tests/test_gc.rs index ba63b2a5..d6a27a17 100644 --- a/tests/test_gc.rs +++ b/tests/test_gc.rs @@ -284,7 +284,7 @@ fn gc_during_borrow() { } // 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(); // create an object and check that traversing it works normally