Skip to content

Revert "Fix dead lock on Timer destructor (#3987)"#3994

Merged
aleks-f merged 1 commit intopocoproject:develfrom
vojinilic:revertTimerFix
Apr 4, 2023
Merged

Revert "Fix dead lock on Timer destructor (#3987)"#3994
aleks-f merged 1 commit intopocoproject:develfrom
vojinilic:revertTimerFix

Conversation

@vojinilic
Copy link
Copy Markdown
Contributor

@vojinilic vojinilic commented Apr 2, 2023

This reverts commit 39a8b9a.
It actually brakes cancel since dequeueNotification only returns notification which is due to execute now.
And there is no test which checks if notifications is executed after cancel.

fixes #3986

@aleks-f
Copy link
Copy Markdown
Member

aleks-f commented Apr 2, 2023

The original fix actually made sense because I've experienced some problems in the past that could be because of that; it seems that what is missing here is a way to shut down the notification queue to prevent it from accepting further notifications. Or return false at the end of CancelNotification::execute()

In any case, if you want to revert this, send the 1.12.5 revert pull also. I would prefer it being properly fixed, but I don't have time for it myself.

@vojinilic
Copy link
Copy Markdown
Contributor Author

vojinilic commented Apr 2, 2023 via email

@aleks-f aleks-f merged commit 687f9fb into pocoproject:devel Apr 4, 2023
@aleks-f
Copy link
Copy Markdown
Member

aleks-f commented Apr 4, 2023

@vojinilic make a new pull with your proposal and we'll look into it. I know testing is often not easy with async and timed stuff, but having tests would definitely be beneficial.

@vojinilic vojinilic deleted the revertTimerFix branch April 4, 2023 08:44
aleks-f pushed a commit that referenced this pull request Nov 27, 2023
This reverts commit 39a8b9a.

Co-authored-by: Vojin Ilic <vilic@nvidia.com>
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