From 56c1edb46d2d02e7c8fe0ad3fac53e2f24f18111 Mon Sep 17 00:00:00 2001
From: Jason Rhinelander <jason@imaginary.ca>
Date: Sat, 13 Jan 2018 18:19:36 -0400
Subject: [PATCH] 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.
---
 include/pybind11/detail/class.h |  6 +++++-
 tests/test_call_policies.cpp    | 16 ++++++++++++++++
 tests/test_call_policies.py     | 31 ++++++++++++++++++++++++++++++-
 3 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/include/pybind11/detail/class.h b/include/pybind11/detail/class.h
index 7a5dd0130..ff06370fa 100644
--- a/include/pybind11/detail/class.h
+++ b/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) {
diff --git a/tests/test_call_policies.cpp b/tests/test_call_policies.cpp
index 8642188f9..81fb1702a 100644
--- a/tests/test_call_policies.cpp
+++ b/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 {
diff --git a/tests/test_call_policies.py b/tests/test_call_policies.py
index 7c835599c..8d64afdb7 100644
--- a/tests/test_call_policies.py
+++ b/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):