[libcamera-devel] [PATCH] libcamera: base: thread: Prevent messages to thread without an event loop

[Laurent Pinchart]

Ping for review.

On Tue, Jun 28, 2022 at 12:46:27PM +0300, Laurent Pinchart via libcamera-devel wrote:
> On Tue, Jun 28, 2022 at 11:09:21AM +0300, Tomi Valkeinen wrote:
> > On 27/06/2022 19:46, Laurent Pinchart wrote:
> > > A message posted to an object bound to a thread without an event loop
> > > will never get delivered. This indicates a bug in the caller, and
> > > materializes in ways that can be hard to debug, such as a signal never
> > > being delivered. Add an assertion in Thread::postMessage() to catch this
> > > at the source.
> > > 
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > ---
> > >   src/libcamera/base/thread.cpp | 1 +
> > >   1 file changed, 1 insertion(+)
> > > 
> > > diff --git a/src/libcamera/base/thread.cpp b/src/libcamera/base/thread.cpp
> > > index 6bda9d1462f5..6ead1d7c12d0 100644
> > > --- a/src/libcamera/base/thread.cpp
> > > +++ b/src/libcamera/base/thread.cpp
> > > @@ -530,6 +530,7 @@ void Thread::postMessage(std::unique_ptr<Message> msg, Object *receiver)
> > >   	msg->receiver_ = receiver;
> > >   
> > >   	ASSERT(data_ == receiver->thread()->data_);
> > > +	ASSERT(data_ != mainThread.data_);
> > >   
> > >   	MutexLocker locker(data_->messages_.mutex_);
> > >   	data_->messages_.list_.push_back(std::move(msg));
> > > 
> > > base-commit: 27cc0a6b58bcca32071cb6ab96e5ee79c75031e5
> > 
> > So what was the issue with the code I had? The problem is that 
> > CameraManager is a libcamera Object, and was associated with the main 
> > thread...? And a PyCameraManager which doesn't inherit CameraManager is 
> > not an Object and not associated with any thread, so it works?
> 
> That's right. Because PyCameraManager inherited from Object, it
> automatically got support for queued signal delivery, where a signal
> emitted in one thread would be processed in a different thread. This
> mechanism requires the receiving object to be bound to a thread that has
> an event loop managed by libcamera. By default an Object is bound to the
> thread it's created in.
> 
> The CameraManager is created in the Python main thread, and the signal
> is emitted from an internal libcamera thread. The signal will thus be
> queued (the default connection type is Auto, which means that signals
> are delivered directly if the receiver object is bound to the thread
> that the signal is emitted from, or queued otherwise), which queues a
> message for the receiver's thread event loop. As the receiver thread is
> the Python main thread (the application thread from a libcamera point of
> view), which doesn't have a libcamera event loop, the message is never
> processed.
> 
> If the receiver doesn't inherit from the Object class, then the signal
> will be delivered directly, in the emitter's thread. You can achieve the
> same result with an Object receiver by specifying the direct connection
> type when connecting the signal.

-- 
Regards,

Laurent Pinchart