rust icon indicating copy to clipboard operation
rust copied to clipboard

Tracking Issue for `ReentrantLock`

Open joboet opened this issue 1 year ago • 5 comments

Feature gate: #![feature(reentrant_lock)]

This is a tracking issue for ReentrantLock, a re-entrant lock useful for avoiding a common type of deadlocks.

Public API

// std::sync

pub struct ReentrantLock<T: ?Sized> {}
pub struct ReentrantLockGuard<'a, T: ?Sized + 'a> {}

impl<T> ReentrantLock<T> {
    pub const fn new(t: T) -> ReentrantLock<T>;
    pub fn into_inner(self) -> T;
}

impl<T: ?Sized> ReentrantLock<T> {
    pub fn lock(&self) -> ReentrantLockGuard<'_, T>;
    pub fn get_mut(&mut self) -> &mut T;
}

impl<T: Send + ?Sized> Send for ReentrantLock<T> {}
impl<T: Send + ?Sized> Sync for ReentrantLock<T> {}

impl<T: ?Sized> !Send for ReentrantLockGuard<'_, T> {}
impl<T: ?Sized + Sync> Sync for ReentrantLockGuard<'_, T> {}

impl<T: UnwindSafe + ?Sized> UnwindSafe for ReentrantLock<T> {}
impl<T: RefUnwindSafe + ?Sized> RefUnwindSafe for ReentrantLock<T> {}

impl<T: Default> Default for ReentrantLock<T> {}
impl<T> From<T> for ReentrantLock<T> {}

impl<T: ?Sized> Deref for ReentrantLockGuard<'_, T> {
    type Target = T;
}

impl<T: Debug + ?Sized> Debug for ReentrantLock<T> {}
impl<T: Debug + ?Sized> Debug for ReentrantLockGuard<'_, T> {}
impl<T: Display + ?Sized> Display for ReentrantLockGuard<'_, T> {}

Steps / History

  • [x] ACP: rust-lang/libs-team#193
  • [x] Implementation: #110543
  • [ ] Final comment period (FCP)^1
  • [ ] Stabilization PR

Unresolved Questions

Poisoning

I would argue that ReentrantLock should not implement lock poisoning. Since it only provides shared access, breaking the invariants of the inner type is something that the lock has no control over, as it requires interior mutability. It would make sense to require that T: RefUnwindSafe, but I think that is too inconvenient for now. If RefUnwindSafe were a lint trait, it should definitely be used.

Fallible locking

Is there a need for fallible locking, even if it cannot fail because of reentrancy? How would this look like, considering that TryLockResult also has a poisoning case, which would never be used here?

Naming

New synchronization primitives like OnceLock have used the Lock suffix. IMHO the name ReentrantLock rhymes well with the other types and is more consistent, but ReentrantMutex better highlights the similarity to Mutex.

joboet avatar Feb 22 '24 08:02 joboet

New synchronization primitives like OnceLock have used the Lock suffix. IMHO the name ReentrantLock rhymes well with the other types and is more consistent, but ReentrantMutex better highlights the similarity to Mutex.

I'll put in my vote for ReentrantMutex.

OnceLock isn't even a lock or a mutex, it's a thread-safe version of OnceCell. I don't know why it has "lock" in its name but IMO that's not a good justification for the asymmetry between Mutex and ReentrantMutex.

RalfJung avatar Mar 12 '24 13:03 RalfJung

I'm still dreaming of a future in which we have a poison-free Lock instead of Mutex. In that future, ReentrantLock is the perfect name. But I'm not convinced that it's achievable. Still, I'd like to defer the final decision until stabilization.

joboet avatar Mar 12 '24 13:03 joboet

I don't see why "Mutex" would implicitly mean "poisoned". That's not an obvious naming scheme at all.

RalfJung avatar Apr 04 '24 11:04 RalfJung

If we have unpoisoned mutexes in the future, it'd probably be better to do something like CleanReentrantMutex. And maybe it could be aliased in a future edition, but that's obviously outside this issue's scope.

botahamec avatar Apr 04 '24 20:04 botahamec

Alright question, why not have interior mutability on the ReentrantLock? I don't think there's a data race problem here, but I might be missing something

Eclipse32767 avatar May 13 '24 22:05 Eclipse32767

You mean returning an &mut when acquiring the lock?

Then I could acquire the lock twice in the same thread, have two aliasing mutable references, and proceed to break everything from there.

RalfJung avatar May 14 '24 05:05 RalfJung

Good catch, I didn’t notice why everything blows up with this, just noticed how interior mutability was out of the question and wondered why

Eclipse32767 avatar May 14 '24 19:05 Eclipse32767

Please make try_lock be public to be the same as Mutex and RwLock. ReentrantLock would have more use cases if it can directly replace Mutex when an object is locked more than once.

radiantgurl avatar May 24 '24 08:05 radiantgurl

OnceLock isn't even a lock or a mutex, it's a thread-safe version of OnceCell. I don't know why it has "lock" in its name but IMO that's not a good justification for the asymmetry between Mutex and ReentrantMutex.

It is the sort of lock you throw away the key to.

Perhaps OnceDungeon sounded too grim.

workingjubilee avatar May 24 '24 23:05 workingjubilee

https://github.com/rust-lang/rust/pull/125527 corrects the Sync impl to be something actually suitable for a publicly usable type.

workingjubilee avatar May 25 '24 01:05 workingjubilee