Mutex<T> should be Share even if T isn't

[sfackler]

I think it's sound? It seems like a straightforward solution would be to add a #[force_share] attribute. We could also add an UnsafeForceShare marker type, but I think that'd be a bit strange since unlike the other markers, this one would remove a restriction on the type, not add one.

The current situation requires some gross workarounds like this: rust-postgres/rust-postgres@98c92c1

[sfackler]

In an opt-in kind world, this would probably be an #[unsafe_share] attribute that would work like #[unsafe_destructor].

[eddyb]

There is still an issue if the wrapped type is not safe to access from a different thread than the one it was created on.
EDIT: @nikomatsakis mentioned impl<T: Send> Share for Mutex<T> on IRC, which would solve the issue I thought of.

[alexcrichton]

I talked with @nikomatsakis about this on IRC.

He thinks that Unsafe<T> can be special (it's already a lang item), and Unsafe<T> will be Share regardless of whether T is share. This would fix the inference problem. Assuming an opt-in world, everything would look like:

struct Mutex<T> {
   lock: RawMutex,
   data: Unsafe<T>
}

impl<T: Send> Share for Mutex<T> {}

Today's compiler would reject this code because it does not think that Unsafe<T> is Share. If the compiler thought that Unsafe<T> were indeed Share, then the type Mutex<T> would be Share

[sfackler]

@eddyb Methods on Mutex are only implemented for T: Send so that shouldn't be an issue.

[flaper87]

Unsafe<T> could be special cased now too. However, given the fact that opt-in kinds are coming, I don't it'd be worth it.

I'll make sure to do this as part of the opt-in kinds work.

[nikomatsakis]

FWIW, I've updated the RFC to include text about this.

[larsbergstrom]

With the removal of MutexArc (which previously handled this scenario for us - making something that is not Share into Share), this issue is currently blocking Servo's current Rust upgrade, because we cannot simply replace previous uses of MutexArc<T> with Arc<Mutex<T>>.

cc @Ms2ger

[flaper87]

@larsbergstrom As mentioned in my previous comment, with few changes Unsafe could be made always Share now too. It'll be a couple of weeks before the opt-in work is done. This is probably worth discussing in the next meeting.

The issue is already assigned to me, I'm happy to tackle it asap if needed.

[larsbergstrom]

@flaper87 Thanks! I think the biggest things we're hoping for are:

1. a workaround that we can use for now until the Right Thing lands that isn't as ugly as copying over all of the old sync crate
2. making sure that this gets fixed at some point in the future

I appreciate the offer, but don't need to you drop everything you're doing and work on this, especially since it sounds invasive enough that we probably couldn't cherry-pick it and we would like to finish off this Rust upgrade before we start in on yet another.

[sfackler]

@larsbergstrom if you can pay for an extra allocation, I've worked around this issue for now by representing an Arc<Mutex<Foo>> as an Arc<Mutex<*()>> and casting between *() and ~Foo as needed. It's a bit hacky, and you'll need to make sure that there's a destructor that can clean up after it, but it's not too horrible.

[larsbergstrom]

@sfackler Thanks! That seems totally reasonable. We'll just have to make sure to clean it up when the fix lands, which we don't always do :-/