[PATCH 03/12] test: object-delete: Test deferred delete just before thread stops

[Milan Zamazal]

Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:

> On Mon, Jan 22, 2024 at 08:22:10PM +0100, Milan Zamazal wrote:
>> Laurent Pinchart <laurent.pinchart at ideasonboard.com> writes:
>> 
>
>> > The Object::deleterLater() function is expected to not race with
>>                    ^^^
>> deleteLater
>> 
>> > stopping the thread the object is bound to. Add a test for this.
>> >
>> > The test currently fails, demonstrating a bug in libcamera.
>> 
>> Shouldn't it then be marked as an expected failure?
>
> I don't think so. should_fail in meson indicates that the test
> executable returning an error (including terminating due to an uncaught
> signal, such as SIGSEGV) indicates success. In this case, the test
> should pass, but a there's bug in libcamera that makes it fail. Once the
> bug gets fixed, the test will then pass.

My point was whether it is OK to have a commit with a failing test, whether it
can disturb anything.  Right, marking it as an expected failure would be wrong,
but maybe disabling it's automatic run and enabling it in the fix commit would
be a better idea?

>> > Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

Anyway, I don't have anything else to add, so:

Reviewed-by: Milan Zamazal <mzamazal at redhat.com>

>> > ---
>> >  test/object-delete.cpp | 30 +++++++++++++++++++++++++-----
>> >  1 file changed, 25 insertions(+), 5 deletions(-)
>> >
>> > diff --git a/test/object-delete.cpp b/test/object-delete.cpp
>> > index eabefe935974..80b7dc41cd37 100644
>> > --- a/test/object-delete.cpp
>> > +++ b/test/object-delete.cpp
>> > @@ -33,10 +33,10 @@ public:
>> >  	unsigned int *deleteCount_;
>> >  };
>> >  
>> > -class NewThread : public Thread
>> > +class DeleterThread : public Thread
>> >  {
>> >  public:
>> > -	NewThread(Object *obj)
>> > +	DeleterThread(Object *obj)
>> >  		: object_(obj)
>> >  	{
>> >  	}
>> > @@ -63,9 +63,9 @@ protected:
>> >  		unsigned int count = 0;
>> >  		TestObject *obj = new TestObject(&count);
>> >  
>> > -		NewThread thread(obj);
>> > -		thread.start();
>> > -		thread.wait();
>> > +		DeleterThread delThread(obj);
>> > +		delThread.start();
>> > +		delThread.wait();
>> >  
>> >  		Thread::current()->dispatchMessages(Message::Type::DeferredDelete);
>> >  
>> > @@ -89,6 +89,26 @@ protected:
>> >  			return TestFail;
>> >  		}
>> >  
>> > +		/*
>> > +		 * Test that deleteLater() works properly when called just
>> > +		 * before the object's thread exits.
>> > +		 */
>> > +		Thread boundThread;
>> > +		boundThread.start();
>> > +
>> > +		count = 0;
>> > +		obj = new TestObject(&count);
>> > +		obj->moveToThread(&boundThread);
>> > +
>> > +		obj->deleteLater();
>> > +		boundThread.exit();
>> > +		boundThread.wait();
>> > +
>> > +		if (count != 1) {
>> > +			cout << "Object deletion right before thread exit failed (" << count << ")" << endl;
>> > +			return TestFail;
>> > +		}
>> > +
>> >  		return TestPass;
>> >  	}
>> >  };