Clippy arc_with_non_send_sync (seems unjustified)

1

Hello,

I have a clippy warning that is enabled and unfortunately I am unable to understand it.

I have a trait:

pub trait ValueChangeCallback {
    fn on_value_changed(&self, name: &str, value: &Variant);
}

I have another struct:

struct ValueChangeCallbackManager<'a> {
    callbacks: RwLock<HashMap<String, Box<dyn ValueChangeCallback + 'a + Send>>>,
}

I also have another struct:

pub struct Proxy<'a> {
    callbacks_member_change: Arc<ValueChangeCallbackManager<'a>>,
}

Bu, in RustRover on the line where I instanciate the Proxy structure:

let mut proxy = Proxy {
            callbacks_member_change: Arc::new(RwLock::new(ValueChangeCallbackManager::new())),
};

I get:

usage of an `Arc` that is not `Send` and `Sync`
Note: `Arc<ValueChangeCallbackManager<'_>>` is not `Send` and `Sync` as `ValueChangeCallbackManager<'_>` is not `Sync`
Help: if the `Arc` will not be used across threads replace it with an `Rc`
Help: otherwise make `ValueChangeCallbackManager<'_>` `Send` and `Sync` or consider a wrapper type such as `Mutex`
Help: for further information visit https://rust-lang.github.io/rust-clippy/rust-1.91.0/index.html#arc_with_non_send_sync
Note: `-D clippy::arc-with-non-send-sync` implied by `-D warnings`
Help: to override `-D warnings` add `#[allow(clippy::arc_with_non_send_sync)]`

What is the problem with my code ?

Thank you very much in advance for any help

2

I think the problem is here:

Box<dyn ValueChangeCallback + 'a + Send>

The trait object is Send, but not necessarily Sync. As such, your RwLock can't be safely shared between threads, because you can’t take a shared reference to a non-Sync type. Something like this should work, provided your callbacks are safe to access from multiple threads:

struct ValueChangeCallbackManager<'a> {
    callbacks: RwLock<HashMap<String, Box<dyn ValueChangeCallback + 'a + Send + Sync>>>,
}

If you don't need threads at all, clippy tells you an even simpler solution: just switch to Rc<>.

3

Arc only implements Send if the type inside the Arc is Send and Sync. (Arc in std::sync - Rust)
Your type only implements Send, you could make it implement Send by also adding it to the Box<dyn ...>. The lint exists because a Arc that is impossible to send between threads isn't very useful, as you could also just use Rc.

4

Thank you for your answers.

I'm still doubtful about the Sync. Doesn't the RwLock structure make it Sync ?

Thank you very much in advance for any help.

5

Mutex<T>: Send + Sync when T: Send, but since read locks can be acquired from multiple threads at the same time, RwLock<T>: Send + Sync only when T: Send + Sync.

1 Like
6

Mutex<T>: Send + Sync when T: Send

Why only when Send ? (if I understood correctly)

