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.

Author: jagerman

include/pybind11/detail/class.h

@@ -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) {

tests/test_call_policies.cpp

@@ -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 {

tests/test_call_policies.py

@@ -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):