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

[Laurent Pinchart]

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).
		 */

> > +		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;
> >   	}
> >   

-- 
Regards,

Laurent Pinchart