What I meant (in case I wasn't clear enough) was, isn't Sync a bit redundant ? Doesn't RwLock already make it Sync ?

Thank you very much in advance for any help.

7

Brief intuitive explanation before the details below: wrapping a Cell<T> in a RwLock can’t synchronize your access to the !Sync Cell<T>. Being !Sync is somewhat infectious, and the invariants of RwLock aren’t strong enough to handle it. (The invariants of Mutex are stronger, and can provide a Sync wrapper even to a !Sync type.) Additionally, !Send is also somewhat infectious in a different way.

T: Sync only if it’s safe to have simultaneous shared access to a value of T from multiple threads. That is, T: Sync only if it’s sound for multiple threads to have &T references to the same T at the same time.

RwLock<T> cannot force its contents to implement any traits, and it itself needs to implement traits in a sound way. If RwLock<T> were to be Sync, then multiple threads could call RwLock::read on a shared &RwLock<T> reference, since multiple readers are allowed. The resulting read guards can dereference to &T (for suitable elided lifetimes). Thus, if RwLock<T>: Sync, you could get multiple threads to have &T references to the same value at the same time. This is sound only if T: Sync.

The bounds involving Send are because you can use a &Mutex<T> or &RwLock<T> to get a &mut T, and then use mem::replace to take the original T value out of the &mut T, even if the original value came from a different thread. Thus, Mutex and RwLock cannot soundly implement Sync unless T: Send.

Lastly, Mutex<T>: Send and RwLock<T>: Send if T: Send, since Mutex and RwLock are basically wrappers around T. Sending them to a different thread directly sends the T to a different thread, no roundabout locking stuff needed. And when dropped, they drop their inner T values.

8

Thank you for your explanation. I'm sorry but I still have some questions.

wrapping a Cell<T> in a RwLock can’t synchronize your access to the !Sync Cell<T>

I'm sorry but why are you mentioning Cell ?

Being !Sync is somewhat infectious, and the invariants of RwLock aren’t strong enough to handle it. (The invariants of Mutex are stronger, and can provide a Sync wrapper even to a !Sync type.) Additionally, !Send is also somewhat infectious in a different way.

I'm really sorry but I'm not sure to understand what you are trying to explain ?

T: Sync only if it’s safe to have simultaneous shared access to a value of T from multiple threads. That is, T: Sync only if it’s sound for multiple threads to have &T references to the same T at the same time.

If you're explaining what Sync means thank you. But I know what Sync and Send mean.

RwLock<T> cannot force its contents to implement any traits, and it itself needs to implement traits in a sound way.

Doesn't RwLock enforce the content T to be Sync ? Isn't it the "purpose" of RwLock ? Or am I missing something ?

If RwLock<T> were to be Sync, then multiple threads could call RwLock::read on a shared &RwLock<T> reference, since multiple readers are allowed.

Isn't it (apart from write) one of the purpose of RwLock ? (I know that multiple threads can't write (at the same time) contrarily to read). But I'm sorry but I'm still not sure why you are talking about references ?

Thus, if RwLock<T>: Sync, you could get multiple threads to have &T references to the same value at the same time. This is sound only if T: Sync.

You are talking about references (so same address). Isn't Arc the same address ?

The bounds involving Send are because you can use a &Mutex<T> or &RwLock<T> to get a &mut T, and then use mem::replace to take the original T value out of the &mut T, even if the original value came from a different thread. Thus, Mutex and RwLock cannot soundly implement Sync unless T: Send.

I'm really sorry but I miss to see the explanation ?

Lastly, Mutex<T>: Send and RwLock<T>: Send if T: Send, since Mutex and RwLock are basically wrappers around T. Sending them to a different thread directly sends the T to a different thread, no roundabout locking stuff needed. And when dropped, they drop their inner T values.

That I understand with no problem. But (unless mistaken) the problem of this thread seams to be with Sync not Send. But thank you for the precision.

Sorry for all the questions but I really wish to understand.

Thank you for your help.

9

I just mentioned Cell as an example. In retrospect, I think Rc would be an even better / more obvious example. Note that RwLock<Cell<T>> and RwLock<Rc<T>> are perfectly valid and usable types. It’s just that they can only be soundly used from a single thread.

You’re correct that the main purpose of RwLock is to use it with Sync; that’s why there’s a clippy lint against using RwLock<T> with a T such that RwLock<T>: !Sync. Even if someone could use a RwLock<Rc<T>>, it’s probably a mistake.

RwLock<Rc<T>> is still permitted… since you can use it (from a single thread), and there’s no good reason to strictly prohibit it altogether… but it cannot implement Sync, else you could have Rc<T> clones across multiple threads.

RwLock<Arc<str>>, on the other hand, is perfectly fine. Having multiple threads with Arc<str> clones pointing to the same value is fine, since Arc does extra work to ensure that multiple threads can safely use it at the same time; Rc does not do that work, and is always !Sync. Transitively, RwLock<Arc<RwLock<Arc<Rc<T>>>>> is !Sync. That innermost Rc prevents the entire type from being !Sync.

10

Basically just because Arc<T> acts a lot like &T, since it dereferences to &T. For any problems that sharing a &T across multiple threads would cause, those same problems would be caused by Arc<T>.

Tying back to RwLock, it’s sort of the same deal with that; any problems that a &T reference can cause by being simultaneously used by multiple threads also apply to RwLock<T> and Arc<RwLock<T>>.

11

