[libcamera-devel] [PATCH] qcam: Fix camera reference leak on hot-unplug

[Umang Jain]

Hi again,

On 7/27/20 9:20 PM, Laurent Pinchart wrote:
> Hi Umang,
>
> On Mon, Jul 27, 2020 at 03:38:00PM +0000, Umang Jain wrote:
>> On 7/27/20 8:35 PM, Laurent Pinchart wrote:
>>> On Mon, Jul 27, 2020 at 12:32:59PM +0000, Umang Jain wrote:
>>>> If the currently streaming camera is hot-unplugged,
>>>> a camera reference was still held by MainWindow::camera_,
>>>> preventing it to be destructed, until qcam window is
>>>> closed. Plug the leak in the hot-unplug handler itself.
>>>>
>>>> Signed-off-by: Umang Jain <email at uajain.com>
>>> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>>
>>> Does this fix all the shared pointer refcount issues you've been chasing
>>> ?
>> The refcount issues are sorted here. I spent extensive time on it (both for hotplug
>> and un-plug refcount).
>>
>> However, the other issue I am chasing (earlier believed was caused by refcount issue)
>> is still present - which primarily deals with event loop and how event-notifiers are processed.
>>
>> I can still trigger ASSERT(iter != notifiers_.end()); in EventDispatcherPoll::processNotifiers(),
>> on hot-unplugging a currently streaming camera. This is probably due to the fact that, when
>> destroying a Camera object, it always destroys V4L2VideoDevice() - which in turn, "delete"s
>> two EventNotifiers in it's destructor() and then processNotifiers() isn't aware about the deletion
>> and still trying to find a EventNotifier with concerned pollfd and fails  :-(
> The issue is that the camera object is meant to be destroyed in the
> thread it belongs to, as explained in a previous e-mail (see [1]). Is
> that something you can try fixing, with the pointers in that e-mail, or
> would you like to discuss it further first ?
>
> [1] https://lists.libcamera.org/pipermail/libcamera-devel/2020-July/010951.html
hmm, I figured that I need to call the destructor via 
Object::invokeMethod() with
connection type ConnectionTypeQueued to ensure the destructor is called 
within
it's own thread. Something on the lines:

void Object::deleteLater()

{

this->invokeMethod(&(typeid(this).name()::customDeletorFunc), 
ConnectionTypeQueued);

}

and

void className::customDeletorFunc()
{

~className();

}

which obviously does not compile for me.


How would one go about calling a destructor(in this case Camera's) from 
it's one
of the parent class(i.e. Object)? I am very cloudy around the syntax 
formation for
this->invokeMethod() above. Any help is appreciated.

>
>>>> ---
>>>>    src/qcam/main_window.cpp | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
>>>> index 7bc1360..c8298ec 100644
>>>> --- a/src/qcam/main_window.cpp
>>>> +++ b/src/qcam/main_window.cpp
>>>> @@ -587,6 +587,8 @@ void MainWindow::processHotplug(HotplugEvent *e)
>>>>    		/* Check if the currently-streaming camera is removed. */
>>>>    		if (camera == camera_.get()) {
>>>>    			toggleCapture(false);
>>>> +			camera_->release();
>>>> +			camera_.reset();
>>>>    			cameraCombo_->setCurrentIndex(0);
>>>>    		}
>>>>