[libcamera-devel] [PATCH v3] android: camera_device: Fix crash in calling CameraDevice::close()
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Sep 2 05:11:40 CEST 2021
Hi Jacopo,
On Wed, Sep 01, 2021 at 05:47:08PM +0200, Jacopo Mondi wrote:
> On Wed, Sep 01, 2021 at 03:37:39AM +0900, Hirokazu Honda wrote:
> > The problem is happening because we seem to add a CameraStream
> > associated buffer(depending on the CameraStream::Type) to the Request,
> > in CameraDevice::processCaptureRequest().
> >
> > However, when the camera stops, all the current buffers are marked with
> > FrameMetadata::FrameCancelled and proceed to completion. But the buffer
> > associated with the CameraStream (that was previously added to the
> > request) has now been cleared out with a part of streams_.clear(), even
> > before the camera stop() has been invoked. Any access to those request
> > buffers after they have been cleared, shall result in a crash.
>
> I recall it was painful at the time when we had to deal with flush()
> to get the right ordering, and I had done the same. However, since
> stop() is only called by configureStreams() I removed the
> streams_.clear() there and had issues, so I gave up.
Do you mean you have tried removing streams_.clear() from
configureStreams() while testing this patch and ran into issues, or that
you tried that when implementing support for flush() ?
> But I have this patch gone through cros_camera_test and also CTS seems
> fine. Also looking at the code it seems an harmless change so I wonder
> what was wrong at the time.
>
> As far as I can tell:
>
> Tested-by: Jacopo Mondi <jacopo at jmondi.org>
> Acked-by: Jacopo Mondi <jacopo at jmondi.org>
>
> > Signed-off-by: Hirokazu Honda <hiroh at chromium.org>
> > ---
> > src/android/camera_device.cpp | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 8ca76719..fda77db4 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -423,8 +423,6 @@ int CameraDevice::open(const hw_module_t *hardwareModule)
> >
> > void CameraDevice::close()
> > {
> > - streams_.clear();
> > -
> > stop();
> >
> > camera_->release();
> > @@ -457,6 +455,8 @@ void CameraDevice::stop()
> > camera_->stop();
> >
> > descriptors_.clear();
> > + streams_.clear();
> > +
> > state_ = State::Stopped;
> > }
> >
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list