Skip to content

Commit

Permalink
lock_api: Make RawMutex::unlock() unsafe.
Browse files Browse the repository at this point in the history
Unlocking a Mutex that you do not hold is likely to cause undefined behavior. If unlock()
is a safe method, then it likely impossible to provide a sound and efficient implemention
of the method. Since this API is usually only called by the lock_api Mutex structs, this
change should be invisible to the users, but clarifies what guarantees implementers must make.
  • Loading branch information
Thomas Bächler committed Jun 17, 2020
1 parent 1f6e68e commit 14e28ca
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 12 deletions.
32 changes: 27 additions & 5 deletions lock_api/src/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,27 @@ pub unsafe trait RawMutex {
fn try_lock(&self) -> bool;

/// Unlocks this mutex.
fn unlock(&self);
///
/// # Safety
///
/// This method may only be called if the mutex is held in the current context, i.e. it must
/// be paired with a successful call to [`lock`], [`try_lock`], [`try_lock_for`] or [`try_lock_until`].
///
/// [`lock`]: #tymethod.lock
/// [`try_lock`]: #tymethod.try_lock
/// [`try_lock_for`]: trait.RawMutexTimed.html#tymethod.try_lock_for
/// [`try_lock_until`]: trait.RawMutexTimed.html#tymethod.try_lock_until
unsafe fn unlock(&self);

/// Checks whether the mutex is currently locked.
#[inline]
fn is_locked(&self) -> bool {
let acquired_lock = self.try_lock();
if acquired_lock {
self.unlock();
// Safety: The lock has been successfully acquired above.
unsafe {
self.unlock();
}
}
!acquired_lock
}
Expand Down Expand Up @@ -436,7 +449,10 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> MutexGuard<'a, R, T> {
where
F: FnOnce() -> U,
{
s.mutex.raw.unlock();
// Safety: A MutexGuard always holds the lock.
unsafe {
s.mutex.raw.unlock();
}
defer!(s.mutex.raw.lock());
f()
}
Expand Down Expand Up @@ -506,7 +522,10 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> DerefMut for MutexGuard<'a, R, T> {
impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> Drop for MutexGuard<'a, R, T> {
#[inline]
fn drop(&mut self) {
self.mutex.raw.unlock();
// Safety: A MutexGuard always holds the lock.
unsafe {
self.mutex.raw.unlock();
}
}
}

Expand Down Expand Up @@ -638,7 +657,10 @@ impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> DerefMut for MappedMutexGuard<'a, R,
impl<'a, R: RawMutex + 'a, T: ?Sized + 'a> Drop for MappedMutexGuard<'a, R, T> {
#[inline]
fn drop(&mut self) {
self.raw.unlock();
// Safety: A MappedMutexGuard always holds the lock.
unsafe {
self.raw.unlock();
}
}
}

Expand Down
21 changes: 17 additions & 4 deletions lock_api/src/remutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,12 @@ impl<R: RawMutex, G: GetThreadId> RawReentrantMutex<R, G> {

/// Unlocks this mutex. The inner mutex may not be unlocked if
/// this mutex was acquired previously in the current thread.
///
/// # Safety
///
/// This method may only be called if the mutex is held by the current thread.
#[inline]
pub fn unlock(&self) {
pub unsafe fn unlock(&self) {
let lock_count = self.lock_count.get() - 1;
self.lock_count.set(lock_count);
if lock_count == 0 {
Expand Down Expand Up @@ -554,7 +558,10 @@ impl<'a, R: RawMutex + 'a, G: GetThreadId + 'a, T: ?Sized + 'a> ReentrantMutexGu
where
F: FnOnce() -> U,
{
s.remutex.raw.unlock();
// Safety: A ReentrantMutexGuard always holds the lock.
unsafe {
s.remutex.raw.unlock();
}
defer!(s.remutex.raw.lock());
f()
}
Expand Down Expand Up @@ -623,7 +630,10 @@ impl<'a, R: RawMutex + 'a, G: GetThreadId + 'a, T: ?Sized + 'a> Drop
{
#[inline]
fn drop(&mut self) {
self.remutex.raw.unlock();
// Safety: A ReentrantMutexGuard always holds the lock.
unsafe {
self.remutex.raw.unlock();
}
}
}

Expand Down Expand Up @@ -762,7 +772,10 @@ impl<'a, R: RawMutex + 'a, G: GetThreadId + 'a, T: ?Sized + 'a> Drop
{
#[inline]
fn drop(&mut self) {
self.raw.unlock();
// Safety: A MappedReentrantMutexGuard always holds the lock.
unsafe {
self.raw.unlock();
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/raw_fair_mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ unsafe impl lock_api::RawMutex for RawFairMutex {
}

#[inline]
fn unlock(&self) {
unsafe fn unlock(&self) {
self.unlock_fair()
}

Expand Down
4 changes: 2 additions & 2 deletions src/raw_mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,8 +97,8 @@ unsafe impl lock_api::RawMutex for RawMutex {
}

#[inline]
fn unlock(&self) {
unsafe { deadlock::release_resource(self as *const _ as usize) };
unsafe fn unlock(&self) {
deadlock::release_resource(self as *const _ as usize);
if self
.state
.compare_exchange(LOCKED_BIT, 0, Ordering::Release, Ordering::Relaxed)
Expand Down

0 comments on commit 14e28ca

Please sign in to comment.