Add MappedArcMutexGuard to mirror MappedMutexGuard

[gyscos]

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 the Arc is a bit wasteful (the allocation shouldn't be necessary).
  I realize that since T: ?Sized, Mutex<T> is also ?Sized, which means the size of Arc<Mutex<R, T>> depends on T (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 2 usize (8 or 16 bytes on x86_64). Maybe we could use a SmallBox?
• [EDIT: That point is now fixed.] The whole AnyCloneable trait was so we could have trait object that can be cloned, but now I think don't actually need cloning at all? When we map or try_map, we should be able to just move/re-use the Arc as-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 even mem::forget. Might involve wrapping ArcMutexGuard::mutex into something like ManuallyDrop so we could take from 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.

[gyscos]

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.

[Amanieu]

Rather than creating an allocation here, you could use Arc::into_raw after having turned the original Arc into an Arc<dyn Any>.

[gyscos]

I don't think I can turn a Arc<T: ?Sized> into a Arc<dyn Any>. If we could, we'd just need to directly store the Arc<dyn Any> :)

(I guess it's because the *const ArcInner<T> inside Arc<T> is already a fat pointer if T is unsized, and rust doesn't have super-fat pointers to store both the size and the trait object vtable. Would be a neat addition though!)

What I want is just the vtable for the drop glue, which trait objects include by default - so dropping the Box<dyn Any> will properly decrement the Arc counter. Even if I turn the Arc<Mutex<T>> into raw, I'm not sure how to decrement the counter during drop without knowing the T type. All the decrement functions I see need the T type at compile time, since they need to know what drop implementation to call.
Could do that with a trait object of a dedicated trait that decrements the counter, but that's already what Box<dyn Any> does.

[gyscos]

Well - we could start using Arc<dyn Any> everywhere, and downcasting to the Mutex when we know the types. A bit more intrusive change.

Nevermind we can't since we take the Arc<Mutex<R, T>> from the user.

EDIT: Also, I only now realize it's not just the conversion which is impossible, it's the representation itself. It's simply not possible to have a Arc<dyn Any> where the actual value is unsized. So if we wanted to have a Mutex<[u8]>, it could never be directly stored in a Arc<dyn Any> :(

[gyscos]

We could require T: Sized specifically for the arc-mapping functions, which would simplify a bit the code (and possibly relax this requirement in the future if we find a suitable solution).

For now I added an example mapping a ArcMutexGuard<Mutex<dyn Any>> to the concrete type, which requires an unsized T.

[Amanieu]

I would be OK restricting the mapping functions to T: Sized if it can remove the Box allocation. Unsized payloads (especially behind a mutex) are rarely used in practice anyways.

[gyscos]

I can update this PR for that. Will have it ready shortly. EDIT: Now pushed.

Of note, I do see a bunch of Arc<Mutex<dyn ...>> on github, and these are all unsized.

Of course we'd only prevent using map on them, so it wouldn't break anything (and many of them don't even use parking_lot), but I think there is a demand/use for unsized types in mutex in general.

[gyscos]

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.

[gyscos]

This test only shows that the happy path works. It's almost more of an example than a full test.

If desired, we could use more extensive tests to:

• Use a custom Drop impl to track when the value is dropped.
• Try (and catch_unwind) a mapping function that panics, see if the mutex is still usable.
• The same after running multiple mapping steps.
• Use some unsized type to show that they still work in either "core" or "mapped" position.

There's almost no upper limit to how detailed tests we could have. Just a lower limit to what we need :)

EDIT: Updated that test to include an example of unsized T (dyn Any), a fallible test, and a chained mapping.
EDIT2: Removed the test of an unsized type now that we require T: Sized for these methods.