Skip to content
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

std::sync::Once: Should Once be UnwindSafe or RefUnwindSafe? #43469

Closed
joshlf opened this issue Jul 25, 2017 · 5 comments · Fixed by #90625
Closed

std::sync::Once: Should Once be UnwindSafe or RefUnwindSafe? #43469

joshlf opened this issue Jul 25, 2017 · 5 comments · Fixed by #90625
Assignees
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@joshlf
Copy link
Contributor

joshlf commented Jul 25, 2017

Currently, the following program will not compile because Once isn't RefUnwindSafe. Is this the desired behavior, or should we consider making it RefUnwindSafe or UnwindSafe?

use std::sync::Once;
use std::panic::catch_unwind;

fn main() {
    let o = Once::new();
    catch_unwind(|| o.call_once(|| panic!("foo")));
}
@joshlf joshlf changed the title std::sync::Once: Should call_once be UnwindSafe or RefUnwindSafe? std::sync::Once: Should Once be UnwindSafe or RefUnwindSafe? Jul 25, 2017
@Mark-Simulacrum Mark-Simulacrum added C-bug Category: This is a bug. I-needs-decision Issue: In need of a decision. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. C-feature-request Category: A feature request, i.e: not implemented / a PR. and removed C-bug Category: This is a bug. labels Jul 26, 2017
@alexcrichton
Copy link
Member

It also won't compile because call_once requires 'static self?

@joshlf
Copy link
Contributor Author

joshlf commented Jul 30, 2017

Ah, right you are. This works:

use std::sync::Once;
use std::panic::catch_unwind;

static o: Once = Once::new();

fn main() {
    catch_unwind(|| o.call_once(|| panic!("foo")));
}

@joshlf joshlf closed this as completed Jul 30, 2017
@lilyball
Copy link
Contributor

Once::call_once() does not require &'static self anymore (tested with rustc 1.55.0). The original issue is still valid. Since Once poisons itself on panic, it should be considered unwind safe.

@lilyball lilyball reopened this Oct 12, 2021
@m-ou-se m-ou-se added E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. and removed I-needs-decision Issue: In need of a decision. labels Oct 27, 2021
@m-ou-se
Copy link
Member

m-ou-se commented Oct 27, 2021

We discussed this very briefly in the libs api meeting. It seems fine to add this impl.

@Milo123459
Copy link
Contributor

@rustbot claim

Going to take a stab at this one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-feature-request Category: A feature request, i.e: not implemented / a PR. E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. E-help-wanted Call for participation: Help is requested to fix this issue. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants