Skip to content

Fix hang in destructor#3999

Merged
obiltschnig merged 1 commit intopocoproject:develfrom
vojinilic:fixDeadLockDestructor
Apr 20, 2023
Merged

Fix hang in destructor#3999
obiltschnig merged 1 commit intopocoproject:develfrom
vojinilic:fixDeadLockDestructor

Conversation

@vojinilic
Copy link
Copy Markdown
Contributor

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

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 pocoproject#3986
@vojinilic vojinilic force-pushed the fixDeadLockDestructor branch from 52326f7 to b8d1792 Compare April 4, 2023 10:09
@obiltschnig
Copy link
Copy Markdown
Member

looks good to me

@obiltschnig obiltschnig merged commit dd21b48 into pocoproject:devel Apr 20, 2023
@obiltschnig obiltschnig added this to the Release 1.12.5 milestone Apr 20, 2023
@vojinilic vojinilic deleted the fixDeadLockDestructor branch June 2, 2023 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix dead lock on Timer destructor

2 participants