Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pybind11::array argument leaks reference count of dtype singleton #1858

Closed
pv opened this issue Jul 24, 2019 · 2 comments · Fixed by #1860
Closed

pybind11::array argument leaks reference count of dtype singleton #1858

pv opened this issue Jul 24, 2019 · 2 comments · Fixed by #1860

Comments

@pv
Copy link
Contributor

pv commented Jul 24, 2019

Issue description

Calling a function with a const pybind11::array& argument appears to leak refcounts to some Python object. However, it's not the object passed in nor there is a memory leak, so possibly the refcount of some global singleton is not decremented properly.

Numpy itself has a known refcount leak (numpy/numpy#8503), but only with dtype=object arrays --- which are not involved in the case below. Unless, pybind11 uses numpy object arrays internally?
The leaking refcount seems to be that of a dtype singleton.

Reproducible example code

Python debug build is needed for sys.gettotalrefcount(), but you can see the dtype leak also on normal Python:

set -e
PYTHON=python3.7-dbg

cat <<__EOF__ > refleak.cxx
#include <pybind11/pybind11.h>
#include <pybind11/numpy.h>

/* calling this leaks sys.gettotalrefcount() */
void doit(const pybind11::array& a)
{
    return;
}

PYBIND11_MODULE(refleak, m) {
    m.def("doit", doit);
}
__EOF__

cat <<__EOF__ > run.py
import refleak
import numpy as np
import sys

if not hasattr(sys, 'gettotalrefcount'):
    sys.gettotalrefcount = lambda: float('nan')

x = np.array([1])
dtype = np.dtype(np.float_)

before_dtype = sys.getrefcount(dtype)
before = sys.gettotalrefcount()
for j in range(10000):
    refleak.doit(x)
after = sys.gettotalrefcount()
after_dtype = sys.getrefcount(dtype)

import pybind11, numpy
print("numpy:", numpy.__version__)
print("pybind11:", pybind11.__version__)
print("leaked (dtype):", after_dtype - before_dtype, "refs")
print("leaked (total):", after - before, "refs")
print("OK" if after < before + 5000 else "FAIL")
__EOF__

cat <<__EOF__ > setup.py
from setuptools import Extension, setup
import numpy, pybind11
include_dirs = [numpy.get_include(), pybind11.get_include(True), pybind11.get_include(False)]
ext_modules = [Extension("refleak", ["refleak.cxx"], include_dirs=include_dirs)]
setup(name='foo', ext_modules=ext_modules)
__EOF__

$PYTHON setup.py build_ext -i -g
$PYTHON run.py

gives here (same for pybind11 2.3.0 and 4a3464f; numpy 1.16.4 or current master)

numpy: 1.16.4
pybind11: 2.3.0
leaked (dtype): 10000 refs
leaked (total): 10004 refs
FAIL
@pv
Copy link
Contributor Author

pv commented Jul 24, 2019

Should this be reinterpret_steal?

return reinterpret_borrow<pybind11::dtype>(ptr);

PyArray_DescrFromType returns a new reference. Changing this to steal fixes the refcount leak.

@pv pv changed the title sys.gettotalrefcount() increases with pybind11::array argument pybind11::array argument leaks reference count of dtype singleton Jul 24, 2019
@wjakob
Copy link
Member

wjakob commented Jul 26, 2019

Yes, that sounds plausible. Can you make a PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants