fixed GH #2058: Synchronization issue in Poco::Util::Timer at destruction

Util/src/Timer.cpp

@@ -32,18 +32,18 @@ public:
 		_queue(queue)
 	{
 	}
-	
+
 	~TimerNotification()
 	{
 	}
-	
+
 	virtual bool execute() = 0;
-	
+
 	Poco::TimedNotificationQueue& queue()
 	{
 		return _queue;
 	}
-	
+
 private:
 	Poco::TimedNotificationQueue& _queue;
 };
@@ -56,11 +56,11 @@ public:
 		TimerNotification(queue)
 	{
 	}
-	
+
 	~StopNotification()
 	{
 	}
-	
+
 	bool execute()
 	{
 		queue().clear();
@@ -76,23 +76,36 @@ public:
 		TimerNotification(queue)
 	{
 	}
-	
+
 	~CancelNotification()
 	{
 	}
-	
+
 	bool execute()
 	{
+		// Check if there's a StopNotification pending.
+		Poco::AutoPtr<TimerNotification> pNf = static_cast<TimerNotification*>(queue().dequeueNotification());
+		while (pNf)
+		{
+			if (pNf.cast<StopNotification>())
+			{
+				queue().clear();
+				_finished.set();
+				return false;
+			}
+			pNf = static_cast<TimerNotification*>(queue().dequeueNotification());
+		}
+
 		queue().clear();
 		_finished.set();
 		return true;
 	}
-	
+
 	void wait()
 	{
 		_finished.wait();
 	}
-	
+
 private:
 	Poco::Event _finished;
 };
@@ -106,16 +119,16 @@ public:
 		_pTask(pTask)
 	{
 	}
-	
+
 	~TaskNotification()
 	{
 	}
-	
+
 	TimerTask::Ptr task()
 	{
-		return _pTask;	
+		return _pTask;
 	}
-	
+
 	bool execute()
 	{
 		if (!_pTask->isCancelled())
@@ -140,7 +153,7 @@ public:
 		}
 		return true;
 	}
-			
+
 private:
 	TimerTask::Ptr _pTask;
 };
@@ -154,13 +167,13 @@ public:
 		_interval(interval)
 	{
 	}
-	
+
 	~PeriodicTaskNotification()
 	{
 	}
-	
+
 	bool execute()
-	{	
+	{
 		TaskNotification::execute();
 
 		if (!task()->isCancelled())
@@ -172,9 +185,9 @@ public:
 			queue().enqueueNotification(this, nextExecution);
 			duplicate();
 		}
-		return true;		
+		return true;
 	}
-	
+
 private:
 	long _interval;
 };
@@ -189,13 +202,13 @@ public:
 		_nextExecution(clock)
 	{
 	}
-	
+
 	~FixedRateTaskNotification()
 	{
 	}
-	
+
 	bool execute()
-	{	
+	{
 		TaskNotification::execute();
 
 		if (!task()->isCancelled())
@@ -206,9 +219,9 @@ public:
 			queue().enqueueNotification(this, _nextExecution);
 			duplicate();
 		}
-		return true;			
+		return true;
 	}
-	
+
 private:
 	long _interval;
 	Poco::Clock _nextExecution;
@@ -241,7 +254,7 @@ Timer::~Timer()
 	}
 }
 
-	
+
 void Timer::cancel(bool wait)
 {
 	Poco::AutoPtr<CancelNotification> pNf = new CancelNotification(_queue);
@@ -266,7 +279,7 @@ void Timer::schedule(TimerTask::Ptr pTask, Poco::Clock clock)
 	_queue.enqueueNotification(new TaskNotification(_queue, pTask), clock);
 }
 
-	
+
 void Timer::schedule(TimerTask::Ptr pTask, long delay, long interval)
 {
 	Poco::Clock clock;
@@ -288,7 +301,7 @@ void Timer::schedule(TimerTask::Ptr pTask, Poco::Clock clock, long interval)
 	_queue.enqueueNotification(new PeriodicTaskNotification(_queue, pTask, interval), clock);
 }
 
-	
+
 void Timer::scheduleAtFixedRate(TimerTask::Ptr pTask, long delay, long interval)
 {
 	Poco::Clock clock;

Util/testsuite/src/TimerTest.cpp

@@ -197,18 +197,18 @@ void TimerTest::testScheduleAtFixedRate()
 void TimerTest::testCancel()
 {
 	Timer timer;
-	
+
 	Timestamp time;
-	
+
 	TimerTask::Ptr pTask = new TimerTaskAdapter<TimerTest>(*this, &TimerTest::onTimer);
-	
+
 	assert (pTask->lastExecution() == 0);
-	
+
 	timer.scheduleAtFixedRate(pTask, 5000, 5000);
 
 	pTask->cancel();
 	assert (pTask->isCancelled());
-	
+
 	try
 	{
 		timer.scheduleAtFixedRate(pTask, 5000, 5000);
@@ -224,6 +224,42 @@ void TimerTest::testCancel()
 }
 
 
+void TimerTest::testCancelAllStop()
+{
+	{
+		Timer timer;
+
+		TimerTask::Ptr pTask = new TimerTaskAdapter<TimerTest>(*this, &TimerTest::onTimer);
+
+		timer.scheduleAtFixedRate(pTask, 5000, 5000);
+
+		Poco::Thread::sleep(100);
+
+		timer.cancel(false);
+	}
+
+	assert (true); // don't hang
+}
+
+
+void TimerTest::testCancelAllWaitStop()
+{
+	{
+		Timer timer;
+
+		TimerTask::Ptr pTask = new TimerTaskAdapter<TimerTest>(*this, &TimerTest::onTimer);
+
+		timer.scheduleAtFixedRate(pTask, 5000, 5000);
+
+		Poco::Thread::sleep(100);
+
+		timer.cancel(true);
+	}
+
+	assert (true); // don't hang
+}
+
+
 void TimerTest::setUp()
 {
 }
@@ -252,6 +288,8 @@ CppUnit::Test* TimerTest::suite()
 	CppUnit_addTest(pSuite, TimerTest, testScheduleIntervalClock);
 	CppUnit_addTest(pSuite, TimerTest, testScheduleAtFixedRate);
 	CppUnit_addTest(pSuite, TimerTest, testCancel);
+	CppUnit_addTest(pSuite, TimerTest, testCancelAllStop);
+	CppUnit_addTest(pSuite, TimerTest, testCancelAllWaitStop);
 
 	return pSuite;
 }

Util/testsuite/src/TimerTest.h

@@ -33,6 +33,8 @@ public:
 	void testScheduleIntervalTimestamp();
 	void testScheduleIntervalClock();
 	void testCancel();
+	void testCancelAllStop();
+	void testCancelAllWaitStop();
 
 	void setUp();
 	void tearDown();