always_construct_holder feature to support intrusively reference-counted types (#561)

* always_construct_holder feature to support intrusively reference-counted types

* added testcase

docs/advanced/smart_ptrs.rst

@@ -123,7 +123,7 @@ Custom smart pointers
 pybind11 supports ``std::unique_ptr`` and ``std::shared_ptr`` right out of the
 box. For any other custom smart pointer, transparent conversions can be enabled
 using a macro invocation similar to the following. It must be declared at the
-level before any binding code:
+top namespace level before any binding code:
 
 .. code-block:: cpp
 
@@ -134,6 +134,19 @@ placeholder name that is used as a template parameter of the second argument.
 Thus, feel free to use any identifier, but use it consistently on both sides;
 also, don't use the name of a type that already exists in your codebase.
 
+The macro also accepts a third optional boolean parameter that is set to false
+by default. Specify
+
+.. code-block:: cpp
+
+    PYBIND11_DECLARE_HOLDER_TYPE(T, SmartPtr<T>, true);
+
+if ``SmartPtr<T>`` can always be initialized from a ``T*`` pointer without the
+risk of inconsistencies (such as multiple independent ``SmartPtr`` instances
+believing that they are the sole owner of the ``T*`` pointer). A common
+situation where ``true`` should be passed is when the ``T`` instances use
+*intrusive* reference counting.
+
 Please take a look at the :ref:`macro_notes` before using this feature.
 
 .. seealso::

include/pybind11/cast.h

@@ -952,10 +952,14 @@ protected:
 template <typename T>
 class type_caster<std::shared_ptr<T>> : public type_caster_holder<T, std::shared_ptr<T>> { };
 
+template <typename T, bool Value = false> struct always_construct_holder { static constexpr bool value = Value; };
+
 /// Create a specialization for custom holder types (silently ignores std::shared_ptr)
-#define PYBIND11_DECLARE_HOLDER_TYPE(type, holder_type) \
+#define PYBIND11_DECLARE_HOLDER_TYPE(type, holder_type, ...) \
     namespace pybind11 { namespace detail { \
     template <typename type> \
+    struct always_construct_holder<holder_type> : always_construct_holder<void, ##__VA_ARGS__>  { }; \
+    template <typename type> \
     class type_caster<holder_type, enable_if_t<!is_shared_ptr<holder_type>::value>> \
         : public type_caster_holder<type, holder_type> { }; \
     }}

include/pybind11/pybind11.h

@@ -1151,7 +1151,7 @@ private:
         if (holder_ptr) {
             new (&inst->holder) holder_type(*holder_ptr);
             inst->holder_constructed = true;
-        } else if (inst->owned) {
+        } else if (inst->owned || detail::always_construct_holder<holder_type>::value) {
             new (&inst->holder) holder_type(inst->value);
             inst->holder_constructed = true;
         }
@@ -1161,7 +1161,7 @@ private:
     template <typename T = holder_type,
               detail::enable_if_t<!std::is_copy_constructible<T>::value, int> = 0>
     static void init_holder_helper(instance_type *inst, const holder_type * /* unused */, const void * /* dummy */) {
-        if (inst->owned) {
+        if (inst->owned || detail::always_construct_holder<holder_type>::value) {
             new (&inst->holder) holder_type(inst->value);
             inst->holder_constructed = true;
         }

tests/test_smart_ptr.cpp

@@ -82,7 +82,11 @@ private:
 };
 
 /// Make pybind aware of the ref-counted wrapper type (s)
-PYBIND11_DECLARE_HOLDER_TYPE(T, ref<T>); // Required for custom holder type
+
+// ref<T> is a wrapper for 'Object' which uses intrusive reference counting
+// It is always possible to construct a ref<T> from an Object* pointer without
+// possible incosistencies, hence the 'true' argument at the end.
+PYBIND11_DECLARE_HOLDER_TYPE(T, ref<T>, true);
 PYBIND11_DECLARE_HOLDER_TYPE(T, std::shared_ptr<T>); // Not required any more for std::shared_ptr,
                                                      // but it should compile without error
 
@@ -125,6 +129,18 @@ test_initializer smart_ptr([](py::module &m) {
     py::class_<MyObject1, ref<MyObject1>>(m, "MyObject1", obj)
         .def(py::init<int>());
 
+    m.def("test_object1_refcounting",
+        []() -> bool {
+            ref<MyObject1> o = new MyObject1(0);
+            bool good = o->getRefCount() == 1;
+            py::object o2 = py::cast(o, py::return_value_policy::reference);
+            // always request (partial) ownership for objects with intrusive
+            // reference counting even when using the 'reference' RVP
+            good &= o->getRefCount() == 2;
+            return good;
+        }
+    );
+
     m.def("make_object_1", &make_object_1);
     m.def("make_object_2", &make_object_2);
     m.def("make_myobject1_1", &make_myobject1_1);

tests/test_smart_ptr.py

@@ -116,6 +116,11 @@ def test_smart_ptr(capture):
     assert cstats.move_assignments == 0
 
 
+def test_smart_ptr_refcounting():
+    from pybind11_tests import test_object1_refcounting
+    assert test_object1_refcounting()
+
+
 def test_unique_nodelete():
     from pybind11_tests import MyObject4
     o = MyObject4(23)