diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a20a58b..6d59059f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. * Implementing the Using the `gc` parameter for `pyclass` (e.g. `#[pyclass(gc)]`) without implementing the `class::PyGCProtocol` trait is now a compile-time error. Failing to implement this trait could lead to segfaults. [#532](https://github.com/PyO3/pyo3/pull/532) * `PyByteArray::data` has been replaced with `PyDataArray::to_vec` because returning a `&[u8]` is unsound. (See [this comment](https://github.com/PyO3/pyo3/issues/373#issuecomment-512332696) for a great write-up for why that was unsound) * Replace `mashup` with `paste`. + * `GILPool` gained a `Python` marker to prevent it from being misused to release Python objects without the GIL held. ## [0.7.0] - 2018-05-26 diff --git a/pyo3-derive-backend/src/module.rs b/pyo3-derive-backend/src/module.rs index 2b6d4562..181cde1c 100644 --- a/pyo3-derive-backend/src/module.rs +++ b/pyo3-derive-backend/src/module.rs @@ -204,8 +204,8 @@ fn function_c_wrapper(name: &Ident, spec: &method::FnSpec<'_>) -> TokenStream { { const _LOCATION: &'static str = concat!(stringify!(#name), "()"); - let _pool = pyo3::GILPool::new(); let _py = pyo3::Python::assume_gil_acquired(); + let _pool = pyo3::GILPool::new(_py); let _args = _py.from_borrowed_ptr::(_args); let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs); diff --git a/pyo3-derive-backend/src/pymethod.rs b/pyo3-derive-backend/src/pymethod.rs index f9731bf6..08e2ba7d 100644 --- a/pyo3-derive-backend/src/pymethod.rs +++ b/pyo3-derive-backend/src/pymethod.rs @@ -113,8 +113,8 @@ fn impl_wrap_common( { const _LOCATION: &'static str = concat!( stringify!(#cls), ".", stringify!(#name), "()"); - let _pool = pyo3::GILPool::new(); let _py = pyo3::Python::assume_gil_acquired(); + let _pool = pyo3::GILPool::new(_py); #slf let _result = { pyo3::derive_utils::IntoPyResult::into_py_result(#body) @@ -135,8 +135,8 @@ fn impl_wrap_common( { const _LOCATION: &'static str = concat!( stringify!(#cls), ".", stringify!(#name), "()"); - let _pool = pyo3::GILPool::new(); let _py = pyo3::Python::assume_gil_acquired(); + let _pool = pyo3::GILPool::new(_py); #slf let _args = _py.from_borrowed_ptr::(_args); let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs); @@ -163,8 +163,8 @@ pub fn impl_proto_wrap(cls: &syn::Type, name: &syn::Ident, spec: &FnSpec<'_>) -> _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject { const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#name),"()"); - let _pool = pyo3::GILPool::new(); let _py = pyo3::Python::assume_gil_acquired(); + let _pool = pyo3::GILPool::new(_py); let _slf = _py.mut_from_borrowed_ptr::<#cls>(_slf); let _args = _py.from_borrowed_ptr::(_args); let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs); @@ -194,8 +194,8 @@ pub fn impl_wrap_new(cls: &syn::Type, name: &syn::Ident, spec: &FnSpec<'_>) -> T use pyo3::type_object::PyTypeInfo; const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#name),"()"); - let _pool = pyo3::GILPool::new(); let _py = pyo3::Python::assume_gil_acquired(); + let _pool = pyo3::GILPool::new(_py); match pyo3::type_object::PyRawObject::new(_py, #cls::type_object(), _cls) { Ok(_obj) => { let _args = _py.from_borrowed_ptr::(_args); @@ -240,8 +240,8 @@ fn impl_wrap_init(cls: &syn::Type, name: &syn::Ident, spec: &FnSpec<'_>) -> Toke _kwargs: *mut pyo3::ffi::PyObject) -> pyo3::libc::c_int { const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#name),"()"); - let _pool = pyo3::GILPool::new(); let _py = pyo3::Python::assume_gil_acquired(); + let _pool = pyo3::GILPool::new(_py); let _slf = _py.mut_from_borrowed_ptr::<#cls>(_slf); let _args = _py.from_borrowed_ptr::(_args); let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs); @@ -274,8 +274,8 @@ pub fn impl_wrap_class(cls: &syn::Type, name: &syn::Ident, spec: &FnSpec<'_>) -> _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject { const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#name),"()"); - let _pool = pyo3::GILPool::new(); let _py = pyo3::Python::assume_gil_acquired(); + let _pool = pyo3::GILPool::new(_py); let _cls = pyo3::types::PyType::from_type_ptr(_py, _cls as *mut pyo3::ffi::PyTypeObject); let _args = _py.from_borrowed_ptr::(_args); let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs); @@ -303,8 +303,8 @@ pub fn impl_wrap_static(cls: &syn::Type, name: &syn::Ident, spec: &FnSpec<'_>) - _kwargs: *mut pyo3::ffi::PyObject) -> *mut pyo3::ffi::PyObject { const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#name),"()"); - let _pool = pyo3::GILPool::new(); let _py = pyo3::Python::assume_gil_acquired(); + let _pool = pyo3::GILPool::new(_py); let _args = _py.from_borrowed_ptr::(_args); let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs); @@ -329,8 +329,8 @@ pub(crate) fn impl_wrap_getter(cls: &syn::Type, name: &syn::Ident, takes_py: boo { const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#name),"()"); - let _pool = pyo3::GILPool::new(); let _py = pyo3::Python::assume_gil_acquired(); + let _pool = pyo3::GILPool::new(_py); let _slf = _py.mut_from_borrowed_ptr::<#cls>(_slf); let result = pyo3::derive_utils::IntoPyResult::into_py_result(#fncall); @@ -370,8 +370,8 @@ pub(crate) fn impl_wrap_setter( _value: *mut pyo3::ffi::PyObject, _: *mut ::std::os::raw::c_void) -> pyo3::libc::c_int { const _LOCATION: &'static str = concat!(stringify!(#cls),".",stringify!(#name),"()"); - let _pool = pyo3::GILPool::new(); let _py = pyo3::Python::assume_gil_acquired(); + let _pool = pyo3::GILPool::new(_py); let _slf = _py.mut_from_borrowed_ptr::<#cls>(_slf); let _value = _py.from_borrowed_ptr(_value); diff --git a/src/class/basic.rs b/src/class/basic.rs index ea823f85..c3153dfa 100644 --- a/src/class/basic.rs +++ b/src/class/basic.rs @@ -215,8 +215,8 @@ where where T: for<'p> PyObjectGetAttrProtocol<'p>, { - let _pool = crate::GILPool::new(); let py = Python::assume_gil_acquired(); + let _pool = crate::GILPool::new(py); // Behave like python's __getattr__ (as opposed to __getattribute__) and check // for existing fields and methods first @@ -450,8 +450,8 @@ where where T: for<'p> PyObjectRichcmpProtocol<'p>, { - let _pool = crate::GILPool::new(); let py = Python::assume_gil_acquired(); + let _pool = crate::GILPool::new(py); let slf = py.from_borrowed_ptr::(slf); let arg = py.from_borrowed_ptr::(arg); diff --git a/src/class/buffer.rs b/src/class/buffer.rs index 000b434d..d36524e7 100644 --- a/src/class/buffer.rs +++ b/src/class/buffer.rs @@ -85,8 +85,8 @@ where where T: for<'p> PyBufferGetBufferProtocol<'p>, { - let _pool = crate::GILPool::new(); let py = crate::Python::assume_gil_acquired(); + let _pool = crate::GILPool::new(py); let slf = py.mut_from_borrowed_ptr::(slf); let result = slf.bf_getbuffer(arg1, arg2).into(); diff --git a/src/class/gc.rs b/src/class/gc.rs index 139004a9..eaa4d901 100644 --- a/src/class/gc.rs +++ b/src/class/gc.rs @@ -85,8 +85,8 @@ where where T: for<'p> PyGCTraverseProtocol<'p>, { - let _pool = crate::GILPool::new(); let py = Python::assume_gil_acquired(); + let _pool = crate::GILPool::new(py); let slf = py.mut_from_borrowed_ptr::(slf); let visit = PyVisit { @@ -122,8 +122,8 @@ where where T: for<'p> PyGCClearProtocol<'p>, { - let _pool = crate::GILPool::new(); let py = Python::assume_gil_acquired(); + let _pool = crate::GILPool::new(py); let slf = py.mut_from_borrowed_ptr::(slf); slf.__clear__(); diff --git a/src/class/macros.rs b/src/class/macros.rs index eb716836..b849aad5 100644 --- a/src/class/macros.rs +++ b/src/class/macros.rs @@ -17,8 +17,8 @@ macro_rules! py_unary_func { where T: for<'p> $trait<'p>, { - let _pool = $crate::GILPool::new(); let py = $crate::Python::assume_gil_acquired(); + let _pool = $crate::GILPool::new(py); let slf = py.mut_from_borrowed_ptr::(slf); let res = slf.$f().into(); $crate::callback::cb_convert($conv, py, res.map(|x| x)) @@ -36,8 +36,8 @@ macro_rules! py_unary_pyref_func { T: for<'p> $trait<'p>, { use $crate::instance::PyRefMut; - let _pool = $crate::GILPool::new(); let py = $crate::Python::assume_gil_acquired(); + let _pool = $crate::GILPool::new(py); let slf = py.mut_from_borrowed_ptr::(slf); let res = $class::$f(PyRefMut::from_mut(slf)).into(); $crate::callback::cb_convert($conv, py, res) @@ -54,8 +54,8 @@ macro_rules! py_len_func { where T: for<'p> $trait<'p>, { - let _pool = $crate::GILPool::new(); let py = Python::assume_gil_acquired(); + let _pool = $crate::GILPool::new(py); let slf = py.mut_from_borrowed_ptr::(slf); let result = slf.$f().into(); @@ -84,8 +84,8 @@ macro_rules! py_binary_func { T: for<'p> $trait<'p>, { use $crate::ObjectProtocol; - let _pool = $crate::GILPool::new(); let py = $crate::Python::assume_gil_acquired(); + let _pool = $crate::GILPool::new(py); let slf = py.mut_from_borrowed_ptr::(slf); let arg = py.from_borrowed_ptr::<$crate::types::PyAny>(arg); @@ -112,8 +112,8 @@ macro_rules! py_binary_num_func { T: for<'p> $trait<'p>, { use $crate::ObjectProtocol; - let _pool = $crate::GILPool::new(); let py = $crate::Python::assume_gil_acquired(); + let _pool = $crate::GILPool::new(py); let lhs = py.from_borrowed_ptr::<$crate::types::PyAny>(lhs); let rhs = py.from_borrowed_ptr::<$crate::types::PyAny>(rhs); @@ -144,8 +144,8 @@ macro_rules! py_binary_self_func { { use $crate::ObjectProtocol; - let _pool = $crate::GILPool::new(); let py = $crate::Python::assume_gil_acquired(); + let _pool = $crate::GILPool::new(py); let slf1 = py.mut_from_borrowed_ptr::(slf); let arg = py.from_borrowed_ptr::<$crate::types::PyAny>(arg); @@ -180,8 +180,8 @@ macro_rules! py_ssizearg_func { where T: for<'p> $trait<'p>, { - let _pool = $crate::GILPool::new(); let py = $crate::Python::assume_gil_acquired(); + let _pool = $crate::GILPool::new(py); let slf = py.mut_from_borrowed_ptr::(slf); let result = slf.$f(arg.into()).into(); $crate::callback::cb_convert($conv, py, result) @@ -213,8 +213,8 @@ macro_rules! py_ternary_func { { use $crate::ObjectProtocol; - let _pool = $crate::GILPool::new(); let py = $crate::Python::assume_gil_acquired(); + let _pool = $crate::GILPool::new(py); let slf = py.mut_from_borrowed_ptr::(slf); let arg1 = py.from_borrowed_ptr::<$crate::types::PyAny>(arg1); let arg2 = py.from_borrowed_ptr::<$crate::types::PyAny>(arg2); @@ -247,8 +247,8 @@ macro_rules! py_ternary_num_func { { use $crate::ObjectProtocol; - let _pool = $crate::GILPool::new(); let py = $crate::Python::assume_gil_acquired(); + let _pool = $crate::GILPool::new(py); let arg1 = py.from_borrowed_ptr::<$crate::types::PyAny>(arg1); let arg2 = py.from_borrowed_ptr::<$crate::types::PyAny>(arg2); let arg3 = py.from_borrowed_ptr::<$crate::types::PyAny>(arg3); @@ -284,8 +284,8 @@ macro_rules! py_ternary_self_func { { use $crate::ObjectProtocol; - let _pool = $crate::GILPool::new(); let py = $crate::Python::assume_gil_acquired(); + let _pool = $crate::GILPool::new(py); let slf1 = py.mut_from_borrowed_ptr::(slf); let arg1 = py.from_borrowed_ptr::<$crate::types::PyAny>(arg1); let arg2 = py.from_borrowed_ptr::<$crate::types::PyAny>(arg2); @@ -323,8 +323,8 @@ macro_rules! py_func_set { { use $crate::ObjectProtocol; - let _pool = $crate::GILPool::new(); let py = $crate::Python::assume_gil_acquired(); + let _pool = $crate::GILPool::new(py); let slf = py.mut_from_borrowed_ptr::<$generic>(slf); let result = if value.is_null() { @@ -371,8 +371,8 @@ macro_rules! py_func_del { { use $crate::ObjectProtocol; - let _pool = $crate::GILPool::new(); let py = $crate::Python::assume_gil_acquired(); + let _pool = $crate::GILPool::new(py); let result = if value.is_null() { let slf = py.mut_from_borrowed_ptr::(slf); @@ -413,8 +413,8 @@ macro_rules! py_func_set_del { { use $crate::ObjectProtocol; - let _pool = $crate::GILPool::new(); let py = $crate::Python::assume_gil_acquired(); + let _pool = $crate::GILPool::new(py); let slf = py.mut_from_borrowed_ptr::<$generic>(slf); let name = py.from_borrowed_ptr::<$crate::types::PyAny>(name); diff --git a/src/class/sequence.rs b/src/class/sequence.rs index 0d31353b..1f969722 100644 --- a/src/class/sequence.rs +++ b/src/class/sequence.rs @@ -221,8 +221,8 @@ where where T: for<'p> PySequenceSetItemProtocol<'p>, { - let _pool = crate::GILPool::new(); let py = Python::assume_gil_acquired(); + let _pool = crate::GILPool::new(py); let slf = py.mut_from_borrowed_ptr::(slf); let result = if value.is_null() { @@ -295,8 +295,8 @@ mod sq_ass_item_impl { where T: for<'p> PySequenceDelItemProtocol<'p>, { - let _pool = crate::GILPool::new(); let py = Python::assume_gil_acquired(); + let _pool = crate::GILPool::new(py); let slf = py.mut_from_borrowed_ptr::(slf); let result = if value.is_null() { @@ -341,8 +341,8 @@ mod sq_ass_item_impl { where T: for<'p> PySequenceSetItemProtocol<'p> + for<'p> PySequenceDelItemProtocol<'p>, { - let _pool = crate::GILPool::new(); let py = Python::assume_gil_acquired(); + let _pool = crate::GILPool::new(py); let slf = py.mut_from_borrowed_ptr::(slf); let result = if value.is_null() { diff --git a/src/derive_utils.rs b/src/derive_utils.rs index 55641c44..f2c2be6e 100644 --- a/src/derive_utils.rs +++ b/src/derive_utils.rs @@ -135,8 +135,8 @@ pub unsafe fn make_module( return module; } - let _pool = GILPool::new(); let py = Python::assume_gil_acquired(); + let _pool = GILPool::new(py); let module = match py.from_owned_ptr_or_err::(module) { Ok(m) => m, Err(e) => { diff --git a/src/gil.rs b/src/gil.rs index b6534eeb..7034af6c 100644 --- a/src/gil.rs +++ b/src/gil.rs @@ -111,7 +111,7 @@ impl Drop for GILGuard { fn drop(&mut self) { unsafe { let pool: &'static mut ReleasePool = &mut *POOL; - pool.drain(self.owned, self.borrowed, true); + pool.drain(self.python(), self.owned, self.borrowed, true); ffi::PyGILState_Release(self.gstate); } @@ -156,7 +156,7 @@ impl ReleasePool { vec.set_len(0); } - pub unsafe fn drain(&mut self, owned: usize, borrowed: usize, pointers: bool) { + pub unsafe fn drain(&mut self, _py: Python, owned: usize, borrowed: usize, pointers: bool) { // Release owned objects(call decref) while owned < self.owned.len() { let last = self.owned.pop_back().unwrap(); @@ -176,35 +176,31 @@ impl ReleasePool { static mut POOL: *mut ReleasePool = ::std::ptr::null_mut(); #[doc(hidden)] -pub struct GILPool { +pub struct GILPool<'p> { + py: Python<'p>, owned: usize, borrowed: usize, pointers: bool, no_send: marker::PhantomData>, } -impl Default for GILPool { +impl<'p> GILPool<'p> { #[inline] - fn default() -> GILPool { + pub fn new(py: Python) -> GILPool { let p: &'static mut ReleasePool = unsafe { &mut *POOL }; GILPool { + py, owned: p.owned.len(), borrowed: p.borrowed.len(), pointers: true, no_send: marker::PhantomData, } } -} - -impl GILPool { #[inline] - pub fn new() -> GILPool { - GILPool::default() - } - #[inline] - pub fn new_no_pointers() -> GILPool { + pub fn new_no_pointers(py: Python) -> GILPool { let p: &'static mut ReleasePool = unsafe { &mut *POOL }; GILPool { + py, owned: p.owned.len(), borrowed: p.borrowed.len(), pointers: false, @@ -213,11 +209,11 @@ impl GILPool { } } -impl Drop for GILPool { +impl<'p> Drop for GILPool<'p> { fn drop(&mut self) { unsafe { let pool: &'static mut ReleasePool = &mut *POOL; - pool.drain(self.owned, self.borrowed, self.pointers); + pool.drain(self.py, self.owned, self.borrowed, self.pointers); } } } @@ -394,7 +390,7 @@ mod test { let p: &'static mut ReleasePool = &mut *POOL; { - let _pool = GILPool::new(); + let _pool = GILPool::new(py); assert_eq!(p.owned.len(), 0); let _ = gil::register_owned(py, obj.into_nonnull()); @@ -402,7 +398,7 @@ mod test { assert_eq!(p.owned.len(), 1); assert_eq!(ffi::Py_REFCNT(obj_ptr), 2); { - let _pool = GILPool::new(); + let _pool = GILPool::new(py); let obj = get_object(); let _ = gil::register_owned(py, obj.into_nonnull()); assert_eq!(p.owned.len(), 2); @@ -463,7 +459,7 @@ mod test { assert_eq!(ffi::Py_REFCNT(obj_ptr), 1); { - let _pool = GILPool::new(); + let _pool = GILPool::new(py); assert_eq!(p.borrowed.len(), 1); gil::register_borrowed(py, NonNull::new(obj_ptr).unwrap()); assert_eq!(p.borrowed.len(), 2); diff --git a/src/type_object.rs b/src/type_object.rs index 878558a7..4cdd2d8d 100644 --- a/src/type_object.rs +++ b/src/type_object.rs @@ -420,8 +420,8 @@ unsafe extern "C" fn tp_dealloc_callback(obj: *mut ffi::PyObject) where T: PyObjectAlloc, { - let _pool = gil::GILPool::new_no_pointers(); let py = Python::assume_gil_acquired(); + let _pool = gil::GILPool::new_no_pointers(py); ::dealloc(py, obj) } fn py_class_flags(type_object: &mut ffi::PyTypeObject) { diff --git a/src/types/dict.rs b/src/types/dict.rs index d677d43f..380b8ae0 100644 --- a/src/types/dict.rs +++ b/src/types/dict.rs @@ -416,7 +416,7 @@ mod test { let cnt; { - let _pool = crate::GILPool::new(); + let _pool = crate::GILPool::new(py); let none = py.None(); cnt = none.get_refcnt(); let _dict = [(10, none)].into_py_dict(py); diff --git a/src/types/iterator.rs b/src/types/iterator.rs index ef36fb1d..0d7ab4ab 100644 --- a/src/types/iterator.rs +++ b/src/types/iterator.rs @@ -140,7 +140,7 @@ mod tests { let none; let count; { - let _pool = GILPool::new(); + let _pool = GILPool::new(py); let l = PyList::empty(py); none = py.None(); l.append(10).unwrap(); @@ -150,7 +150,7 @@ mod tests { } { - let _pool = GILPool::new(); + let _pool = GILPool::new(py); let inst = obj.as_ref(py); let mut it = inst.iter().unwrap(); diff --git a/src/types/list.rs b/src/types/list.rs index 8c6b3a8e..039ef6d4 100644 --- a/src/types/list.rs +++ b/src/types/list.rs @@ -278,7 +278,7 @@ mod test { let cnt; { - let _pool = crate::GILPool::new(); + let _pool = crate::GILPool::new(py); let v = vec![2]; let ob = v.to_object(py); let list = ::try_from(ob.as_ref(py)).unwrap(); @@ -313,7 +313,7 @@ mod test { let cnt; { - let _pool = crate::GILPool::new(); + let _pool = crate::GILPool::new(py); let list = PyList::empty(py); let none = py.None(); cnt = none.get_refcnt(); @@ -342,7 +342,7 @@ mod test { let cnt; { - let _pool = crate::GILPool::new(); + let _pool = crate::GILPool::new(py); let list = PyList::empty(py); let none = py.None(); cnt = none.get_refcnt();