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

[Laurent Pinchart]

On Thu, Dec 09, 2021 at 12:54:07PM +0200, Tomi Valkeinen wrote:
> On 09/12/2021 12:34, 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?
> 
> That is what is done in Camera.cpp, Camera::create(). Afaik that is the 
> only place where cameras are instantiated, and it, of course, has access 
> to the destructor.
> 
> The pybind11 template code generates code to manage the wrapped class, 
> and I think that code includes constructing and destructing of the 
> instance. Or maybe only destructing, as otherwise we should get a 
> similar error for the constructor if that is required.

If std::shared_ptr<> causes issue, we could manually implement a
PyCamera (name to be bikeshedded, could be just Camera in a py::
namespace) C++ class that would wrap std::shared_ptr<Camera>.

-- 
Regards,

Laurent Pinchart