[libcamera-devel] [RFC v3 1/5] HACK: Camera public destructor

[Laurent Pinchart]

Hi Kieran,

On Thu, Dec 09, 2021 at 10:34:51AM +0000, Kieran Bingham wrote:
> Quoting Tomi Valkeinen (2021-12-09 10:22:16)
> > On 09/12/2021 12:05, Kieran Bingham wrote:
> > > Quoting Tomi Valkeinen (2021-12-09 09:29:02)
> > >> pybind11 needs a public destructor for Camera to be able to manage the
> > >> shared_ptr. It's not clear why this is needed, and can it be fixed in
> > >> pybind11.
> > > 
> > > :-( The 'HACK' here seems to be the only thing that would count as a
> > > blocker to integration.
> > > 
> > > I wonder if this is really a HACK or if we just need to explain that the
> > > destructor needs to be public to support the python bindings, but I
> > > myself don't understand the reasoning itself.
> > > 
> > > Is the python code the only instance where we create a shared_ptr for
> > > the Camera? If that's true - then perhaps this patch should actually be
> > > squashed with the one that returns the Camera as a shared_ptr...
> > 
> > This is not related to the patch 2.
> > 
> > pybind11 needs a holder type that is used to manage the references to 
> > the C++ instances. By default it's unique_ptr, but can be defined. We 
> > use shared_ptr for many classes 
> > (https://pybind11.readthedocs.io/en/stable/advanced/smart_ptrs.html?highlight=shared_ptr#std-shared-ptr).
> > 
> > I think normally it makes sense that the destructor must be public: if 
> > pybind11 wrapper code can create the instances, it can also destruct 
> > them. Here we don't create Cameras, nor destruct them, but the related 
> > code is still created, causing the build error.
> > 
> > I believe it should be possible to fix this in pybind11, by making the 
> > template code to detect these kind of classes and skip the destructor 
> > code. Maybe. Somehow. I have no idea how =).
> 
> But how could a shared_ptr skip calling the destructor? Isn't the
> purpose of a shared_ptr to keep the object 'alive' until the last user
> is removed? Therefore the shared_ptr has to be able to destruct the
> object?
> 
> Is there some magic that means the destructor can still be private for a
> shared_ptr in that instance?
> 
> A quick google on shared_ptr private destructor brings up:
> 	https://stackoverflow.com/a/8202667
> 
> Saying it can be solved by making a custom deleter for the shared
> pointer. Is that what we're missing perhaps?

We already have a deleter, see Camera::create().

-- 
Regards,

Laurent Pinchart