Merge pull request #598 from kngwyu/buf-leak

Fix memory leak of PyBytes -> Vec conversion

examples/rustapi_module/requirements-dev.txt

@@ -2,3 +2,4 @@ pip>=19.1
 hypothesis>=3.55
 pytest>=3.5.0
 setuptools-rust>=0.10.2
+psutil>=5.6

examples/rustapi_module/setup.py

@@ -97,6 +97,7 @@ setup(
         make_rust_extension("rustapi_module.datetime"),
         make_rust_extension("rustapi_module.subclassing"),
         make_rust_extension("rustapi_module.test_dict"),
+        make_rust_extension("rustapi_module.buf_and_str"),
     ],
     install_requires=install_requires,
     tests_require=tests_require,

examples/rustapi_module/src/buf_and_str.rs

@@ -0,0 +1,31 @@
+//! Objects related to PyBuffer and PyStr
+use pyo3::prelude::*;
+use pyo3::types::{PyBytes, PyString};
+
+/// This is for confirming that PyBuffer does not cause memory leak
+#[pyclass]
+struct BytesExtractor {}
+
+#[pymethods]
+impl BytesExtractor {
+    #[new]
+    pub fn __new__(obj: &PyRawObject) {
+        obj.init({ BytesExtractor {} });
+    }
+
+    pub fn to_vec(&mut self, bytes: &PyBytes) -> PyResult<usize> {
+        let byte_vec: Vec<u8> = bytes.extract().unwrap();
+        Ok(byte_vec.len())
+    }
+
+    pub fn to_str(&mut self, bytes: &PyString) -> PyResult<usize> {
+        let byte_str: String = bytes.extract().unwrap();
+        Ok(byte_str.len())
+    }
+}
+
+#[pymodule]
+pub fn buf_and_str(_py: Python, m: &PyModule) -> PyResult<()> {
+    m.add_class::<BytesExtractor>()?;
+    Ok(())
+}

examples/rustapi_module/src/lib.rs

@@ -1,3 +1,4 @@
+pub mod buf_and_str;
 pub mod datetime;
 pub mod dict_iter;
 pub mod othermod;

examples/rustapi_module/tests/test_buf_and_str.py

@@ -0,0 +1,38 @@
+import os
+import platform
+import psutil
+import pytest
+from rustapi_module.buf_and_str import BytesExtractor
+
+PYPY = platform.python_implementation() == "PyPy"
+
+
+@pytest.mark.skipif(
+    PYPY,
+    reason="PyPy has a segfault bug around this area."
+    "See https://github.com/PyO3/pyo3/issues/589 for detail.",
+)
+def test_pybuffer_doesnot_leak_memory():
+    N = 1000
+    extractor = BytesExtractor()
+    process = psutil.Process(os.getpid())
+
+    def memory_diff(f):
+        before = process.memory_info().rss
+        f()
+        after = process.memory_info().rss
+        return after - before
+
+    message_b = b'\\(-"-;) Praying that memory leak would not happen..'
+    message_s = '\\(-"-;) Praying that memory leak would not happen..'
+
+    def to_vec():
+        for i in range(N):
+            extractor.to_vec(message_b)
+
+    def to_str():
+        for i in range(N):
+            extractor.to_str(message_s)
+
+    memory_diff(to_vec) == 0
+    memory_diff(to_str) == 0

src/buffer.rs

@@ -582,7 +582,6 @@ impl PyBuffer {
             let ptr = &*self.0 as *const ffi::Py_buffer as *mut ffi::Py_buffer;
             ffi::PyBuffer_Release(ptr)
         };
-        mem::forget(self);
     }
 }
 

tests/ui/reject_generics.stderr

@@ -3,5 +3,3 @@ error: #[pyclass] cannot have generic parameters
   |
 4 | struct ClassWithGenerics<A> {
   |                         ^^^
-
-error: Could not compile `pyo3-tests`.

tests/ui/too_many_args_to_getter.stderr

@@ -3,5 +3,3 @@ error: Getter function can only have one argument of type pyo3::Python!
    |
 11 |     fn get_num(&self, index: u32) {}
    |                              ^^^
-
-error: Could not compile `pyo3-tests`.