Why is `MutexGuard<'a, T>` like that?

1

Recently, I wrote a semaphore implementation in rust.

When writing the guard type, I settled on something that looks like this:

#[repr(transparent)]
pub struct Guard<T: Borrow<Semaphore>> {
    semaphore: ManuallyDrop<T>,
}

impl<T: Borrow<Semaphore>> Drop for Guard<T> {
    fn drop(&mut self) {
        std::mem::drop(unsafe { self.destroy() });
    }
}

impl<T: Borrow<Semaphore>> Guard<T> {
    fn new(semaphore: T) -> Self {
        Self {
            semaphore: ManuallyDrop::new(semaphore),
        }
    }

    /// Moves the semaphore out of `self` and releases it
    // SAFETY: callers must ensure that `self` is never used again
    unsafe fn destroy(&mut self) -> T {
        let sem = unsafe { ManuallyDrop::take(&mut self.semaphore) };
        sem.borrow().release_raw();
        sem
    }

    /// Drops the guard, returning the underlying reference used to create it
    pub fn release(mut self) -> T {
        let sem = unsafe { self.destroy() };
        std::mem::forget(self);
        sem
    }
}


pub trait SemaphoreExt: Borrow<Semaphore> + Sized {
    /// Blocks until a value can be acquired from the semaphore. Returns a guard
    /// that contains `Self`
    fn acquire(self) -> Guard<Self>;

    fn try_acquire(self) -> Result<Guard<Self>, Self>;
}

impl<T: Borrow<Semaphore>> SemaphoreExt for T {
    fn acquire(self) -> Guard<Self> {
        self.borrow().acquire_raw();
        Guard::new(self)
    }

    fn try_acquire(self) -> Result<Guard<Self>, Self> {
        if self.borrow().try_acquire_raw() {
            Ok(Guard::new(self))
        } else {
            Err(self)
        }
    }
}

I was comparing it to std::sync::MutexGuard, and would appreciate some other opinions about those differences.

The standard mutex guard seems to capture a &Mutex. I like my approach because it allows for constructs like Guard<Arc<Semaphore>>, which is 'static. AFAIK, you can't create a std MutexGuard with an owned lifetime. At the same time, Guard<&'_ Semaphore> is a perfectly fine type, and behaves exactly as you'd expect.

I feel as though there must be limitations to my approach that I am not thinking of. Otherwise, this seems strictly more flexible than std::sync::MutexGuard.

2

There are mutexes out there that allow owned guards, but they don't do it like that. They just have separate MutexGuard and OwnedMutexGuard types.

I do think that just having two types is preferable to the approach you suggested. Two types is easier to understand and makes the common case simpler.

3

One thing to consider is that an implementation of Borrow (or even Deref) may return different references at different calls.

impl Borrow<Bar> for Foo {
    fn borrow(&self) -> &Foo {
        if rand() {
            &self.left
        } else {
            &self.right
        }
    }
}

Therefore, a guard implemented this way could be convinced to release/unlock a different lock or semaphore than it locked, resulting in incorrect behavior or unsoundness depending on the details.

In order to get robust behavior you need a “stable deref” trait which promises to always return the same reference — but that’s not a property you can require of a safe-to-implement trait.

2 Likes
4

It makes uncommon cases impossible, though. For example, if the OwnedMutexGuard bakes in the choice of std::sync::Arc, then it cannot be used with, say, loom::sync::Arc or triomphe::Arc.

(I think the best solution is for the language to gain support for safe self-referential structs, so a user’s choice of OwnedMutexGuard design can be constructed out of an Arc and a MutexGuard that borrows it. This would solve a lot of other composition problems too.)

5

That does make sense. I was not aware of the alternatives. Looking at tokio::sync::OwnedMutexGuard, though, it still seems slightly less flexible, as it selects Arc as the owning container, for you. If I have some struct Foo { sem: Semaphore, other_data: OtherType }, I could eventually create a Guard<Arc<Foo>>, which I could not do with an OwnedMutex.

I'm a little less readily convinced of this. pub type NormalSemaphoreGuard<'a> = Guard<&'a Semaphore> would effectively give the user-interface of two distinct types, and by writing everything as one type I can limit the amount of unsafe I need to verify.

6

I had not considered this, and that does make sense. I understand why the potential for misuse makes this a bad idea for inclusion in core libraries.