[libcamera-devel] [PATCH] test: threads: Test thread cleanup upon abnormal termination

[Umang Jain]

Hi,

On 10/4/22 7:43 PM, Laurent Pinchart wrote:
> On Tue, Oct 04, 2022 at 07:35:42PM +0530, Umang Jain wrote:
>> On 10/4/22 5:28 AM, Laurent Pinchart via libcamera-devel wrote:
>>> If a thread ends abnormally (that is, without retuning normally from its
>>> run() function, for instance with a direct call to pthread_cancel()),
>>> thread cleanup should still be performed. Add a test to ensure this.
>>>
>>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>> ---
>>>    test/threads.cpp | 44 ++++++++++++++++++++++++++++++++++++++++++++
>>>    1 file changed, 44 insertions(+)
>>>
>>> diff --git a/test/threads.cpp b/test/threads.cpp
>>> index d83b5833998f..7daa76edb00e 100644
>>> --- a/test/threads.cpp
>>> +++ b/test/threads.cpp
>>> @@ -8,7 +8,9 @@
>>>    #include <chrono>
>>>    #include <iostream>
>>>    #include <memory>
>>> +#include <pthread.h>
>>>    #include <thread>
>>> +#include <time.h>
>>>    
>>>    #include <libcamera/base/thread.h>
>>>    
>>> @@ -35,6 +37,32 @@ private:
>>>    	chrono::steady_clock::duration duration_;
>>>    };
>>>    
>>> +class CancelThread : public Thread
>>> +{
>>> +public:
>>> +	CancelThread(bool &cancelled)
>>> +		: cancelled_(cancelled)
>>> +	{
>>> +	}
>>> +
>>> +protected:
>>> +	void run()
>>> +	{
>>> +		cancelled_ = true;
>>> +
>>> +		/* Cancel the thread can call a guaranteed cancellation point. */
> I meant s/can/and/. I'll turn this into
>
> 		/*
> 		 * Cancel the thread can call a guaranteed cancellation point
> 		 *(nanosleep).
> 		 */

Perfect, thanks!
>>> +		pthread_cancel(pthread_self());
>>> +
>>> +		struct timespec req{ 0, 100*000*000 };
>>> +		nanosleep(&req, nullptr);
>> Took a bit of reading to determine that nanosleep() is a cancellation
>> point :)
>> Maybe it deserves a comment as well, upto you.
>>
>> Reviewed-by: Umang Jain <umang.jain at ideasonboard.com>
>>
>>> +
>>> +		cancelled_ = false;
>>> +	}
>>> +
>>> +private:
>>> +	bool &cancelled_;
>>> +};
>>> +
>>>    class ThreadTest : public Test
>>>    {
>>>    protected:
>>> @@ -118,6 +146,22 @@ protected:
>>>    			return TestFail;
>>>    		}
>>>    
>>> +		/* Test thread cleanup upon abnormal termination. */
>>> +		bool cancelled = false;
>>> +		bool finished = false;
>>> +
>>> +		thread = std::make_unique<CancelThread>(cancelled);
>>> +		thread->finished.connect(this, [&finished]() { finished = true; });
>>> +
>>> +		thread->start();
>>> +		thread->exit(0);
>>> +		thread->wait(chrono::milliseconds(1000));
>>> +
>>> +		if (!cancelled || !finished) {
>>> +			cout << "Cleanup failed upon abnormal termination" << endl;
>>> +			return TestFail;
>>> +		}
>>> +
>>>    		return TestPass;
>>>    	}
>>>