Main loop: rework timer logic#1308
Conversation
|
Looks good to me. We have to wait for the ci to work again though |
|
You could maybe use an #if to use the new glib function where available |
Wouldn't that be just redundant, given that currently this does the same thing? |
|
In my opinion, using |
|
Could you please rebase to master for the ci? |
This logic was over-complicated and caused multiple bugs. This hopefully remediates to this for good. Fixes dunst-project#1196.
8baa6f3 to
5702095
Compare
|
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #1308 +/- ##
==========================================
- Coverage 65.65% 65.64% -0.01%
==========================================
Files 50 50
Lines 8051 8052 +1
Branches 928 925 -3
==========================================
Hits 5286 5286
- Misses 2765 2766 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
|
Thanks @tobast |
|
Whoops, sorry for not merging the review! I've been pretty busy… |
This logic was over-complicated and caused multiple bugs. This hopefully remediates to this for good. Fixes #1196.
I finally took the time to look at the timer logic of the
runfunction, check what was actually needed, and came up with this simplification. I tried to document it in the function, feedback welcome :)This passes
make testcorrectly:Issue #1196 is also fixed: I tried manually the scenarios I could think of (including the one mentioned in the issue) and could not get a
Source ID xx was not founderror.Note for the future: replacing
g_timeout_addwithg_timeout_add_oncewould enhance robustness, in case a bug causes two timers to be running concurrently — which would double the number of invocations ofrununnecessarily, until no notifications are shown for a time, and could exponentially blow-up if the bug occurs within the bug. However, this function was introduced in GLib 2.74, which is too recent for eg. Debian Bullseye.