Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix hang in destructor #3999

Merged
merged 1 commit into from Apr 20, 2023

Conversation

vojinilic
Copy link
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
@obiltschnig
Copy link
Member

looks good to me

@obiltschnig obiltschnig merged commit dd21b48 into pocoproject:devel Apr 20, 2023
13 of 15 checks passed
@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