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

[Umang Jain]

Hi Laurent,

Thanks for the review.

On 7/27/20 8:35 PM, Laurent Pinchart wrote:
> Hi Umang,
>
> Thank you for the patch.
>
> 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  :-(

>
>> ---
>>   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);
>>   		}
>>