Skip to content

Commit

Permalink
Fix dtype string leak
Browse files Browse the repository at this point in the history
`PyArray_DescrConverter_` doesn't steal a reference to the argument,
and so the passed arguments shouldn't be `.release()`d.
  • Loading branch information
jagerman committed Sep 20, 2017
1 parent 0aef642 commit c6a57c1
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 1 deletion.
2 changes: 1 addition & 1 deletion include/pybind11/numpy.h
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ class dtype : public object {
/// This is essentially the same as calling numpy.dtype(args) in Python.
static dtype from_args(object args) {
PyObject *ptr = nullptr;
if (!detail::npy_api::get().PyArray_DescrConverter_(args.release().ptr(), &ptr) || !ptr)
if (!detail::npy_api::get().PyArray_DescrConverter_(args.ptr(), &ptr) || !ptr)
throw error_already_set();
return reinterpret_steal<dtype>(ptr);
}
Expand Down
3 changes: 3 additions & 0 deletions tests/test_numpy_dtypes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -448,4 +448,7 @@ TEST_SUBMODULE(numpy_dtypes, m) {

// test_register_dtype
m.def("register_dtype", []() { PYBIND11_NUMPY_DTYPE(SimpleStruct, bool_, uint_, float_, ldbl_); });

// test_str_leak
m.def("dtype_wrapper", [](py::object d) { return py::dtype::from_args(std::move(d)); });
}
13 changes: 13 additions & 0 deletions tests/test_numpy_dtypes.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,19 @@ def test_register_dtype():
assert 'dtype is already registered' in str(excinfo.value)


@pytest.unsupported_on_pypy
def test_str_leak():
from sys import getrefcount
fmt = "f4"
pytest.gc_collect()
start = getrefcount(fmt)
d = m.dtype_wrapper(fmt)
assert d is np.dtype("f4")
del d
pytest.gc_collect()
assert getrefcount(fmt) == start


@pytest.requires_numpy
def test_compare_buffer_info():
assert all(m.compare_buffer_info())

0 comments on commit c6a57c1

Please sign in to comment.