Confusion around intrusive reference counting

[Ahajha]

I'm converting a codebase from pybind11 to nanobind. There were some classes that used shared_ptr holder types, so I'm moving those to use intrusive pointers. I think I've done everything the docs say:

• Inherit from nb::intrusive_base
• Call nb::intrusive_init in the module init function
• Add an nb::intrusive_ptr function call within the class bindings
• Include nanobind/intrusive/counter.inl in one compilation unit

I'm running into some segfaults that I suspect might be related to this but I don't know for sure. I can try to make a proper repro if requested, but at a high level we're doing this:

• We have an InferenceSession (intrusively reference counted, previously inherited from std::enable_shared_from_this) and a ModelHandle (previously held an std::shared_ptr<InferenceSession>, now converted to an nb::ref<InferenceSession>)
• InferenceSession::compile returns a ModelHandle, holding a pointer to the InferenceSession used.

Then we call ModelHandle::load, but here we're seeing that some of the members of ModelHandle are weirdly invalid. We're not sure how, but one theory is that ModelHandle has been moved from, but that doesn't seem to be the case since it still compiles if we delete the move and copy constructor and assignment operator.

One specific question that seems odd to me is how, if the move and copy assignment operators and constructors are deleted, does nanobind construct a Python object from an existing object? My best guess is this is something to do with RVO and out-pointers behind the scenes.

Overall though, does this sound like this should work? Or is there something architecturally incorrect?

[wjakob]

Nanobind supports shared pointers just fine, so you don't necessarily need to stop using them. There are mainly two advantages of intrusive pointers: a small performance increase, and leak-free reference counting when bound classes implement C++ <-> Python callbacks.

It's not really clear what the issue could be based on your explanation. The intrusive pointers are in active use in various projects without problems. I suggest that you try to reduce the bug into something self-contained, which may also help to pinpoint the problem.

[Ahajha]

The problem here ended up being in compile, we were using nb::rv_policy::take_ownership (not entirely sure why we were using it before). We removed that and let the default kick in, and it works now.

[Ahajha]

If we hadn't discovered a solution, good to know that we could have gone back to shared pointers.

[wjakob]

Hm, those annotations should not play a role when the class in question supports intrusive reference counting. So maybe there is still a bug. Could you post the signature of the problematic function binding and the (high level) declaration of the classes occurring as parameters of this function?

[Ahajha]

InferenceSession is using intrusive refcounting, ModelHandle (which the problematic function was in) did not. So it makes sense that the return value policy would change something.