From 72c4eda0d59f1839a170a81ce8e125fe9dc163d1 Mon Sep 17 00:00:00 2001 From: Sunjay Varma Date: Tue, 12 May 2020 12:29:27 -0400 Subject: [PATCH] Merging MappedMutexGuard into MutexGuard --- tokio/src/sync/mod.rs | 2 +- tokio/src/sync/mutex.rs | 151 ++++++++++------------------------------ 2 files changed, 37 insertions(+), 116 deletions(-) diff --git a/tokio/src/sync/mod.rs b/tokio/src/sync/mod.rs index 2fe3c53f1fb..3d96106d2df 100644 --- a/tokio/src/sync/mod.rs +++ b/tokio/src/sync/mod.rs @@ -442,7 +442,7 @@ cfg_sync! { pub mod mpsc; mod mutex; - pub use mutex::{Mutex, MutexGuard, TryLockError, OwnedMutexGuard, MappedMutexGuard}; + pub use mutex::{Mutex, MutexGuard, TryLockError, OwnedMutexGuard}; mod notify; pub use notify::Notify; diff --git a/tokio/src/sync/mutex.rs b/tokio/src/sync/mutex.rs index 82c531603e7..1dae4176149 100644 --- a/tokio/src/sync/mutex.rs +++ b/tokio/src/sync/mutex.rs @@ -132,7 +132,10 @@ 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> { - lock: &'a Mutex, + 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>, } /// An owned handle to a held `Mutex`. @@ -154,27 +157,14 @@ 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 OwnedMutexGuard where T: ?Sized + Send + Sync {} -unsafe impl<'a, T> Sync for MappedMutexGuard<'a, T> where T: ?Sized + Send + Sync {} /// Error returned from the [`Mutex::try_lock`] function. /// @@ -255,7 +245,11 @@ impl Mutex { /// ``` pub async fn lock(&self) -> MutexGuard<'_, T> { self.acquire().await; - MutexGuard { lock: self } + MutexGuard { + s: &self.s, + data: self.c.get(), + marker: marker::PhantomData, + } } /// Locks this mutex, causing the current task to yield until the lock has @@ -316,7 +310,11 @@ impl Mutex { /// ``` pub fn try_lock(&self) -> Result, TryLockError> { match self.s.try_acquire(1) { - Ok(_) => Ok(MutexGuard { lock: self }), + Ok(_) => Ok(MutexGuard { + s: &self.s, + data: self.c.get(), + marker: marker::PhantomData, + }), Err(_) => Err(TryLockError(())), } } @@ -392,7 +390,9 @@ where // === impl MutexGuard === impl<'a, T: ?Sized> MutexGuard<'a, T> { - /// Makes a new [`MappedMutexGuard`] for a component of the locked data. + /// Makes a new [`MutexGuard`] for a component of the locked data. + /// + /// This can be used to hold a subfield of the protected data. /// /// This operation cannot fail as the [`MutexGuard`] passed in already locked the mutex. /// @@ -421,23 +421,26 @@ impl<'a, T: ?Sized> MutexGuard<'a, T> { /// ``` /// /// [`MutexGuard`]: struct@MutexGuard - /// [`MappedMutexGuard`]: struct@MappedMutexGuard #[inline] - pub fn map(mut this: Self, f: F) -> MappedMutexGuard<'a, U> + pub fn map(mut this: Self, f: F) -> MutexGuard<'a, U> where F: FnOnce(&mut T) -> &mut U, { let data = f(&mut *this) as *mut U; - let s = &this.lock.s; + 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` mem::forget(this); - MappedMutexGuard { + + MutexGuard { s, data, marker: marker::PhantomData, } } - /// Attempts to make a new [`MappedMutexGuard`] for a component of the locked data. The + /// Attempts to make a new [`MutexGuard`] 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. @@ -468,9 +471,8 @@ 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>, { @@ -478,9 +480,13 @@ impl<'a, T: ?Sized> MutexGuard<'a, T> { Some(data) => data as *mut U, None => return Err(this), }; - let s = &this.lock.s; + 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` mem::forget(this); - Ok(MappedMutexGuard { + + Ok(MutexGuard { s, data, marker: marker::PhantomData, @@ -490,20 +496,20 @@ impl<'a, T: ?Sized> MutexGuard<'a, T> { impl Drop for MutexGuard<'_, T> { fn drop(&mut self) { - self.lock.s.release(1) + self.s.release(1) } } impl Deref for MutexGuard<'_, T> { type Target = T; fn deref(&self) -> &Self::Target { - unsafe { &*self.lock.c.get() } + unsafe { &*self.data } } } impl DerefMut for MutexGuard<'_, T> { fn deref_mut(&mut self) -> &mut Self::Target { - unsafe { &mut *self.lock.c.get() } + unsafe { &mut *self.data } } } @@ -551,88 +557,3 @@ 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) - } -}