-
Notifications
You must be signed in to change notification settings - Fork 12.8k
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Eagerly run TLS destructors to properly handle stack overflows #109858
Conversation
r? @thomcc (rustbot has picked a reviewer for you, use r? to override) |
Hey! It looks like you've submitted a new PR for the library teams! If this PR contains changes to any Examples of
|
This comment has been minimized.
This comment has been minimized.
Resolved it by running the destructors eagerly. |
@rustbot author |
3a37385
to
1b57f85
Compare
This comment has been minimized.
This comment has been minimized.
1b57f85
to
9242a5b
Compare
This comment has been minimized.
This comment has been minimized.
9242a5b
to
900dc41
Compare
@rustbot ready |
The job Click to see the possible cause of the failure (guessed by this bot)
|
@rustbot author |
ping from triage - can you post your status on this PR? There hasn't been an update in a few months. Thanks! FYI: when a PR is ready for review, send a message containing |
I'm still completely lost as to what the issue here might be, but I'll make sure to change the status again once I've found it. Until then I'd like to keep this open if it's alright with you, just so that the tracking issue has an open PR to link to. |
☔ The latest upstream changes (presumably #115753) made this pull request unmergeable. Please resolve the merge conflicts. |
I'm marking this as a draft until #116850 is merged, which should simplify the changes here, while preserving the platform-specific parts. |
I've found a better way to implement this and will put that up after #126953 is merged. |
Fixes #109785 for UNIX systems with native TLS support.
By eagerly running the TLS destructors while the stack overflow handler is still active, stack overflows are now correctly reported. Since the destructors are invoked only after any user code has finished running, there should be no observable differences in behaviour. Also note that TLS destructors still work as expected for threads not created by
std
.This also removes the specialized destructor registration implementations on Linux and macOS. Going through a native TLS key probably brings no noticeable performance degradations, and it simplifies the code considerably.