Skip to content

Expose mutex poisoning helpers #21565

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

Merged
merged 2 commits into from
Feb 8, 2015
Merged

Expose mutex poisoning helpers #21565

merged 2 commits into from
Feb 8, 2015

Conversation

kmcallister
Copy link
Contributor

I needed these to implement efficient poisoning in seqloq.

@rust-highfive
Copy link
Contributor

r? @huonw

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

I think adding PoisonError::new is probably fine, but can this avoid exposing map_lock_result for now? That should be implement-able with new exposed, right?

cc @aturon, an interesting conventions dilemma of whether we should provide constructors for all errors or largely just some method to reconstruct "wrapper errors" after decomposing them.

@kmcallister
Copy link
Contributor Author

can this avoid exposing map_lock_result for now? That should be implement-able with new exposed, right?

Yeah, I could copy it out, but I don't want to ;)

@alexcrichton
Copy link
Member

As an API we'll have to support to the end of time the threshold should be fairly high for inclusion. A bare map_lock_result doesn't really fit into any conventions that we have so far, so it should likely be left out until a later date if it is necessary to re-include.

@kmcallister
Copy link
Contributor Author

As an API we'll have to support to the end of time

It's marked as #[unstable]. I thought we could add things like this as part of the "stability not stagnation".

@alexcrichton
Copy link
Member

It's marked as #[unstable]. I thought we could add things like this as part of the "stability not stagnation".

Although we have a distinction of #[unstable] APIs in the standard library it does not mean that it will become a "dumping ground" for everything submitted. The lifetime of a new API is:

  1. An RFC is created for a new API and is discussed.
  2. The RFC is accepted.
  3. The RFC is implemented with #[unstable] APIs
  4. Some time later, the APIs are removed or promoted to #[stable]

All currently #[unstable] APIs are either planned for removal or planned to be revisited at some point in the future to stabilize.

Along these lines, I do not think that this map_lock_result is an API that we would want to stabilize, so it should not be included at this time. The PoisonError::new API is a fine candidate for stabilization, however, as it follows a well accepted pattern of constructors and is a very minor (and probably expected) addition to the API.

@huonw
Copy link
Member

huonw commented Feb 1, 2015

r? @alexcrichton

@rust-highfive rust-highfive assigned alexcrichton and unassigned huonw Feb 1, 2015
@aturon
Copy link
Member

aturon commented Feb 2, 2015

(FWIW, I agree with @alexcrichton here.)

@kmcallister
Copy link
Contributor Author

That sounds good, I'll update this PR soon.

@kmcallister
Copy link
Contributor Author

Updated to keep map_lock_result private.

@alexcrichton
Copy link
Member

@bors: r+ 7324c2c

@bors
Copy link
Collaborator

bors commented Feb 8, 2015

⌛ Testing commit 7324c2c with merge d4f9ec5...

bors added a commit that referenced this pull request Feb 8, 2015
@bors bors merged commit 7324c2c into rust-lang:master Feb 8, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants