Require the GIL to be held to drain the ReleasePool

This adds a `Python` marker to `GILPool`, to prevent the caller from
misusing it to drain the `ReleasePool` and release Python objects
without the GIL held.

Signed-off-by: Anders Kaseorg <andersk@mit.edu>

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
 

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::<pyo3::types::PyTuple>(_args);
             let _kwargs: Option<&pyo3::types::PyDict> = _py.from_borrowed_ptr_or_opt(_kwargs);
 

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::<pyo3::types::PyTuple>(_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::<pyo3::types::PyTuple>(_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::<pyo3::types::PyTuple>(_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::<pyo3::types::PyTuple>(_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::<pyo3::types::PyTuple>(_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::<pyo3::types::PyTuple>(_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);
 

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::<T>(slf);
             let arg = py.from_borrowed_ptr::<PyAny>(arg);
 

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::<T>(slf);
 
             let result = slf.bf_getbuffer(arg1, arg2).into();

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::<T>(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::<T>(slf);
 
             slf.__clear__();

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::<T>(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::<T>(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::<T>(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::<T>(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::<T>(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::<T>(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::<T>(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::<T>(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::<U>(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);
 

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::<T>(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::<T>(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::<T>(slf);
 
                 let result = if value.is_null() {

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::<PyModule>(module) {
         Ok(m) => m,
         Err(e) => {

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<rc::Rc<()>>,
 }
 
-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 {
+    pub fn new_no_pointers(py: Python) -> GILPool {
-        GILPool::default()
-    }
-    #[inline]
-    pub fn new_no_pointers() -> 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);

src/type_object.rs

@@ -420,8 +420,8 @@ unsafe extern "C" fn tp_dealloc_callback<T>(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);
     <T as PyObjectAlloc>::dealloc(py, obj)
 }
 fn py_class_flags<T: PyTypeInfo>(type_object: &mut ffi::PyTypeObject) {

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);

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();
 

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 = <PyList as PyTryFrom>::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();