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

Add some convenience methods for locks. #79434

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 76 additions & 1 deletion library/std/src/sync/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::mem;
use crate::ops::{Deref, DerefMut};
use crate::ptr;
use crate::sys_common::mutex as sys;
use crate::sys_common::poison::{self, LockResult, TryLockError, TryLockResult};
use crate::sys_common::poison::{self, LockResult, PoisonError, TryLockError, TryLockResult};

/// A mutual exclusion primitive useful for protecting shared data
///
Expand Down Expand Up @@ -399,6 +399,73 @@ impl<T: ?Sized> Mutex<T> {
let data = self.data.get_mut();
poison::map_result(self.poison.borrow(), |_| data)
}

/// Run the given closure while holding the lock. The guarded value is mutably passed to the
/// closure, and the result of the closure is returned from `with`.
///
/// This function is functionally equivalent to calling `lock` before the closure and dropping
/// the `MutexGuard` immediately after.
///
/// # Errors
///
/// If another user of this mutex panicked while holding the mutex, then
/// this call will return an error once the mutex is acquired.
///
/// # Panics
///
/// This function might panic when called if the lock is already held by
/// the current thread.
///
/// # Example
///
/// ```
/// #![feature(mutex_with)]
/// use std::sync::Mutex;
///
/// let mutex = Mutex::new(0);
/// let result = mutex.with(|myint| { *myint += 1; *myint * 42 }).unwrap();
/// assert_eq!(*mutex.lock().unwrap(), 1);
/// assert_eq!(result, 42);
/// ```
#[unstable(feature = "mutex_with", issue = "none")]
pub fn with<R, F>(&self, f: F) -> Result<R, PoisonError<MutexGuard<'_, T>>>
m-ou-se marked this conversation as resolved.
Show resolved Hide resolved
where
F: FnOnce(&mut T) -> R,
{
Ok(f(&mut *self.lock()?))
}

/// Attempt to acquire the lock. If it cannot be acquired at this time, [`Err`] is returned. If
/// it is acquired, run the given closure with the lock held.
///
/// This function is functionally equivalent to calling `try_lock`, and calling the closure if
/// the lock is acquired. After the closure is run, the `MutexGuard` is dropped immediately.
///
/// This function does not block.
///
/// # Errors
///
/// If another user of this mutex panicked while holding the mutex, then
/// this call will return an error if the mutex would otherwise be
/// acquired.
///
/// # Examples
/// ```
/// #![feature(mutex_with)]
/// use std::sync::Mutex;
///
/// let mutex = Mutex::new(0);
/// let result = mutex.try_with(|myint| { *myint += 1; *myint * 42 }).unwrap();
/// assert_eq!(*mutex.lock().unwrap(), 1);
/// assert_eq!(result, 42);
/// ```
#[unstable(feature = "mutex_with", issue = "none")]
pub fn try_with<R, F>(&self, f: F) -> Result<R, TryLockError<MutexGuard<'_, T>>>
where
F: FnOnce(&mut T) -> R,
{
Ok(f(&mut *self.try_lock()?))
}
}

#[stable(feature = "mutex_from", since = "1.24.0")]
Expand Down Expand Up @@ -444,6 +511,14 @@ impl<'mutex, T: ?Sized> MutexGuard<'mutex, T> {
unsafe fn new(lock: &'mutex Mutex<T>) -> LockResult<MutexGuard<'mutex, T>> {
poison::map_result(lock.poison.borrow(), |guard| MutexGuard { lock, poison: guard })
}

/// Immediately drops `self`, and consequently unlocks the mutex.
///
/// This is equivalent to calling [`Drop::drop`] on the guard, but is more self-documenting.
#[unstable(feature = "guard_unlock", issue = "none")]
pub fn unlock(self) {
drop(self);
}
Comment on lines +518 to +521
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MutexGuard doesn't have any methods right now, because it implements Deref. If the T already has an .unlock(), this will make it harder to call that .unlock() through this MutexGuard. Any existing code calling such an .unlock() would change meaning with this change.

Other types like Box solve this by not taking self in their methods (e.g. Box::into_raw), and must be called with the more verbose Box::into_raw(mybox) syntax.

But I think MutexGuard::unlock(guard); wouldn't be a big improvement over drop(guard);. :(

What do you think?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm... I had not thought of that. I guess the primary difference in my mind was to make the code a bit self-documenting. It is not obvious to me when I see drop(foo) that a lock might be unlocked. I'm ok with the more verbose syntax, personally, but if we are going to do that perhaps we could do it like this instead:

impl Mutex {
  pub fn unlock(guard: MutexGuard) { drop(guard); }
}

Then, one would use it like

Mutex::unlock(guard);

which seems about as self-documenting as it gets...

}

#[stable(feature = "rust1", since = "1.0.0")]
Expand Down