Added unlock functionality to mutex guard and rw lock guards#149189
Added unlock functionality to mutex guard and rw lock guards#149189SpaceBroetchen wants to merge 5 commits intorust-lang:mainfrom
Conversation
|
r? @ChrisDenton rustbot has assigned @ChrisDenton. Use |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
The job Click to see the possible cause of the failure (guessed by this bot) |
| unsafe { self.lock.inner.unlock() }; | ||
|
|
||
| func(); | ||
|
|
||
| self.lock.inner.lock(); |
There was a problem hiding this comment.
Not a fundamental design issue, but the implementation looks unsound in the presence of panics since it leaves the Mutex unlocked when exiting the function
let mutex = Mutex::new(1);
let mut guard = mutex.lock().unwrap();
panic::catch_unwind(AssertUnwindSafe(|| MutexGuard::unlocked(&mut guard, || panic!())));
let ref1 = &mut *guard;
let ref2 = &mut *mutex.lock();
dbg!(ref1, ref2);lock_api doesn't have this issue since it calls lock() in a local's Drop impl that always executes, should probably do the same here.
edit: Ah, I overlooked the new unlocked: bool field, the comment for it also talks about panicking in the closure, but from a cursory look I'm not sure it prevents this issue?
There was a problem hiding this comment.
Oh yes, that's unsound. I don't think the "no relock during panic" guarantee can be soundly provided, at least if unlocked only takes the mutex guard by reference.
|
|
||
| impl<'mutex, T: ?Sized> MutexGuard<'mutex, T> { | ||
| /// Unlocks the [`MutexGuard`] for the scope of `func` and acquires it again after. | ||
| /// Panics won't lock the guard again. |
There was a problem hiding this comment.
How important is this guarantee? I find it unfortunate that this necessitates adding a field and an additional check in the Drop implementation, which affects all code using MutexGuards, not just code that calls unlocked.
|
r? joboet |
Added unlock functionality for nonpoison mutex guards and RwLock guards.
Associated Tracking Issue: #148568