Add MappedArcMutexGuard to mirror MappedMutexGuard
Here's a draft PR/experiment around bringing a MappedArcMutexGuard, without exposing the inner type in the generic parameters.
This picks up on #457 and type-erases the guard, as it was seen as the main drawback.
Compared to #457, it also adds a T: 'static bound to the map and try_map function (I suppose we could work out a lifetime-generic solution that doesn't require that if there is a need).
It uses a Box<dyn Any> to hide the Arc<Mutex<R, T>>. I wish I could have directly used a Arc<dyn Any> without the extra allocation/indirection, but R: ?Sized implies Mutes<R, T>: ?Sized so the conversion is not possible. Box it is, then.
It also holds a pointer to the raw mutex in a &'static R. The safety relies on the &'static R being outlived by the Arc it comes from (which lives in the same struct). It uses transmute for this.
While I believe this is safe, and the guarantees are local enough to be reasoned about, I understand if there is a blanket ban on transmute (it is quite a dangerous tool). The main idea here is a self-referencing struct (where we store both a Arc<Mutex> and a reference to the inner RawMutex). There might be existing generic self-referential crates available if we do not want to internalize this.
EDIT: I see some code to work with owning_ref (just to implement StableAddress). We could potentially use it for this case of storing both the Arc<Mutex> and the &RawMutex reference. Not sure it'd be better than doing it directly here.
EDIT2: I have tested this and it does work for my use-case (locking and downcasting a Arc<Mutex<Any>> to a MappedArcMutexGuard<T>).
EDIT3: Things I'm not 100% satisfied with (but could live with, with only minimal drinking involved):
- Using
Box<dyn AnyCloneable>to store theArcis a bit wasteful (the allocation shouldn't be necessary). I realize that sinceT: ?Sized,Mutex<T>is also?Sized, which means the size ofArc<Mutex<R, T>>depends onT(it can be a thin or a fat pointer), so we truely can't give it a fixed size. Though it's not like the size can actually vary wildly - it's either 1 or 2usize(8 or 16 bytes on x86_64). Maybe we could use aSmallBox? -
[EDIT: That point is now fixed.] The whole
AnyCloneabletrait was so we could have trait object that can be cloned, but now I think don't actually need cloning at all? When wemaportry_map, we should be able to just move/re-use theArcas-is, without having to clone it (increment) just to drop the original right after (decrement). Still wondering how to do that cleanly without having a partially-moved-out guard that we can't evenmem::forget. Might involve wrappingArcMutexGuard::mutexinto something likeManuallyDropso we couldtakefrom it before forgetting the full guard?... Would be nice getting rid of the atomic operations here.
On the plus side, it doesn't rely much on the arc internals, so it should be compatible with a move to triomphe.
Thanks to @dflemstr for the PR this is based on, and to all the parking_lot maintainers for your great work.
The last commit removes the need to clone Arc when moving them from one guard to the next. It does this by using ptr::read on the Arc that's about to be forgotten.
It does add a bit of unsafe boilerplate. I wish we could safely mem::forget partially-moved-out structs (so we could just move the Arc away from the guard, then forget it), but I don't think rust supports that at the moment.
I have another question regarding PhantomData, and if I should use one for MappedArcMutexGuard.
ArcMutexGuard has a PhantomData<*const ()>. I'm not sure what the PhantomData brings here - it doesn't alter any type parameter variance as far as I understand.
EDIT: Is it to disable the auto-trait implementation of Send/Sync? Doesn't the manual implementation already disable the automatic one?
MappedMutexGuard has a PhantomData<&'a mut T>, but it also has a data: *mut T field. That data field already makes the guard invariant over T, so the PhantomData is not adding anything here. Is it only for the implied T: 'a requirement?
EDIT: Ah looks like it might be for the covariance re: 'a.
MappedArcMutexGuard does not have the same lifetime requirement that MappedMutexGuard has. It already has a *mut U field, so it's already invariant over U. So I think it doesn't need any PhantomData.