Don't add duplicate patients

This fixes #1251 (patient vector grows without bounds) for the 2.2.2
branch by checking that the vector doesn't already have the given
patient.

This is a little less elegant than the same fix for `master` (which
changes the patients `vector` to an `unordered_set`), but that requires
an internals layout change, which this approach avoids.
This commit is contained in:
Jason Rhinelander 2018-01-13 18:19:36 -04:00 committed by Wenzel Jakob
parent 20d6d1d457
commit 56c1edb46d
3 changed files with 51 additions and 2 deletions

View File

@ -289,9 +289,13 @@ extern "C" inline int pybind11_object_init(PyObject *self, PyObject *, PyObject
inline void add_patient(PyObject *nurse, PyObject *patient) {
auto &internals = get_internals();
auto instance = reinterpret_cast<detail::instance *>(nurse);
auto &current_patients = internals.patients[nurse];
instance->has_patients = true;
for (auto &p : current_patients)
if (p == patient)
return;
Py_INCREF(patient);
internals.patients[nurse].push_back(patient);
current_patients.push_back(patient);
}
inline void clear_patients(PyObject *self) {

View File

@ -8,6 +8,7 @@
*/
#include "pybind11_tests.h"
#include "constructor_stats.h"
struct CustomGuard {
static bool enabled;
@ -59,6 +60,21 @@ TEST_SUBMODULE(call_policies, m) {
.def("returnNullChildKeepAliveChild", &Parent::returnNullChild, py::keep_alive<1, 0>())
.def("returnNullChildKeepAliveParent", &Parent::returnNullChild, py::keep_alive<0, 1>());
// test_keep_alive_single
m.def("add_patient", [](py::object /*nurse*/, py::object /*patient*/) { }, py::keep_alive<1, 2>());
m.def("get_patients", [](py::object nurse) {
py::list patients;
for (PyObject *p : pybind11::detail::get_internals().patients[nurse.ptr()])
patients.append(py::reinterpret_borrow<py::object>(p));
return patients;
});
m.def("refcount", [](py::handle h) {
#ifdef PYPY_VERSION
ConstructorStats::gc(); // PyPy doesn't update ref counts until GC occurs
#endif
return h.ref_count();
});
#if !defined(PYPY_VERSION)
// test_alive_gc
class ParentGC : public Parent {

View File

@ -1,6 +1,6 @@
import pytest
from pybind11_tests import call_policies as m
from pybind11_tests import ConstructorStats
from pybind11_tests import ConstructorStats, UserType
def test_keep_alive_argument(capture):
@ -69,6 +69,35 @@ def test_keep_alive_return_value(capture):
"""
def test_keep_alive_single():
"""Issue #1251 - patients are stored multiple times when given to the same nurse"""
nurse, p1, p2 = UserType(), UserType(), UserType()
b = m.refcount(nurse)
assert [m.refcount(nurse), m.refcount(p1), m.refcount(p2)] == [b, b, b]
m.add_patient(nurse, p1)
assert m.get_patients(nurse) == [p1, ]
assert [m.refcount(nurse), m.refcount(p1), m.refcount(p2)] == [b, b + 1, b]
m.add_patient(nurse, p1)
assert m.get_patients(nurse) == [p1, ]
assert [m.refcount(nurse), m.refcount(p1), m.refcount(p2)] == [b, b + 1, b]
m.add_patient(nurse, p1)
assert m.get_patients(nurse) == [p1, ]
assert [m.refcount(nurse), m.refcount(p1), m.refcount(p2)] == [b, b + 1, b]
m.add_patient(nurse, p2)
assert m.get_patients(nurse) == [p1, p2]
assert [m.refcount(nurse), m.refcount(p1), m.refcount(p2)] == [b, b + 1, b + 1]
m.add_patient(nurse, p2)
assert m.get_patients(nurse) == [p1, p2]
assert [m.refcount(nurse), m.refcount(p1), m.refcount(p2)] == [b, b + 1, b + 1]
m.add_patient(nurse, p2)
m.add_patient(nurse, p1)
assert m.get_patients(nurse) == [p1, p2]
assert [m.refcount(nurse), m.refcount(p1), m.refcount(p2)] == [b, b + 1, b + 1]
del nurse
assert [m.refcount(p1), m.refcount(p2)] == [b, b]
# https://bitbucket.org/pypy/pypy/issues/2447
@pytest.unsupported_on_pypy
def test_alive_gc(capture):