Send is about the soundness of moving a value of T to a different thread. Since sharing a Mutex<T> or RwLock<T> across threads (with Sync) could let you move a T to a different thread (with mem::replace, etc etc), it follows that if Mutex<T> were to be Sync when T: !Send, you could do something you shouldn’t be allowed to do. Therefore, we prohibit you from doing something bad by requiring T: Send for Mutex<T> to be Sync, and likewise for RwLock.

I guess... just to explicitly explain part of my reasoning… all my explanations have been “if X and Y happened, bad thing Z would happen. Therefore we need so-and-so Send/Sync bounds to stop X and Y from happening”.

12

I'm really sorry, but I'm not sure to understand why RwLock<Cell<T>> or RwLock<Rc<T>>. Rc is the non thread safe version of Arc (so !Send) and Cell is in order to get get_mut (so !Sync). But since we have RwLock it's still Sync (if I'm not mistaken). But still I don't understand why not just Arc. Also, more to the subject, why specify Sync when we have RwLock ? I'm sorry but I'm still not sure to understand.

So my question (sorry for being so stupid) is: "what is the problem with my code ? what is the meaning of this lint ?"

I'm really sorry but I'm still not sure I have understood yet.

Thank you very much in advance for any help.

13

I think that’s the crux of the confusion. RwLock<T> is “supposed to be” Sync (in order to actually make good use of it), but it is not always Sync. It’s only Sync when it is sound for it to be Sync, which is a condition that depends on the inner T, and depends on how RwLock<T> exposes access to the T to safe Rust.

Likewise, Arc<T> is only Sync if it is sound for Arc<T> to be Sync for that particular T, and the TLDR is that because of how the Arc<T> can interact with safe Rust, T: Send + Sync is required for soundness.

The lint is saying “Arc is most useful if it is Sync. But you’re using an Arc which cannot be Sync, which looks like a mistake”.

No problem, I’m glad to help (or try to). And there’s nothing wrong with asking dumb questions. There’s still plenty of parts of Rust (never mind other topics) where I end up confidently saying something stupid, which is surely worse than asking something silly.

14

RwLock<T> is “supposed to be” Sync (in order to actually make good use of it), but it is not always Sync. It’s only Sync when it is sound for it to be Sync, which is a condition that depends on the inner T, and depends on how RwLock<T> exposes access to the T to safe Rust.

Thank you. I really didn't know that. But how can you have RwLock and be !Sync ? I'm not sure to understand. Sure, you said it's Sync when "it is sound for it to be Sync", but it still seems contradictory. RwLock and !Sync. But thank you for this knowledge.

So I suppose that by specifying Sync we "force" the Sync trait. And since with RwLock it's not always Sync (as you indicated) it then makes sense to specify / force the Sync trait. And, RwLock seems to be Sync only if T is Sync (RwLock in std::sync - Rust). If I understood correctly it seems contradictory. For me, RwLock is to make T Sync (even if it isn't)

The lint is saying “Arc is most useful if it is Sync. But you’re using an Arc which cannot be Sync, which looks like a mistake”.

Sure, the main "purpose" of Arc is to be shared. The main purpose is not to be Sync (not the same thing). So this part "Arc is not Sync" is quite "obvious" to me (Sync and Arc are not the same thing).

No problem, I’m glad to help (or try to). And there’s nothing wrong with asking dumb questions. There’s still plenty of parts of Rust (never mind other topics) where I end up confidently saying something stupid, which is surely worse than asking something silly.

Thank you again for your time and help

15

RwLock is for the purpose of sharing possibly-mutable access to a T. Even if T: Sync, you cannot normally mutate a value from multiple threads without using unsafe. (You would only be able to share Arcs, references, or similar things across the threads, not &mut references). Shared mutability is the utility that Mutex and RwLock provide. If a type is extremely un-threadsafe, they might not be able to help with that.

Fair. Arc<T> can still can be useful in generic contexts where you don’t know whether or not T: Send + Sync, but if clippy knows for sure that your choice(s) of T will make the Arc !Send + !Sync (such as if the Arc wraps a concrete type that clippy can check), then Arc<T> has a slight amount of pointless overhead compared to Rc<T> and provides no advantages over Rc<T>; that’s the rationale for the lint.