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.

Sign up for GitHub

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

Jump to bottom

Tracking issue for std::sync::Once poisoning #33577

Closed
3 tasks
durka opened this issue May 12, 2016 · 15 comments · Fixed by #81745
Closed
3 tasks

Tracking issue for std::sync::Once poisoning #33577

durka opened this issue May 12, 2016 · 15 comments · Fixed by #81745
Labels
A-concurrency Area: Concurrency B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@durka
Copy link
Contributor

durka commented May 12, 2016 •
edited
Loading

This tracks the once_poison feature, which currently covers the following things under std::sync::once:

  • OnceState, a struct describing the poisonedness of a Once
  • OnceState::poisoned, which reveals said poisonedness
  • Once::call_once_force, which is like call_once but ignores poisoning
@durka durka mentioned this issue May 12, 2016
Merged
@alexcrichton alexcrichton added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. B-unstable Blocker: Implemented in the nightly compiler and unstable. labels May 12, 2016
durka added a commit to durka/rust that referenced this issue May 21, 2016
@durka
update tracking issue for once_poison
b9d1e76 
The tracking issue for once_poison was noted as rust-lang#31688 which was closed, so it now points to the new rust-lang#33577.
Manishearth added a commit to Manishearth/rust that referenced this issue May 21, 2016
@Manishearth
Rollup merge of rust-lang#33578 - durka:patch-21, r=alexcrichton
87a2288 
update tracking issue for once_poison

The tracking issue for once_poison was noted as rust-lang#31688 which was closed, so it now points to the new rust-lang#33577.
@brson
Copy link
Contributor

brson commented Dec 29, 2016

This issue is primarily about the call_once_force method. Once implements poisoning, and this method is how you get around a poisoned Once.

@Mark-Simulacrum Mark-Simulacrum added the C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC label Jul 22, 2017
@frewsxcv
Copy link
Member

frewsxcv commented Oct 21, 2017 •
edited
Loading

Anyone have insight why poisoned is not just a method on Once?

EDIT: nevermind

@dtolnay
Copy link
Member

dtolnay commented Nov 19, 2017

This naming seems inconsistent:

Could they both be is_poisoned since that one is stable already? Or is poisoned better enough that we should deprecate and rename the one on Mutex?

@joshlf
Copy link
Contributor

joshlf commented Nov 19, 2017

Relevant: #43448 contains discussion of whether forking a process should cause poisoning of various synchronization primitives. TLDR: An option is to make it so that if you fork while a Once is in use, thus invalidating it, that Once becomes poisoned.

@steveklabnik
Copy link
Member

Triage: no changes here that I'm aware of.

@sanmai-NL
Copy link

#43448 was closed. @joshlf Can this issue be expedited, without discussing more than one scenario in which the state should be poisoned?

@joshlf
Copy link
Contributor

joshlf commented Apr 30, 2020

Yes, the "poison on fork" proposal should be considered dead for the time being. The discussion in this issue can safely ignore that proposal.

@Amanieu
Copy link
Member

Amanieu commented Apr 30, 2020

I think this API is well designed and worth stabilizing. Note that parking_lot uses a difference OnceState, but I think the current one we have in libstd is fine as it is.

@rfcbot fcp merge

@rfcbot
Copy link

rfcbot commented Apr 30, 2020 •
edited by dtolnay
Loading

Team member @Amanieu has proposed to merge this. The next step is review by the rest of the tagged team members:

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@rfcbot rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Apr 30, 2020
@rfcbot
Copy link

rfcbot commented May 1, 2020

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot rfcbot removed the proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. label May 1, 2020
@Kixunil
Copy link
Contributor

Kixunil commented May 2, 2020

Apart from consistency I find is_poisoned more clear than poisoned. The latter sounds a bit like a constructor that creates a poisoned instance.

@rfcbot rfcbot added the finished-final-comment-period The final comment period is finished for this PR / Issue. label May 11, 2020
@rfcbot
Copy link

rfcbot commented May 11, 2020

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

The RFC will be merged soon.

@rfcbot rfcbot removed the final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. label May 11, 2020
@spastorino spastorino mentioned this issue Jul 9, 2020
Merged
@KodrAus KodrAus added A-concurrency Area: Concurrency Libs-Tracked Libs issues that are tracked on the team's project board. labels Jul 29, 2020
@nbdd0121 nbdd0121 mentioned this issue Aug 26, 2020
Open
@Kixunil
Copy link
Contributor

Kixunil commented Feb 3, 2021

Should this API stabilize after successful FCP or do I misunderstand stabilization process?

@Amanieu
Copy link
Member

Amanieu commented Feb 3, 2021

Someone needs to submit a stabilization PR for this feature.

@Kixunil Kixunil mentioned this issue Feb 4, 2021
Merged
@Kixunil
Copy link
Contributor

Kixunil commented Feb 4, 2021

@Amanieu OK, I decided to become that someone. :) Thanks for clarifying!

m-ou-se added a commit to m-ou-se/rust that referenced this issue Feb 4, 2021
@m-ou-se
Rollup merge of rust-lang#81745 - Kixunil:stabilize_once_poison, r=m-…
7066afb 
…ou-se

Stabilize poison API of Once, rename poisoned()

This stabilizes:

* `OnceState`
* `OnceState::is_poisoned()` (previously named `poisoned()`)
* `Once::call_once_force()`

`poisoned()` was renamed because the new name is more clear as a few
people agreed and nobody objected.

Closes rust-lang#33577

Notes:

* I'm not entirely sure it's supposed to be 1.51, LMK if I did it wrong
* I failed to run tests locally, so we will have to leave it to bors or someone else can try
m-ou-se added a commit to m-ou-se/rust that referenced this issue Feb 4, 2021
@m-ou-se
Rollup merge of rust-lang#81745 - Kixunil:stabilize_once_poison, r=m-…
6f014cd 
…ou-se

Stabilize poison API of Once, rename poisoned()

This stabilizes:

* `OnceState`
* `OnceState::is_poisoned()` (previously named `poisoned()`)
* `Once::call_once_force()`

`poisoned()` was renamed because the new name is more clear as a few
people agreed and nobody objected.

Closes rust-lang#33577

Notes:

* I'm not entirely sure it's supposed to be 1.51, LMK if I did it wrong
* I failed to run tests locally, so we will have to leave it to bors or someone else can try
@bors bors closed this as completed in f42e961 Feb 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-concurrency Area: Concurrency B-unstable Blocker: Implemented in the nightly compiler and unstable. C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Stabilize poison API of Once, rename poisoned() Kixunil/rust