| Message ID | 20240121035948.4226-7-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > The slow receiver test verifies there's no race condition between > concurrent message delivery and object deletion. This is no a valid use ^^^^ no > case in the first place, as objects are not allowed to be deleted ... or messaged ... > from a different thread than the one they are bound to. Remove the incorrect > test. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > --- > test/message.cpp | 34 ---------------------------------- > 1 file changed, 34 deletions(-) > > diff --git a/test/message.cpp b/test/message.cpp > index 0e76f323e3b9..a34e0f0b5e10 100644 > --- a/test/message.cpp > +++ b/test/message.cpp > @@ -93,25 +93,6 @@ private: > bool success_; > }; > > -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: > @@ -148,21 +129,6 @@ 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(std::make_unique<Message>(Message::None)); > - > - this_thread::sleep_for(chrono::milliseconds(10)); > - > - delete slowReceiver; > - > - this_thread::sleep_for(chrono::milliseconds(100)); > - > /* > * Test recursive calls to Thread::dispatchMessages(). Messages > * should be delivered correctly, without crashes or memory
On Mon, Jan 22, 2024 at 08:59:42PM +0100, Milan Zamazal wrote: > Laurent Pinchart <laurent.pinchart@ideasonboard.com> writes: > > > The slow receiver test verifies there's no race condition between > > concurrent message delivery and object deletion. This is no a valid use > ^^^^ > no Will fix. > > case in the first place, as objects are not allowed to be deleted > > ... or messaged ... Posting a message is fine, that function is thread-safe. > > from a different thread than the one they are bound to. Remove the incorrect > > test. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > Reviewed-by: Milan Zamazal <mzamazal@redhat.com> > > > --- > > test/message.cpp | 34 ---------------------------------- > > 1 file changed, 34 deletions(-) > > > > diff --git a/test/message.cpp b/test/message.cpp > > index 0e76f323e3b9..a34e0f0b5e10 100644 > > --- a/test/message.cpp > > +++ b/test/message.cpp > > @@ -93,25 +93,6 @@ private: > > bool success_; > > }; > > > > -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: > > @@ -148,21 +129,6 @@ 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(std::make_unique<Message>(Message::None)); > > - > > - this_thread::sleep_for(chrono::milliseconds(10)); > > - > > - delete slowReceiver; > > - > > - this_thread::sleep_for(chrono::milliseconds(100)); > > - > > /* > > * Test recursive calls to Thread::dispatchMessages(). Messages > > * should be delivered correctly, without crashes or memory
diff --git a/test/message.cpp b/test/message.cpp index 0e76f323e3b9..a34e0f0b5e10 100644 --- a/test/message.cpp +++ b/test/message.cpp @@ -93,25 +93,6 @@ private: bool success_; }; -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: @@ -148,21 +129,6 @@ 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(std::make_unique<Message>(Message::None)); - - this_thread::sleep_for(chrono::milliseconds(10)); - - delete slowReceiver; - - this_thread::sleep_for(chrono::milliseconds(100)); - /* * Test recursive calls to Thread::dispatchMessages(). Messages * should be delivered correctly, without crashes or memory
The slow receiver test verifies there's no race condition between concurrent message delivery and object deletion. This is no a valid use case in the first place, as objects are not allowed to be deleted from a different thread than the one they are bound to. Remove the incorrect test. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- test/message.cpp | 34 ---------------------------------- 1 file changed, 34 deletions(-)