Merge pull request #3999 from vojinilic/fixDeadLockDestructor

Fix hang in destructor

Foundation/include/Poco/TimedNotificationQueue.h

@@ -89,6 +89,16 @@ public:
 		/// assigned to a Notification::Ptr, to avoid potential
 		/// memory management issues.
 
+	Notification* dequeueNextNotification();
+		/// Dequeues the next notification regardless of timestamp.
+		/// Returns 0 (null) if no notification is available.
+		/// The caller gains ownership of the notification and
+		/// is expected to release it when done with it.
+		///
+		/// It is highly recommended that the result is immediately
+		/// assigned to a Notification::Ptr, to avoid potential
+		/// memory management issues.
+
 	Notification* waitDequeueNotification();
 		/// Dequeues the next pending notification.
 		/// If no notification is available, waits for a notification

Foundation/src/TimedNotificationQueue.cpp

@@ -82,6 +82,20 @@ Notification* TimedNotificationQueue::dequeueNotification()
 }
 
 
+Notification* TimedNotificationQueue::dequeueNextNotification()
+{
+	FastMutex::ScopedLock lock(_mutex);
+
+	NfQueue::iterator it = _nfQueue.begin();
+	if (it != _nfQueue.end())
+	{
+		Notification::Ptr pNf = it->second;
+		_nfQueue.erase(it);
+		return pNf.duplicate();
+	}
+	return 0;
+}
+
 Notification* TimedNotificationQueue::waitDequeueNotification()
 {
 	for (;;)

Foundation/testsuite/src/TimedNotificationQueueTest.cpp

@@ -139,6 +139,25 @@ void TimedNotificationQueueTest::testDequeue()
 }
 
 
+void TimedNotificationQueueTest::testDequeueNext()
+{
+	TimedNotificationQueue queue;
+	assertTrue (queue.empty());
+	assertTrue (queue.size() == 0);
+	Notification* pNf = queue.dequeueNextNotification();
+	assertNullPtr(pNf);
+	Timestamp time;
+	time += 100000;
+	queue.enqueueNotification(new Notification, time);
+	assertTrue (!queue.empty());
+	assertTrue (queue.size() == 1);
+	pNf = queue.dequeueNextNotification();
+	assertNotNullPtr(pNf);
+	assertTrue (queue.empty());
+	assertTrue (queue.size() == 0);
+	pNf->release();
+}
+
 void TimedNotificationQueueTest::testWaitDequeue()
 {
 	TimedNotificationQueue queue;
@@ -264,6 +283,7 @@ CppUnit::Test* TimedNotificationQueueTest::suite()
 	CppUnit::TestSuite* pSuite = new CppUnit::TestSuite("TimedNotificationQueueTest");
 
 	CppUnit_addTest(pSuite, TimedNotificationQueueTest, testDequeue);
+	CppUnit_addTest(pSuite, TimedNotificationQueueTest, testDequeueNext);
 	CppUnit_addTest(pSuite, TimedNotificationQueueTest, testWaitDequeue);
 	CppUnit_addTest(pSuite, TimedNotificationQueueTest, testWaitDequeueTimeout);
 

Foundation/testsuite/src/TimedNotificationQueueTest.h

@@ -28,6 +28,7 @@ public:
 	~TimedNotificationQueueTest();
 
 	void testDequeue();
+	void testDequeueNext();
 	void testWaitDequeue();
 	void testWaitDequeueTimeout();
 

Util/src/Timer.cpp

@@ -84,9 +84,15 @@ public:
 	bool execute()
 	{
 		// Check if there's a StopNotification pending.
-		Poco::AutoPtr<TimerNotification> pNf = static_cast<TimerNotification*>(queue().dequeueNotification());
-		while (pNf)
+		int numberOfPendingTasks = queue().size();
+		while (numberOfPendingTasks > 0)
 		{
+			Poco::AutoPtr<TimerNotification> pNf = static_cast<TimerNotification*>(queue().dequeueNextNotification());
+			numberOfPendingTasks--;
+			if (!pNf)
+			{
+				continue;
+			}
 			if (pNf.cast<StopNotification>())
 			{
 				queue().clear();
@@ -98,10 +104,8 @@ public:
 			{
 				pCnf->_finished.set();
 			}
-			pNf = static_cast<TimerNotification*>(queue().dequeueNotification());
 		}
 
-		queue().clear();
 		_finished.set();
 		return true;
 	}

Util/testsuite/src/TimerTest.cpp

@@ -231,11 +231,16 @@ void TimerTest::testCancelAllStop()
 
 		TimerTask::Ptr pTask = new TimerTaskAdapter<TimerTest>(*this, &TimerTest::onTimer);
 
-		timer.scheduleAtFixedRate(pTask, 5000, 5000);
-
-		Poco::Thread::sleep(100);
+		// We are scheduling a timer event in 100ms
+		Timestamp time;
+		time += 100000;
+		timer.schedule(pTask, time);
 
 		timer.cancel(false);
+		// Timer should fire in 100ms and onTimer has 100ms sleep in it.
+		// So we are waiting 2 times that plus a small buffer that to make sure that event was never executed.
+		bool timerExecuted = _event.tryWait(200 + 50);
+		assertFalse (timerExecuted);
 	}
 
 	assertTrue (true); // don't hang
@@ -249,11 +254,16 @@ void TimerTest::testCancelAllWaitStop()
 
 		TimerTask::Ptr pTask = new TimerTaskAdapter<TimerTest>(*this, &TimerTest::onTimer);
 
-		timer.scheduleAtFixedRate(pTask, 5000, 5000);
-
-		Poco::Thread::sleep(100);
+		// We are scheduling a timer event in 100ms
+		Timestamp time;
+		time += 100000;
+		timer.schedule(pTask, time);
 
 		timer.cancel(true);
+		// Timer should fire in 100ms and onTimer has 100ms sleep in it.
+		// So we are waiting 2 times that plus a small buffer that to make sure that event was never executed.
+		bool timerExecuted = _event.tryWait(200 + 50);
+		assertFalse (timerExecuted);
 	}
 
 	assertTrue (true); // don't hang