Registered type fixes

include/pybind11/attr.h

@@ -216,8 +216,8 @@ struct type_record {
     /// The global operator new can be overridden with a class-specific variant
     void *(*operator_new)(size_t) = ::operator new;
 
-    /// Function pointer to class_<..>::init_holder
-    void (*init_holder)(instance *, const void *) = nullptr;
+    /// Function pointer to class_<..>::init_instance
+    void (*init_instance)(instance *, const void *) = nullptr;
 
     /// Function pointer to class_<..>::dealloc
     void (*dealloc)(const detail::value_and_holder &) = nullptr;

include/pybind11/cast.h

@@ -44,7 +44,7 @@ struct type_info {
     const std::type_info *cpptype;
     size_t type_size, holder_size_in_ptrs;
     void *(*operator_new)(size_t);
-    void (*init_holder)(instance *, const void *);
+    void (*init_instance)(instance *, const void *);
     void (*dealloc)(const value_and_holder &v_h);
     std::vector<PyObject *(*)(PyObject *, PyTypeObject *)> implicit_conversions;
     std::vector<std::pair<const std::type_info *, void *(*)(void *)>> implicit_casts;
@@ -515,7 +515,6 @@ inline PyThreadState *get_thread_state_unchecked() {
 
 // Forward declarations
 inline void keep_alive_impl(handle nurse, handle patient);
-inline void register_instance(instance *self, void *valptr, const type_info *tinfo);
 inline PyObject *make_new_instance(PyTypeObject *type, bool allocate_value = true);
 
 class type_caster_generic {
@@ -595,8 +594,7 @@ class type_caster_generic {
                 throw cast_error("unhandled return_value_policy: should not happen!");
         }
 
-        register_instance(wrapper, valueptr, tinfo);
-        tinfo->init_holder(wrapper, existing_holder);
+        tinfo->init_instance(wrapper, existing_holder);
 
         return inst.release();
     }

include/pybind11/class_support.h

@@ -234,9 +234,8 @@ inline bool deregister_instance(instance *self, void *valptr, const type_info *t
 
 /// Instance creation function for all pybind11 types. It only allocates space for the C++ object
 /// (or multiple objects, for Python-side inheritance from multiple pybind11 types), but doesn't
-/// call the constructor -- an `__init__` function must do that.  If allocating value, the instance
-/// is registered; otherwise register_instance will need to be called once the value has been
-/// assigned.
+/// call the constructor -- an `__init__` function must do that.  `register_instance` will need to
+/// be called after the object has been initialized.
 inline PyObject *make_new_instance(PyTypeObject *type, bool allocate_value /*= true (in cast.h)*/) {
 #if defined(PYPY_VERSION)
     // PyPy gets tp_basicsize wrong (issue 2482) under multiple inheritance when the first inherited
@@ -257,7 +256,6 @@ inline PyObject *make_new_instance(PyTypeObject *type, bool allocate_value /*= t
         for (auto &v_h : values_and_holders(inst)) {
             void *&vptr = v_h.value_ptr();
             vptr = v_h.type->operator_new(v_h.type->type_size);
-            register_instance(inst, vptr, v_h.type);
         }
     }
 
@@ -316,11 +314,12 @@ inline void clear_instance(PyObject *self) {
     // Deallocate any values/holders, if present:
     for (auto &v_h : values_and_holders(instance)) {
         if (v_h) {
+            // This is allowed to fail (the type might have been allocated but not
+            // initialized/registered):
+            deregister_instance(instance, v_h.value_ptr(), v_h.type);
+
             if (instance->owned || v_h.holder_constructed())
                 v_h.type->dealloc(v_h);
-
-            if (!deregister_instance(instance, v_h.value_ptr(), v_h.type))
-                pybind11_fail("pybind11_object_dealloc(): Tried to deallocate unregistered instance!");
         }
     }
     // Deallocate the value/holder layout internals:

include/pybind11/pybind11.h

@@ -714,7 +714,7 @@ class cpp_function : public function {
         } else {
             if (overloads->is_constructor) {
                 auto tinfo = get_type_info((PyTypeObject *) overloads->scope.ptr());
-                tinfo->init_holder(reinterpret_cast<instance *>(parent.ptr()), nullptr);
+                tinfo->init_instance(reinterpret_cast<instance *>(parent.ptr()), nullptr);
             }
             return result.ptr();
         }
@@ -835,7 +835,7 @@ class generic_type : public object {
         tinfo->type_size = rec.type_size;
         tinfo->operator_new = rec.operator_new;
         tinfo->holder_size_in_ptrs = size_in_ptrs(rec.holder_size);
-        tinfo->init_holder = rec.init_holder;
+        tinfo->init_instance = rec.init_instance;
         tinfo->dealloc = rec.dealloc;
         tinfo->simple_type = true;
         tinfo->simple_ancestors = true;
@@ -971,7 +971,7 @@ class class_ : public detail::generic_type {
         record.type = &typeid(type);
         record.type_size = sizeof(conditional_t<has_alias, type_alias, type>);
         record.holder_size = sizeof(holder_type);
-        record.init_holder = init_holder;
+        record.init_instance = init_instance;
         record.dealloc = dealloc;
         record.default_holder = std::is_same<holder_type, std::unique_ptr<type>>::value;
 
@@ -1170,7 +1170,7 @@ class class_ : public detail::generic_type {
 private:
     /// Initialize holder object, variant 1: object derives from enable_shared_from_this
     template <typename T>
-    static void init_holder_helper(detail::instance *inst, detail::value_and_holder &v_h,
+    static void init_holder(detail::instance *inst, detail::value_and_holder &v_h,
             const holder_type * /* unused */, const std::enable_shared_from_this<T> * /* dummy */) {
         try {
             auto sh = std::dynamic_pointer_cast<typename holder_type::element_type>(
@@ -1198,7 +1198,7 @@ class class_ : public detail::generic_type {
     }
 
     /// Initialize holder object, variant 2: try to construct from existing holder object, if possible
-    static void init_holder_helper(detail::instance *inst, detail::value_and_holder &v_h,
+    static void init_holder(detail::instance *inst, detail::value_and_holder &v_h,
             const holder_type *holder_ptr, const void * /* dummy -- not enable_shared_from_this<T>) */) {
         if (holder_ptr) {
             init_holder_from_existing(v_h, holder_ptr, std::is_copy_constructible<holder_type>());
@@ -1209,10 +1209,14 @@ class class_ : public detail::generic_type {
         }
     }
 
-    /// Initialize holder object of an instance, possibly given a pointer to an existing holder
-    static void init_holder(detail::instance *inst, const void *holder_ptr) {
+    /// Performs instance initialization including constructing a holder and registering the known
+    /// instance.  Should be called as soon as the `type` value_ptr is set for an instance.  Takes an
+    /// optional pointer to an existing holder to use; if not specified and the instance is
+    /// `.owned`, a new holder will be constructed to manage the value pointer.
+    static void init_instance(detail::instance *inst, const void *holder_ptr) {
         auto v_h = inst->get_value_and_holder(detail::get_type_info(typeid(type)));
-        init_holder_helper(inst, v_h, (const holder_type *) holder_ptr, v_h.value_ptr<type>());
+        register_instance(inst, v_h.value_ptr(), v_h.type);
+        init_holder(inst, v_h, (const holder_type *) holder_ptr, v_h.value_ptr<type>());
     }
 
     /// Deallocates an instance; via holder, if constructed; otherwise via operator delete.

tests/test_call_policies.py

@@ -116,7 +116,9 @@ def test_alive_gc_multi_derived(capture):
     from pybind11_tests import Parent, Child, ConstructorStats
 
     class Derived(Parent, Child):
-        pass
+        def __init__(self):
+            Parent.__init__(self)
+            Child.__init__(self)
 
     n_inst = ConstructorStats.detail_reg_inst()
     p = Derived()
@@ -131,6 +133,7 @@ class Derived(Parent, Child):
     assert capture == """
         Releasing parent.
         Releasing child.
+        Releasing child.
     """