Fix hang in destructor

Consider following situation. A class owns a timer. In destructor of that class we call .cancel() asynchronous on timer before it's destruction.
Now timer is executing cancel in it's own internal thread, while it's doing that destructor of timer is called from owner's destructor. Timer destructor enqueues stop notification. If that enqueue is happening just after while loop from cancel notification, stop notification is gonna be dropped and timer will never stop.
Fix: Add new method in TimedNotificationQueue which will return a notification regardless of the time it needs to be executed.
Get number of pending tasks in the queue. Flush out that many notifications from queue while taking special consideration of pending Stop and Cancel notifications.
Add test for new method in TimedNotificationQueue and fix cancel all tests to actually check if notification got executed.
fixes #3986
This commit is contained in:
Vojin Ilic 2023-04-04 10:54:30 +02:00
parent 687f9fb2f5
commit b8d1792fa0
6 changed files with 69 additions and 10 deletions

View File

@ -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

View File

@ -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 (;;)

View File

@ -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);

View File

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

View File

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

View File

@ -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