Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

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

Draft
guilt wants to merge1 commit intorust-lang:master
base:master
Choose a base branch
Loading
fromguilt:master
Draft

Add on_log_quit.#2669

guilt wants to merge1 commit intorust-lang:masterfromguilt:master

Conversation

guilt
Copy link

@guiltguilt commentedFeb 17, 2021
edited by rami3l
Loading

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?

mahdiamer reacted with thumbs up emoji
@kinnison
Copy link
Contributor

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 withstable for releases, but it could point the way to warranting investigation into a way to resolve this in a stable release if you are certain it functions.

In the meantime, since this is not mergeable as-is regardless of correctness, I have marked the PR as a draft

@guilt
Copy link
Author

guilt commentedFeb 20, 2021
edited
Loading

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 runstty sane at that point before typing stuff on the Terminal again.

@rbtcollins
Copy link
Contributor

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 :)

mahdiamer reacted with thumbs up emoji

@rbtcollins
Copy link
Contributor

@workingjubilee
Copy link
Member

workingjubilee commentedJun 9, 2021
edited
Loading

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.

That's because it is not. This PR relies on the unstable-internalrt feature which is not meant to be used by ordinary consumers of the Rust standard library which are not directly implementing libraries and drivers for the Rust compiler or instrumenting the compiler and its runtime.

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.

For the above-mentioned reasons, this is extremely unlikely to happen.
Seehttps://doc.rust-lang.org/unstable-book/library-features/rt.html

guilt reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rustup-init leaves user with colored terminal
4 participants
@guilt@kinnison@rbtcollins@workingjubilee

[8]ページ先頭

©2009-2025 Movatter.jp