Backport #281

Author: konstin

CHANGELOG.md

@@ -4,7 +4,17 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
-## Unreleased
+## [0.5.2] - 2018-11-26
+
+### Fixed
+
+ * Fix undeterministic segfaults when creating many objects by kngwyu in [#281](https://github.com/PyO3/pyo3/pull/281)
+
+## 0.5.1 - 2018-11-24
+
+Yanked
+
+## [0.5.0] - 2018-11-11
 
 ### Added
 
@@ -20,6 +30,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
  * Slowly removing specialization uses
  * `PyString`, `PyUnicode`, and `PyBytes` no longer have a `data()` method
  (replaced by `as_bytes()`) and `PyStringData` has been removed.
+ * The pyobject_extract macro
 
 ### Changed
  * Removes the types from the root module and the prelude. They now live in `pyo3::types` instead.
@@ -31,18 +42,16 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
  * Updated to syn 0.15
  * Splitted `PyTypeObject` into `PyTypeObject` without the create method and `PyTypeCreate` with requires `PyObjectAlloc<Self> + PyTypeInfo + Sized`.
  * Ran `cargo edition --fix` which prefixed path with `crate::` for rust 2018
- * Renamed `async` to `pyasync` as async will be a keyword in the 2018 edition.  
+ * Renamed `async` to `pyasync` as async will be a keyword in the 2018 edition.
+ * Starting to use `NonNull<*mut PyObject>` for Py and PyObject by ijl [#260](https://github.com/PyO3/pyo3/pull/260)
 
 ### Fixed
 
  * Added an explanation that the GIL can temporarily be released even while holding a GILGuard.
  * Lots of clippy errors
  * Fix segfault on calling an unknown method on a PyObject
  * Work around a [bug](https://github.com/rust-lang/rust/issues/55380) in the rust compiler by kngwyu [#252](https://github.com/PyO3/pyo3/pull/252)
-
-### Removed
-
- * The pyobject_extract macro
+ * Fixed a segfault with subclassing pyo3 create classes and using `__class__` by kngwyu [#263](https://github.com/PyO3/pyo3/pull/263)
 
 ## [0.4.1] - 2018-08-20
 
@@ -187,6 +196,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 
 * Initial release
 
+[0.5.2]: https://github.com/pyo3/pyo3/compare/v0.5.0...v0.5.2
+[0.5.0]: https://github.com/pyo3/pyo3/compare/v0.4.1...v0.5.0
 [0.4.1]: https://github.com/pyo3/pyo3/compare/v0.4.0...v0.4.1
 [0.4.0]: https://github.com/pyo3/pyo3/compare/v0.3.2...v0.4.0
 [0.3.2]: https://github.com/pyo3/pyo3/compare/v0.3.1...v0.3.2

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "pyo3"
-version = "0.5.0-alpha.3"
+version = "0.5.2"
 description = "Bindings to Python interpreter"
 authors = ["PyO3 Project and Contributors <https://github.com/PyO3>"]
 readme = "README.md"
@@ -22,7 +22,7 @@ codecov = { repository = "PyO3/pyo3", branch = "master", service = "github" }
 libc = "0.2.43"
 spin = "0.4.9"
 num-traits = "0.2.6"
-pyo3cls = { path = "pyo3cls", version = "=0.5.0-alpha.3" }
+pyo3cls = { path = "pyo3cls", version = "=0.5.2" }
 mashup = "0.1.9"
 num-complex = { version = "0.2.1", optional = true }
 

benches/bench_dict.rs

@@ -0,0 +1,21 @@
+#![feature(test)]
+extern crate pyo3;
+extern crate test;
+use test::Bencher;
+
+use pyo3::{prelude::*, types::IntoPyDict};
+
+#[bench]
+fn iter_dict(b: &mut Bencher) {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+    const LEN: usize = 1_000_00;
+    let dict = (0..LEN as u64).map(|i| (i, i * 2)).into_py_dict(py);
+    let mut sum = 0;
+    b.iter(|| {
+        for (k, _v) in dict.iter() {
+            let i: u64 = k.extract().unwrap();
+            sum += i;
+        }
+    });
+}

pyo3-derive-backend/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "pyo3-derive-backend"
-version = "0.5.0-alpha.3"
+version = "0.5.2"
 description = "Code generation for PyO3 package"
 authors = ["PyO3 Project and Contributors <https://github.com/PyO3>"]
 keywords = ["pyo3", "python", "cpython", "ffi"]

pyo3cls/Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "pyo3cls"
-version = "0.5.0-alpha.3"
+version = "0.5.2"
 description = "Proc macros for PyO3 package"
 authors = ["PyO3 Project and Contributors <https://github.com/PyO3>"]
 keywords = ["pyo3", "python", "cpython", "ffi"]
@@ -16,4 +16,4 @@ proc-macro = true
 quote= "0.6.9"
 proc-macro2 = "0.4.20"
 syn = { version = "0.15.15", features = ["full", "extra-traits"] }
-pyo3-derive-backend = { path = "../pyo3-derive-backend", version = "=0.5.0-alpha.3" }
+pyo3-derive-backend = { path = "../pyo3-derive-backend", version = "=0.5.2" }

src/instance.rs

@@ -298,7 +298,7 @@ impl<T> PartialEq for Py<T> {
 impl<T> Drop for Py<T> {
     fn drop(&mut self) {
         unsafe {
-            pythonrun::register_pointer(self.0);
+            pythonrun::register_pointer(self.0.as_ptr());
         }
     }
 }

src/object.rs

@@ -316,7 +316,7 @@ impl<'a> FromPyObject<'a> for PyObject {
 impl Drop for PyObject {
     fn drop(&mut self) {
         unsafe {
-            pythonrun::register_pointer(self.0);
+            pythonrun::register_pointer(self.0.as_ptr());
         }
     }
 }

src/pythonrun.rs

@@ -1,6 +1,6 @@
 // Copyright (c) 2017-present PyO3 Project and Contributors
 use crate::ffi;
-use crate::python::{NonNullPyObject, Python};
+use crate::python::Python;
 use crate::types::PyObjectRef;
 use spin;
 use std::{any, marker, rc, sync};
@@ -107,8 +107,8 @@ impl Drop for GILGuard {
 
 /// Release pool
 struct ReleasePool {
-    owned: Vec<*mut ffi::PyObject>,
-    borrowed: Vec<*mut ffi::PyObject>,
+    owned: ArrayList<*mut ffi::PyObject>,
+    borrowed: ArrayList<*mut ffi::PyObject>,
     pointers: *mut Vec<*mut ffi::PyObject>,
     obj: Vec<Box<any::Any>>,
     p: spin::Mutex<*mut Vec<*mut ffi::PyObject>>,
@@ -117,8 +117,8 @@ struct ReleasePool {
 impl ReleasePool {
     fn new() -> ReleasePool {
         ReleasePool {
-            owned: Vec::with_capacity(256),
-            borrowed: Vec::with_capacity(256),
+            owned: ArrayList::new(),
+            borrowed: ArrayList::new(),
             pointers: Box::into_raw(Box::new(Vec::with_capacity(256))),
             obj: Vec::with_capacity(8),
             p: spin::Mutex::new(Box::into_raw(Box::new(Vec::with_capacity(256)))),
@@ -127,39 +127,30 @@ impl ReleasePool {
 
     unsafe fn release_pointers(&mut self) {
         let mut v = self.p.lock();
-
-        // vec of pointers
-        let ptr = *v;
-        let vec: &'static mut Vec<*mut ffi::PyObject> = &mut *ptr;
+        let vec = &mut **v;
         if vec.is_empty() {
             return;
         }
 
         // switch vectors
-        *v = self.pointers;
-        self.pointers = ptr;
+        std::mem::swap(&mut self.pointers, &mut *v);
         drop(v);
 
-        // release py objects
+        // release PyObjects
         for ptr in vec.iter_mut() {
             ffi::Py_DECREF(*ptr);
         }
         vec.set_len(0);
     }
 
     pub unsafe fn drain(&mut self, owned: usize, borrowed: usize, pointers: bool) {
-        let len = self.owned.len();
-        if owned < len {
-            for ptr in &mut self.owned[owned..len] {
-                ffi::Py_DECREF(*ptr);
-            }
-            self.owned.set_len(owned);
-        }
-
-        let len = self.borrowed.len();
-        if borrowed < len {
-            self.borrowed.set_len(borrowed);
+        // Release owned objects(call decref)
+        while owned < self.owned.len() {
+            let last = self.owned.pop_back().unwrap();
+            ffi::Py_DECREF(last);
         }
+        // Release borrowed objects(don't call decref)
+        self.borrowed.truncate(borrowed);
 
         if pointers {
             self.release_pointers();
@@ -230,24 +221,19 @@ pub unsafe fn register_any<'p, T: 'static>(obj: T) -> &'p T {
         .unwrap()
 }
 
-pub unsafe fn register_pointer(obj: NonNullPyObject) {
-    let pool: &'static mut ReleasePool = &mut *POOL;
-
-    let mut v = pool.p.lock();
-    let pool: &'static mut Vec<*mut ffi::PyObject> = &mut *(*v);
-    pool.push(obj.as_ptr());
+pub unsafe fn register_pointer(obj: *mut ffi::PyObject) {
+    let pool = &mut *POOL;
+    (**pool.p.lock()).push(obj);
 }
 
 pub unsafe fn register_owned(_py: Python, obj: *mut ffi::PyObject) -> &PyObjectRef {
-    let pool: &'static mut ReleasePool = &mut *POOL;
-    pool.owned.push(obj);
-    &*(&pool.owned[pool.owned.len() - 1] as *const *mut ffi::PyObject as *const PyObjectRef)
+    let pool = &mut *POOL;
+    &*(pool.owned.push_back(obj) as *const _ as *const PyObjectRef)
 }
 
 pub unsafe fn register_borrowed(_py: Python, obj: *mut ffi::PyObject) -> &PyObjectRef {
-    let pool: &'static mut ReleasePool = &mut *POOL;
-    pool.borrowed.push(obj);
-    &*(&pool.borrowed[pool.borrowed.len() - 1] as *const *mut ffi::PyObject as *const PyObjectRef)
+    let pool = &mut *POOL;
+    &*(pool.borrowed.push_back(obj) as *const _ as *const PyObjectRef)
 }
 
 impl GILGuard {
@@ -277,6 +263,64 @@ impl GILGuard {
     }
 }
 
+use self::array_list::ArrayList;
+
+mod array_list {
+    use std::collections::LinkedList;
+    use std::mem;
+
+    const BLOCK_SIZE: usize = 256;
+
+    /// A container type for Release Pool
+    /// See #271 for why this is crated
+    pub(super) struct ArrayList<T> {
+        inner: LinkedList<[T; BLOCK_SIZE]>,
+        length: usize,
+    }
+
+    impl<T: Clone> ArrayList<T> {
+        pub fn new() -> Self {
+            ArrayList {
+                inner: LinkedList::new(),
+                length: 0,
+            }
+        }
+        pub fn push_back(&mut self, item: T) -> &T {
+            let next_idx = self.next_idx();
+            if next_idx == 0 {
+                self.inner.push_back(unsafe { mem::uninitialized() });
+            }
+            self.inner.back_mut().unwrap()[next_idx] = item;
+            self.length += 1;
+            &self.inner.back().unwrap()[next_idx]
+        }
+        pub fn pop_back(&mut self) -> Option<T> {
+            self.length -= 1;
+            let current_idx = self.next_idx();
+            if self.length >= BLOCK_SIZE && current_idx == 0 {
+                let last_list = self.inner.pop_back()?;
+                return Some(last_list[0].clone());
+            }
+            self.inner.back().map(|arr| arr[current_idx].clone())
+        }
+        pub fn len(&self) -> usize {
+            self.length
+        }
+        pub fn truncate(&mut self, new_len: usize) {
+            if self.length <= new_len {
+                return;
+            }
+            while self.inner.len() > (new_len + BLOCK_SIZE - 1) / BLOCK_SIZE {
+                self.inner.pop_back();
+            }
+            self.length = new_len;
+        }
+        fn next_idx(&self) -> usize {
+            self.length % BLOCK_SIZE
+        }
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::{GILPool, ReleasePool, POOL};

tests/test_dict_iter.rs

@@ -0,0 +1,17 @@
+extern crate pyo3;
+
+use pyo3::{prelude::*, types::IntoPyDict};
+
+#[test]
+fn iter_dict_nosegv() {
+    let gil = Python::acquire_gil();
+    let py = gil.python();
+    const LEN: usize = 10_000_000;
+    let dict = (0..LEN as u64).map(|i| (i, i * 2)).into_py_dict(py);
+    let mut sum = 0;
+    for (k, _v) in dict.iter() {
+        let i: u64 = k.extract().unwrap();
+        sum += i;
+    }
+    assert_eq!(sum, 49999995000000);
+}