-
Notifications
You must be signed in to change notification settings - Fork 265
Add MappedArcMutexGuard to mirror MappedMutexGuard #457
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
I'm going to iterate a bit on the implementation of this PR and test it in various tricky production cases to make sure it is sound, so moving into Draft state |
412586c to
9cd6b13
Compare
d12c8eb to
ae19232
Compare
ae19232 to
7c352e6
Compare
|
Gonna do some more testing but I think this PR is ready to get new eyes on it. |
|
Unfortunately I don't think there is much value in adding this without support for type erasure, which is really the main selling point of |
|
I suspect there is a significant use for MappedMutexGuard (and an Arc version) beyond type erasure. Even when the inner type is public, the "mapping" function used might not, or might not be trivial enough to do on the consumer side. Also, couldn't we get type erasure here? What does triomphe bring to make it easier?
The main risk here is the "lie" in
There's just minor details regarding the use alloc::boxed::Box;
trait AnyCloneable {
fn any_clone(&self) -> Box<dyn AnyCloneable>;
}
impl <T: Clone> AnyCloneable for T {
fn any_clone(&self) -> Box<dyn AnyCloneable> {
Box::new(self.clone())
}
}And store a EDIT: I tried that, and it looks like it's working. Sent a PR with the experiment. Feel free to bash it if we don't want any of that. |
Add a new type,
MappedArcMutexGuard, that mirrors the API ofMappedMutexGuardbut with an ownedArc<>reference to the underlying mutex while locked.I didn't find any tests covering
MappedMutexGuard, so I didn't change the status quo. Instead, I copied the exact same implementation as forMappedMutexGuardbut with lifetimes removed and&'a Rreferences to the raw mutex replaced withArc<Mutex<T, R>>. I believe this should make the new type at least as sound as the previousMappedMutexGuard.Unfortunately, since we only have an
ArcaroundMutex, but not the raw mutex typeR, we cannot type erase the wrapped mutex and need to retain the original typeTinMappedArcMutexGuard<R, T, U>(as opposed toMappedArcMutexGuard<R, U>).The name was chosen based on the existing pattern of
MappedFairMutexGuardto arrive atMappedArcMutexGuard(as opposed to e.g.ArcMappedMutexGuardor whatever).