- Notifications
You must be signed in to change notification settings - Fork963
Add on_log_quit.#2669
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Add on_log_quit.#2669
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Does this demonstrably fix the issue in question? It's relying on a feature flag which means we won't accept it into Rustup because we build with In the meantime, since this is not mergeable as-is regardless of correctness, I have marked the PR as a draft |
guilt commentedFeb 20, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Thanks! I will generally wait for the at_exit to get merged to stable/beta at this point. This isn't a priority for 1.24.x, but the reset handler would have to be called somewhere. To reproduce, I would basically introduce some artificial delays in the log code, press Ctrl-C/SIGKILL and see if it resets correctly. It will need some time from me to report what I find, on macOS/Windows/Linux. I am still unsure how to write a testcase for this, this is a 'noob' issue for me to get acquainted with the code. I am going to try testing this with some signal handling code as well and report what I found. But this isn't an uncommon issue. A lot of commands fail and I usually run |
I don't think at_exit is the right way to tackle this. It's kindof sad Rust is getting at_exit hooks really. But I digress. A simple signal handler on unix family OSes, andSetConsoleCtrlHandler on Windows, can call the term reset function we want - but ideally we'd be integrated into stack unwinding to allow cleaning up of held locks etc - while we should be crash safe, we can be more efficient sometimes if we have cleanup code :) |
Also seehttps://rust-cli.github.io/book/in-depth/signals.html and perhapshttps://doc.rust-lang.org/nomicon/unwinding.html as well |
workingjubilee commentedJun 9, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
That's because it is not. This PR relies on the unstable-internal
For the above-mentioned reasons, this is extremely unlikely to happen. |
Uh oh!
There was an error while loading.Please reload this page.
Request for comments;
Closes#2590
I was unable to find a working at_exit that is publicly available in stable rust today. So I decided to post this and ask what may be the right way to attempt this?