[libcamera-devel] [PATCH] qcam: Clear the pool of free requests upon stopCapture()

[paul.elder at ideasonboard.com]

On Thu, Nov 12, 2020 at 10:40:09AM +0000, Kieran Bingham wrote:
> Hi Paul,
> 
> On 12/11/2020 05:02, Paul Elder wrote:
> > To enable reusing Request objects, we kept a pool of free Requests. This
> > pool was not cleared upon stopping capture, however, which caused a
> > segfault when switching to another camera. Fix this by clearing the
> > Request pool on stopCapture().
> > 
> > Fixes: c753223ad6b9 ("libcamera, android, cam, gstreamer, qcam, v4l2:
> > Reuse Request")
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> 
> Fixes things for me!
> 
> When this crashed for me, it was hitting code in the Request class, is
> there anything we need to do to be more defensive about this?
> 
> Was that just because we were using a Request object which had been
> free'd or something?

Yeah, that's what's happening. processCapture() adds the request (that's
prepared to be reused) to the request pool, meanwhile its unique_ptr
gets freed at stopCapture() (the request_.clear() below). So of course
MainWindow::queueRequest() picks up a freed request from the request
pool :)

I don't think this is anything that needs to be protected against on
libcamera's side. qcam had a request, freed it, and then tried to reuse
it. All the code that's involved in this chain is contained completely
in qcam.

> Anyway, for this fix :
> 
> Tested-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


Thanks,

Paul
> > ---
> >  src/qcam/main_window.cpp | 1 +
> >  1 file changed, 1 insertion(+)
> > 
> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > index f140313e..2502ecd4 100644
> > --- a/src/qcam/main_window.cpp
> > +++ b/src/qcam/main_window.cpp
> > @@ -579,6 +579,7 @@ void MainWindow::stopCapture()
> >  	mappedBuffers_.clear();
> >  
> >  	requests_.clear();
> > +	freeQueue_.clear();
> >  
> >  	delete allocator_;
> >  
> >