| Message ID | 20191127084909.10612-3-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | e54e9ebff4293c2bcacac4ecb10af8b29480fba9 |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your patch. On 2019-11-27 10:49:07 +0200, Laurent Pinchart wrote: > The Object class stores the number of pending messages that have been > posted for it, while the actual messages are stored in a per-thread list > in the ThreadData class. When dispatching messages, the message is > removed from the list, passed to the receiver, and the number of pending > messages is updated. > > In order to avoid too much contention on the list lock, the lock is > dropped to pass the message to the receiver and then taken again. This > creates a race condition window with Thread::removeMessages(), as the > number of pending messages for a receiver is briefly out of sync with > the pending messages list. The assertion at the end of removeMessages() > thus sometimes fails. > > Fix this by decrementing the pending messages counter before releasing > the lock in Thread::dispatchMessages(). This fixes the slow message > receiver test in MessageTest. > > Fixes: 01b930964acd ("libcamera: thread: Add a messaging passing API") > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/libcamera/thread.cpp | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/libcamera/thread.cpp b/src/libcamera/thread.cpp > index 029a0e8fddd5..18ebd16a7e2f 100644 > --- a/src/libcamera/thread.cpp > +++ b/src/libcamera/thread.cpp > @@ -439,11 +439,11 @@ void Thread::dispatchMessages() > Object *receiver = msg->receiver_; > ASSERT(data_ == receiver->thread()->data_); > > + receiver->pendingMessages_--; > + > locker.unlock(); > receiver->message(msg.get()); > locker.lock(); > - > - receiver->pendingMessages_--; > } > } > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/src/libcamera/thread.cpp b/src/libcamera/thread.cpp index 029a0e8fddd5..18ebd16a7e2f 100644 --- a/src/libcamera/thread.cpp +++ b/src/libcamera/thread.cpp @@ -439,11 +439,11 @@ void Thread::dispatchMessages() Object *receiver = msg->receiver_; ASSERT(data_ == receiver->thread()->data_); + receiver->pendingMessages_--; + locker.unlock(); receiver->message(msg.get()); locker.lock(); - - receiver->pendingMessages_--; } }
The Object class stores the number of pending messages that have been posted for it, while the actual messages are stored in a per-thread list in the ThreadData class. When dispatching messages, the message is removed from the list, passed to the receiver, and the number of pending messages is updated. In order to avoid too much contention on the list lock, the lock is dropped to pass the message to the receiver and then taken again. This creates a race condition window with Thread::removeMessages(), as the number of pending messages for a receiver is briefly out of sync with the pending messages list. The assertion at the end of removeMessages() thus sometimes fails. Fix this by decrementing the pending messages counter before releasing the lock in Thread::dispatchMessages(). This fixes the slow message receiver test in MessageTest. Fixes: 01b930964acd ("libcamera: thread: Add a messaging passing API") Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- src/libcamera/thread.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)