add cancellation tokens in stead of channels for shutting down #136
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #136 +/- ##
==========================================
+ Coverage 16.60% 16.64% +0.04%
==========================================
Files 22 22
Lines 1650 1646 -4
==========================================
Hits 274 274
+ Misses 1376 1372 -4 ☔ View full report in Codecov by Sentry. |
|
LGTM! I can not reproduce your hanging problem. My assumption is that there is still an edge case where hangs happen, but this is a good simplification of the code (especially directly encoding the cancellation event in the type system), and I'd be happy to see this merge, if only to simplify the understanding of the underlying problem :) |
|
alright, now I can't reproduce that bug myself either, but there's clearly some kind of issue which presents itself intermittently. More investigation is required, but for now, this needs afew finishing touches and can be merged |
|
Want to acknowledge the good call on @albertotirla 's part to recommend using |
now, in stead of a channel, the task recieves a cancelation token, monitoring that one. Not much of a change from the previous model, but at least the intent is clearer untill all tasks gain the ability to use the cancelation token, the sigint signal handler will have to support both options of shutting down add tokio-utils to workspace dependencies and make tts use cancelation tokens fix clippy warnings and add cancellation token to the at-spi event processor add cancellation tokens to another at-spi processor add cancellation tokens to the input processing handler add cancellation tokens to the screenreader events processor, the one which handles input events fix formatting problems reported by check remove the last use of the shutdown channel from the codebase, the sigint watcher now only cancels that token Update ssip-client-async version Move Receives insated of mutably borrinwg them for the duration of the running of Odilia Update Cargo.lock Cargo format Fix clippy issues Update cargo lock
TTWNO
force-pushed
the
cancelation-tokens
branch
from
c1391ac
to
33b954d
Compare
this introduces a timeout of hardcoded 500 milliseconds to the future which awaits all tasks to complete. If, for example, a task is stuck somewhere and doesn't answer to cancelation tokens, we will exit the program forcefully, with an error code and the error will be displayed before exitting. I believe around 500 milliseconds is enough for all tasks to get unstuck, but this will eventually be changeable within the configuration file. For now though, the odilia is hanging on exit problem should probably be relatively well fixed with this
this works as expected, minimising the use of channels, especially where they are not needed, when shutting down.
furthermore, based on my previous assessments, the tokens method of shutting down is more responsive, producing an imediate shutdown.
warning!
the last commit, which removed the last channel for shutdown purposes in use in the codebase that I could see, introduced the hanging bug again. I used gdb to trace it to somewhere around the
odilia::events::dispatch_wrapperfunction, for no obvious reason.Please don't merge this yet, more review is required. The previous commits except the last one are known to work perfectly, so the change seemns small, but apparently it has long ramifications I can't anticipate, and a cursory look at the affected code doesn't reveal anything of importance.
so then, this should probably be solved with tokio-console, which someone with a tiny bit more vision than me should try and use as well, I tryed, but I couldn't make sense of the interface at all.