| Message ID | 20191127084909.10612-2-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Commit | d9dac46e6f7c1bfbb76daa016fd7aaa580026d70 |
| Headers | show |
| Series |
|
| Related | show |
Hi Laurent, Thanks for your patch. On 2019-11-27 10:49:06 +0200, Laurent Pinchart wrote: > There's a race in the message delivery against object deletion. Add a > test that triggers it reliably. This test is expected to fail with an > assertion error. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> I like how natural this test feels which means the API must be good right ;-) Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > test/message.cpp | 32 ++++++++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/test/message.cpp b/test/message.cpp > index cf21d5ca50d1..7ebedb557502 100644 > --- a/test/message.cpp > +++ b/test/message.cpp > @@ -52,6 +52,25 @@ private: > Status status_; > }; > > +class SlowMessageReceiver : public Object > +{ > +protected: > + void message(Message *msg) > + { > + if (msg->type() != Message::None) { > + Object::message(msg); > + return; > + } > + > + /* > + * Don't access any member of the object here (including the > + * vtable) as the object will be deleted by the main thread > + * while we're sleeping. > + */ > + this_thread::sleep_for(chrono::milliseconds(100)); > + } > +}; > + > class MessageTest : public Test > { > protected: > @@ -88,6 +107,19 @@ protected: > break; > } > > + /* > + * Test for races between message delivery and object deletion. > + * Failures result in assertion errors, there is no need for > + * explicit checks. > + */ > + SlowMessageReceiver *slowReceiver = new SlowMessageReceiver(); > + slowReceiver->moveToThread(&thread_); > + slowReceiver->postMessage(utils::make_unique<Message>(Message::None)); > + > + this_thread::sleep_for(chrono::milliseconds(10)); > + > + delete slowReceiver; > + > return TestPass; > } > > -- > Regards, > > Laurent Pinchart > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
diff --git a/test/message.cpp b/test/message.cpp index cf21d5ca50d1..7ebedb557502 100644 --- a/test/message.cpp +++ b/test/message.cpp @@ -52,6 +52,25 @@ private: Status status_; }; +class SlowMessageReceiver : public Object +{ +protected: + void message(Message *msg) + { + if (msg->type() != Message::None) { + Object::message(msg); + return; + } + + /* + * Don't access any member of the object here (including the + * vtable) as the object will be deleted by the main thread + * while we're sleeping. + */ + this_thread::sleep_for(chrono::milliseconds(100)); + } +}; + class MessageTest : public Test { protected: @@ -88,6 +107,19 @@ protected: break; } + /* + * Test for races between message delivery and object deletion. + * Failures result in assertion errors, there is no need for + * explicit checks. + */ + SlowMessageReceiver *slowReceiver = new SlowMessageReceiver(); + slowReceiver->moveToThread(&thread_); + slowReceiver->postMessage(utils::make_unique<Message>(Message::None)); + + this_thread::sleep_for(chrono::milliseconds(10)); + + delete slowReceiver; + return TestPass; }
There's a race in the message delivery against object deletion. Add a test that triggers it reliably. This test is expected to fail with an assertion error. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- test/message.cpp | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+)