-
Notifications
You must be signed in to change notification settings - Fork 13.3k
Add RwLockReadGuard::map for transforming guards to contain sub-borrows. #30834
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -121,7 +121,7 @@ pub const RW_LOCK_INIT: StaticRwLock = StaticRwLock::new(); | |
#[stable(feature = "rust1", since = "1.0.0")] | ||
pub struct RwLockReadGuard<'a, T: ?Sized + 'a> { | ||
__lock: &'a StaticRwLock, | ||
__data: &'a UnsafeCell<T>, | ||
__data: &'a T, | ||
} | ||
|
||
#[stable(feature = "rust1", since = "1.0.0")] | ||
|
@@ -417,10 +417,37 @@ impl<'rwlock, T: ?Sized> RwLockReadGuard<'rwlock, T> { | |
poison::map_result(lock.poison.borrow(), |_| { | ||
RwLockReadGuard { | ||
__lock: lock, | ||
__data: data, | ||
__data: unsafe { &*data.get() }, | ||
} | ||
}) | ||
} | ||
|
||
/// Transform this guard to hold a sub-borrow of the original data. | ||
/// | ||
/// Applies the supplied closure to the data, returning a new lock | ||
/// guard referencing the borrow returned by the closure. | ||
/// | ||
/// ```rust | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could you add a |
||
/// # use std::sync::{RwLockReadGuard, RwLock}; | ||
/// let x = RwLock::new(vec![1, 2]); | ||
/// | ||
/// let y = RwLockReadGuard::map(x.read().unwrap(), |v| &v[0]); | ||
/// assert_eq!(*y, 1); | ||
/// ``` | ||
#[unstable(feature = "guard_map", | ||
reason = "recently added, needs RFC for stabilization", | ||
issue = "0")] | ||
pub fn map<U: ?Sized, F>(this: Self, cb: F) -> RwLockReadGuard<'rwlock, U> | ||
where F: FnOnce(&'rwlock T) -> &'rwlock U { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Stylistically where clauses like this in the standard library tend to be formatted like: pub fn map<F>(...) -> T
where F: ...
{
// ...
} |
||
let new = RwLockReadGuard { | ||
__lock: this.__lock, | ||
__data: cb(this.__data) | ||
}; | ||
|
||
mem::forget(this); | ||
|
||
new | ||
} | ||
} | ||
|
||
impl<'rwlock, T: ?Sized> RwLockWriteGuard<'rwlock, T> { | ||
|
@@ -434,13 +461,52 @@ impl<'rwlock, T: ?Sized> RwLockWriteGuard<'rwlock, T> { | |
} | ||
}) | ||
} | ||
|
||
/// Transform this guard to hold a sub-borrow of the original data. | ||
/// | ||
/// Applies the supplied closure to the data, returning a new lock | ||
/// guard referencing the borrow returned by the closure. | ||
/// | ||
/// ```rust | ||
/// # use std::sync::{RwLockWriteGuard, RwLock}; | ||
/// let x = RwLock::new(vec![1, 2]); | ||
/// | ||
/// { | ||
/// let y = RwLockWriteGuard::map(x.write().unwrap(), |v| &mut v[0]); | ||
/// assert_eq!(*y, 1); | ||
/// | ||
/// *y = 10; | ||
/// } | ||
/// | ||
/// assert_eq!(&**x.read().unwrap(), &[10, 2]); | ||
/// ``` | ||
#[unstable(feature = "guard_map", | ||
reason = "recently added, needs RFC for stabilization", | ||
issue = "0")] | ||
pub fn map<U: ?Sized, F>(this: Self, cb: F) -> RwLockWriteGuard<'rwlock, U> | ||
where F: FnOnce(&'rwlock mut T) -> &'rwlock mut U { | ||
let new_data = unsafe { | ||
let data: &'rwlock mut T = &mut *this.__data.get(); | ||
mem::transmute::<&'rwlock mut U, &'rwlock UnsafeCell<U>>(cb(data)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The strategy above of changing the stored type to just There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll change both |
||
}; | ||
|
||
let poison = unsafe { ptr::read(&this.__poison) }; | ||
let lock = unsafe { ptr::read(&this.__lock) }; | ||
mem::forget(this); | ||
|
||
RwLockWriteGuard { | ||
__lock: lock, | ||
__data: new_data, | ||
__poison: poison | ||
} | ||
} | ||
} | ||
|
||
#[stable(feature = "rust1", since = "1.0.0")] | ||
impl<'rwlock, T: ?Sized> Deref for RwLockReadGuard<'rwlock, T> { | ||
type Target = T; | ||
|
||
fn deref(&self) -> &T { unsafe { &*self.__data.get() } } | ||
fn deref(&self) -> &T { self.__data } | ||
} | ||
|
||
#[stable(feature = "rust1", since = "1.0.0")] | ||
|
@@ -481,7 +547,7 @@ mod tests { | |
use rand::{self, Rng}; | ||
use sync::mpsc::channel; | ||
use thread; | ||
use sync::{Arc, RwLock, StaticRwLock, TryLockError}; | ||
use sync::{Arc, RwLock, StaticRwLock, TryLockError, RwLockWriteGuard}; | ||
use sync::atomic::{AtomicUsize, Ordering}; | ||
|
||
#[derive(Eq, PartialEq, Debug)] | ||
|
@@ -729,4 +795,20 @@ mod tests { | |
Ok(x) => panic!("get_mut of poisoned RwLock is Ok: {:?}", x), | ||
} | ||
} | ||
|
||
#[test] | ||
fn test_rwlock_write_map_poison() { | ||
let rwlock = Arc::new(RwLock::new(vec![1, 2])); | ||
let rwlock2 = rwlock.clone(); | ||
|
||
thread::spawn(move || { | ||
let _ = RwLockWriteGuard::map::<usize, _>(rwlock2.write().unwrap(), |_| panic!()); | ||
}).join().unwrap_err(); | ||
|
||
match rwlock.read() { | ||
Ok(r) => panic!("Read lock on poisioned RwLock is Ok: {:?}", &*r), | ||
Err(_) => {} | ||
}; | ||
} | ||
} | ||
|
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.
An issue should be opened and these numbers updated before this lands.
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.
We're going to fold this into #27746 so that's the issue number to use here.