Same PyObject for a shared_ptr C++ object

Question

I have a similar problem as in this mail. I want that every time I return the same C++ managed object to Python, I get the same PyObject. With the standard converter I get always a new wrapper-PyObject. The solution should fulfill the following requirements:

• get() == get() (the Python object is always equal, whatever function returns the same C++ object)
• get() is get()
• The same is valid for any objects returned in containers (which the solution from the mail lacks) getArray()[0] == get()
• I can instance attributes (get().new_var = "xx" -> get().new_var == "xx")

I wrote a template converter which takes as input a shared_ptr. It looks in a static map, if this shared_ptr is already returned, than it returns the existing PyObject, else a new one is created. I haven't found a solution there I can automatically remove unused objects.

In this case, I added a function to manually remove an entry. This works because the object is managed in C++ and I can intercept than the object should be removed from the C++ side. If Python has still a reference to this object, it's ok to wait for the GC to collect the Python side and implicitly remove the C++ object. If the last reference is removed in Python, the PyObject is still valid, because the map holds still a reference. Also , as long as the PyObject inside the map exists, the C++ object cannot be removed from the heap.

I made a couple of test, which shows the validity, that can be found here.

The usage is:

boost::python::to_python_converter<boost::shared_ptr<A>,
        c_ptr_to_cached_python<A>>();

Is the following code enough to fulfill the requirements for any C++ class wrapped in a shared_ptr? Is the PyObject generation valid and can it be improved?

#pragma once
#include <boost/python.hpp>
#include <map>
#include <iostream>

template<typename C_CLASS>
using storage_map = std::map<C_CLASS*, PyObject*>;

template<typename C_CLASS>
storage_map<C_CLASS>* getCachedObjects() {
    static storage_map<C_CLASS> *storage = new storage_map<C_CLASS>();
    return storage;
}

template<typename C_CLASS>
using c_sptr = boost::shared_ptr<C_CLASS>;

template<typename C_CLASS>
using py_ptr = typename boost::python::pointee<boost::shared_ptr<C_CLASS>>::type;

template<typename C_CLASS>
PyObject * createObject(const c_sptr<C_CLASS>& obj) {
    return boost::python::objects::make_ptr_instance<py_ptr<C_CLASS>,
            boost::python::objects::pointer_holder<c_sptr<C_CLASS>,
                    py_ptr<C_CLASS>>>::execute(obj);
}

template<typename C_CLASS>
void cleanMap() {
    auto map = getCachedObjects<C_CLASS>();
    for (auto it = map->begin(); it != map->end();) {
        auto py_obj = it->second;
        Py_DecRef(py_obj);
        map->erase(it++);
    }
}

template<typename C_CLASS>
void removeObject(const c_sptr<C_CLASS>& obj) {
    auto map = getCachedObjects<C_CLASS>();
    auto raw_ptr = obj.get();
    auto it = map->find(raw_ptr);
    if (it == map->end()) {
        throw std::logic_error("Object was already removed.");
    }
    auto py_obj = it->second;
    Py_DecRef(py_obj);
    map->erase(it);
}

template<typename C_CLASS>
struct c_ptr_to_cached_python {
    static PyObject* convert(const c_sptr<C_CLASS> &obj) {
        auto raw_ptr = obj.get();
        auto map = getCachedObjects<C_CLASS>();
        auto it = map->find(raw_ptr);
        if (it == map->end()) {
            auto py_obj = createObject<C_CLASS>(obj);
            (*map)[raw_ptr] = py_obj;
            Py_INCREF(py_obj); // Increment to store in map
            return py_obj;
        } else {
            auto old_py_obj = it->second;
            Py_INCREF(old_py_obj);
            return old_py_obj;
        }
    }
};

Answer 1

I'm not sure I understand all the premises of this question. My kneejerk reaction was, "Would it help if you could assume that C_CLASS derived from std::enable_shared_from_this<C_CLASS>? Is it in your power to make that an invariant in your codebase?" If so, I would even go a step further and ask whether you should just write an enable_PyObject_from_this<C_CLASS> along the same lines.

---

Anyway, I see a bit of reducible complexity in your code, I think.

template<typename C_CLASS>
using py_ptr = typename boost::python::pointee<boost::shared_ptr<C_CLASS>>::type;

Isn't this just a really complicated way of saying

template<typename T> using py_ptr = T;

(since pointee<ptr<T>>::type is ptr<T>::element_type is T by definition)? And then you use it in contexts like

template<typename C_CLASS>
PyObject * createObject(const c_sptr<C_CLASS>& obj) {
    return boost::python::objects::make_ptr_instance<py_ptr<C_CLASS>,
            boost::python::objects::pointer_holder<c_sptr<C_CLASS>,
                    py_ptr<C_CLASS>>>::execute(obj);
}

which ought to be more like

template<typename T>
PyObject *createObject(const std::shared_ptr<T>& obj) {
    using namespace boost::python::objects;
    return make_ptr_instance<T, pointer_holder<shared_ptr<T>, T>>::execute(obj);
}

Even that looks more complicated than it feels like it ought to be, but I'm not finding much documentation about what make_ptr_instance is supposed to do (or for that matter why the second template type parameter to pointer_holder isn't defaulted).

---

I see you use Py_INCREF (all caps) but Py_DecRef (camel case). This suggests to the reader that something really subtle (and thus dangerous) is happening here — or else of course you'd be consistently using Py_IncRef/DecRef or Py_INCREF/DECREF (both of which are provided by the underlying library). But is the reader's inference correct? Are you really doing something subtle and dangerous? Or are you just mixing styles because you felt like it?

---

Are you at all worried that getCachedObjects<Base>() returns a completely different map from getCachedObjects<Derived>(), even though I might have a std::shared_ptr<Base> b and a std::shared_ptr<Derived> d such that b == d?