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

[Tomi Valkeinen]

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.

  Tomi