-
Notifications
You must be signed in to change notification settings - Fork 13.2k
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
Implementation of sync_nonpoison
and nonpoison_mutex
#134663
base: master
Are you sure you want to change the base?
Conversation
Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @thomcc (or someone else) some time within the next two weeks. Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (
|
This comment has been minimized.
This comment has been minimized.
/// # Examples | ||
/// | ||
/// ``` | ||
/// use std::sync::Mutex; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These doctests need to be updated, they are using the poisioning mutex
use crate::ptr::NonNull; | ||
use crate::sys::sync as sys; | ||
|
||
/// A mutual exclusion primitive useful for protecting shared data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The docs for this one could probably just have a single example and then suggest looking at std::sync::poison::Mutex
for more, so that we don't have the detailed documentation repeated in multiple locations. Also make sure the doctests are using the right type here.
@Amanieu you may want to double check this |
This comment was marked as resolved.
This comment was marked as resolved.
Looks like you messed up a rebase. No worries, mistakes like this happen sadly (thanks Git for being so hard to use!). To get rid of all these commits, do a |
This comment has been minimized.
This comment has been minimized.
b0a1973
to
f023e42
Compare
This comment has been minimized.
This comment has been minimized.
This seems fine as a first step. Longer term we may want to consider changing the poisoning mutexes to be a wrapper around a non-poisoning |
@Aandreba any updates on this? thanks |
Sorry about the delay, I've been busy with other stuff, but I have a bit of an opening this next days so I'll see how much I can do |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
1a6a770
to
aaa6e69
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
0a6f7bd
to
e03168b
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
The tests look like they are failing because the path displayed in UI output changed. You should be able to update these by running |
Co-authored-by: oxalica <hooccooh1896@gmail.com>
Co-authored-by: oxalica <hooccooh1896@gmail.com>
The job Click to see the possible cause of the failure (guessed by this bot)
|
Implementation of
sync_nonpoison
andnonpoison_mutex
Docs come straight from the poisoned Mutex, so they need to be updated.
Tracked by:
sync_nonpoison
andnonpoison_{condvar,mutex,once,rwlock}
#134645