From 68a18e2d14e523e08d6c311a2c64693e1db75848 Mon Sep 17 00:00:00 2001 From: Sunjay Varma Date: Sun, 18 Apr 2021 20:22:43 -0700 Subject: [PATCH] Re-implemented `map` associated function for `MutexGuard` with re-introduced `MappedMutexGuard` type --- tokio/src/sync/mod.rs | 2 +- tokio/src/sync/mutex.rs | 157 ++++++++++++++++++++++++++++++---------- 2 files changed, 118 insertions(+), 41 deletions(-) diff --git a/tokio/src/sync/mod.rs b/tokio/src/sync/mod.rs index d89a9ddce3a..5f97c1afb2b 100644 --- a/tokio/src/sync/mod.rs +++ b/tokio/src/sync/mod.rs @@ -436,7 +436,7 @@ cfg_sync! { pub mod mpsc; mod mutex; - pub use mutex::{Mutex, MutexGuard, TryLockError, OwnedMutexGuard}; + pub use mutex::{Mutex, MutexGuard, TryLockError, OwnedMutexGuard, MappedMutexGuard}; pub(crate) mod notify; pub use notify::Notify; diff --git a/tokio/src/sync/mutex.rs b/tokio/src/sync/mutex.rs index 851c04fcca0..2e8dfd6f473 100644 --- a/tokio/src/sync/mutex.rs +++ b/tokio/src/sync/mutex.rs @@ -4,11 +4,9 @@ use crate::sync::batch_semaphore as semaphore; use std::cell::UnsafeCell; use std::error::Error; -use std::fmt; -use std::marker; -use std::mem; use std::ops::{Deref, DerefMut}; use std::sync::Arc; +use std::{fmt, marker, mem}; /// An asynchronous `Mutex`-like type. /// @@ -140,10 +138,7 @@ pub struct Mutex { /// The lock is automatically released whenever the guard is dropped, at which /// point `lock` will succeed yet again. pub struct MutexGuard<'a, T: ?Sized> { - s: &'a semaphore::Semaphore, - data: *mut T, - // Needed to tell the borrow checker that we are holding a `&mut T` - marker: marker::PhantomData<&'a mut T>, + lock: &'a Mutex, } /// An owned handle to a held `Mutex`. @@ -165,14 +160,27 @@ pub struct OwnedMutexGuard { lock: Arc>, } +/// A handle to a held `Mutex` that has had a function applied to it via [`MutexGuard::map`]. +/// +/// This can be used to hold a subfield of the protected data. +/// +/// [`MutexGuard::map`]: method@MutexGuard::map +#[must_use = "if unused the Mutex will immediately unlock"] +pub struct MappedMutexGuard<'a, T: ?Sized> { + s: &'a semaphore::Semaphore, + data: *mut T, + // Needed to tell the borrow checker that we are holding a `&mut T` + marker: marker::PhantomData<&'a mut T>, +} + // As long as T: Send, it's fine to send and share Mutex between threads. // If T was not Send, sending and sharing a Mutex would be bad, since you can // access T through Mutex. unsafe impl Send for Mutex where T: ?Sized + Send {} unsafe impl Sync for Mutex where T: ?Sized + Send {} -unsafe impl<'a, T> Send for MutexGuard<'a, T> where T: ?Sized + Send + Sync {} -unsafe impl<'a, T> Sync for MutexGuard<'a, T> where T: ?Sized + Send + Sync {} +unsafe impl Sync for MutexGuard<'_, T> where T: ?Sized + Send + Sync {} unsafe impl Sync for OwnedMutexGuard where T: ?Sized + Send + Sync {} +unsafe impl Sync for MappedMutexGuard<'_, T> where T: ?Sized + Send + Sync {} /// Error returned from the [`Mutex::try_lock`], [`RwLock::try_read`] and /// [`RwLock::try_write`] functions. @@ -283,11 +291,7 @@ impl Mutex { /// ``` pub async fn lock(&self) -> MutexGuard<'_, T> { self.acquire().await; - MutexGuard { - s: &self.s, - data: self.c.get(), - marker: marker::PhantomData, - } + MutexGuard { lock: self } } /// Locks this mutex, causing the current task to yield until the lock has @@ -348,11 +352,7 @@ impl Mutex { /// ``` pub fn try_lock(&self) -> Result, TryLockError> { match self.s.try_acquire(1) { - Ok(_) => Ok(MutexGuard { - s: &self.s, - data: self.c.get(), - marker: marker::PhantomData, - }), + Ok(_) => Ok(MutexGuard { lock: self }), Err(_) => Err(TryLockError(())), } } @@ -466,9 +466,7 @@ where // === impl MutexGuard === impl<'a, T: ?Sized> MutexGuard<'a, T> { - /// Makes a new [`MutexGuard`] for a component of the locked data. - /// - /// This can be used to hold a subfield of the protected data. + /// Makes a new [`MappedMutexGuard`] for a component of the locked data. /// /// This operation cannot fail as the [`MutexGuard`] passed in already locked the mutex. /// @@ -497,26 +495,23 @@ impl<'a, T: ?Sized> MutexGuard<'a, T> { /// ``` /// /// [`MutexGuard`]: struct@MutexGuard + /// [`MappedMutexGuard`]: struct@MappedMutexGuard #[inline] - pub fn map(mut this: Self, f: F) -> MutexGuard<'a, U> + pub fn map(mut this: Self, f: F) -> MappedMutexGuard<'a, U> where F: FnOnce(&mut T) -> &mut U, { let data = f(&mut *this) as *mut U; - let s = this.s; - - // Need to forget `this` so that the mutex does not unlock when that is dropped - // Essentially transferring ownership to the new `MutexGuard` + let s = &this.lock.s; mem::forget(this); - - MutexGuard { + MappedMutexGuard { s, data, marker: marker::PhantomData, } } - /// Attempts to make a new [`MutexGuard`] for a component of the locked data. The + /// Attempts to make a new [`MappedMutexGuard`] for a component of the locked data. The /// original guard is returned if the closure returns `None`. /// /// This operation cannot fail as the [`MutexGuard`] passed in already locked the mutex. @@ -547,8 +542,9 @@ impl<'a, T: ?Sized> MutexGuard<'a, T> { /// ``` /// /// [`MutexGuard`]: struct@MutexGuard + /// [`MappedMutexGuard`]: struct@MappedMutexGuard #[inline] - pub fn try_map(mut this: Self, f: F) -> Result, Self> + pub fn try_map(mut this: Self, f: F) -> Result, Self> where F: FnOnce(&mut T) -> Option<&mut U>, { @@ -556,13 +552,9 @@ impl<'a, T: ?Sized> MutexGuard<'a, T> { Some(data) => data as *mut U, None => return Err(this), }; - let s = this.s; - - // Need to forget `this` so that the mutex does not unlock when that is dropped - // Essentially transferring ownership to the new `MutexGuard` + let s = &this.lock.s; mem::forget(this); - - Ok(MutexGuard { + Ok(MappedMutexGuard { s, data, marker: marker::PhantomData, @@ -572,20 +564,20 @@ impl<'a, T: ?Sized> MutexGuard<'a, T> { impl Drop for MutexGuard<'_, T> { fn drop(&mut self) { - self.s.release(1) + self.lock.s.release(1) } } impl Deref for MutexGuard<'_, T> { type Target = T; fn deref(&self) -> &Self::Target { - unsafe { &*self.data } + unsafe { &*self.lock.c.get() } } } impl DerefMut for MutexGuard<'_, T> { fn deref_mut(&mut self) -> &mut Self::Target { - unsafe { &mut *self.data } + unsafe { &mut *self.lock.c.get() } } } @@ -633,3 +625,88 @@ impl fmt::Display for OwnedMutexGuard { fmt::Display::fmt(&**self, f) } } + +// === impl MappedMutexGuard === + +impl<'a, T: ?Sized> MappedMutexGuard<'a, T> { + /// Makes a new [`MappedMutexGuard`] for a component of the locked data. + /// + /// This operation cannot fail as the [`MappedMutexGuard`] passed in already locked the mutex. + /// + /// This is an associated function that needs to be used as `MappedMutexGuard::map(...)`. A + /// method would interfere with methods of the same name on the contents of the locked data. + /// + /// [`MappedMutexGuard`]: struct@MappedMutexGuard + #[inline] + pub fn map(mut this: Self, f: F) -> MappedMutexGuard<'a, U> + where + F: FnOnce(&mut T) -> &mut U, + { + let data = f(&mut *this) as *mut U; + let s = this.s; + mem::forget(this); + MappedMutexGuard { + s, + data, + marker: marker::PhantomData, + } + } + + /// Attempts to make a new [`MappedMutexGuard`] for a component of the locked data. The + /// original guard is returned if the closure returns `None`. + /// + /// This operation cannot fail as the [`MappedMutexGuard`] passed in already locked the mutex. + /// + /// This is an associated function that needs to be used as `MappedMutexGuard::try_map(...)`. A + /// method would interfere with methods of the same name on the contents of the locked data. + /// + /// [`MappedMutexGuard`]: struct@MappedMutexGuard + #[inline] + pub fn try_map(mut this: Self, f: F) -> Result, Self> + where + F: FnOnce(&mut T) -> Option<&mut U>, + { + let data = match f(&mut *this) { + Some(data) => data as *mut U, + None => return Err(this), + }; + let s = this.s; + mem::forget(this); + Ok(MappedMutexGuard { + s, + data, + marker: marker::PhantomData, + }) + } +} + +impl<'a, T: ?Sized> Drop for MappedMutexGuard<'a, T> { + fn drop(&mut self) { + self.s.release(1) + } +} + +impl<'a, T: ?Sized> Deref for MappedMutexGuard<'a, T> { + type Target = T; + fn deref(&self) -> &Self::Target { + unsafe { &*self.data } + } +} + +impl<'a, T: ?Sized> DerefMut for MappedMutexGuard<'a, T> { + fn deref_mut(&mut self) -> &mut Self::Target { + unsafe { &mut *self.data } + } +} + +impl<'a, T: ?Sized + fmt::Debug> fmt::Debug for MappedMutexGuard<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Debug::fmt(&**self, f) + } +} + +impl<'a, T: ?Sized + fmt::Display> fmt::Display for MappedMutexGuard<'a, T> { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + fmt::Display::fmt(&**self, f) + } +}