[RFC PATCH v1 08/12] apps: lc-compliance: Remove libevent dependency

[Barnabás Pőcze]

2025. január 10., péntek 11:28 keltezéssel, Barnabás Pőcze <pobrn at protonmail.com> írta:

> 2025. január 10., péntek 1:34 keltezéssel, Laurent Pinchart <laurent.pinchart at ideasonboard.com> írta:
> 
> > On Tue, Jan 07, 2025 at 06:06:04PM +0100, Jacopo Mondi wrote:
> > > On Fri, Dec 20, 2024 at 03:08:37PM +0000, Barnabás Pőcze wrote:
> > > > libevent is only used as a way to notify the main thread from
> > > > the CameraManager thread when the capture should be terminated.
> >
> > True, but will that always be the case ? The current tests are fairly
> > simple, but what will happen when we'll have more complex tests that
> > will need to perform multiple actions through a capture session ? I'm
> > sure we could still handle that manually with condition variables, but I
> > think that will become complex, and it feels like reinventing the wheel.
> >
> 
> The current libevent usage is not quite correct, so it needs to be fixed in any case.
> This patch implements probably the simplest option that works for the current
> requirements. Alternatively, I believe manually triggered events of libevent could
> be used to achieve the same, but that would easily be more code and touch more
> components (e.g. `event_loop.{cpp,h}`). But I'm not trying to suggest that libevent
> should not be used anymore. It could be reintroduced in the future easily in my opinion.
> So which one should it be?

It would appear that `EventLoop::callLater()` might be an option as well.


> 
> 
> Regards,
> Barnabás Pőcze
> 
> 
> > > >
> > > > Not only is libevent not really necessary and instead can be
> > > > replaced with a simple combination of C++ STL parts, the current
> > > > usage is prone to race conditions.
> > > >
> > > > Specifically, the camera's `queueRequest()` might complete the
> > > > request before the main thread could set the `loop_` member
> > > > variable and synchronize with the thread. This is a data race,
> > > > practically resulting in a nullptr or dangling pointer dereference.
> > > >
> > > > This can easily be triggered by inserting a sufficiently large
> > > > timeout before `loop_ = new EventLoop;`:
> > >
> > > To be frank this is kind of hard to trigger, as it can only happens
> > > if the main thread does not execute for longer than the frame
> > > interval. But yes, who knows, if there is a chance for this to happen,
> > > it will happen sooner or later, so it's indeed worth addressing it.
> > >
> > > >
> > > >   [46:43:40.529620351] [671833] DEBUG Request request.cpp:129 Request(4:C:0/1:0)
> > > >   ../src/apps/lc-compliance/helpers/capture.cpp:140:14: runtime error: member call on null pointer of type 'struct EventLoop'
> > > >     0 0x632c3e82f81a in CaptureBalanced::requestComplete(libcamera::Request*) ../src/apps/lc-compliance/helpers/capture.cpp:140
> > > >
> > > > Signed-off-by: Barnabás Pőcze <pobrn at protonmail.com>
> > > > ---
> > > > [...]