Remove the T : Send bound for SyncWrapper to be Sync

[danielhenrymantilla]

Indeed, it is not necessary: Inner : Send => Wrapper : Sync is only required for Wrappers allowing &Wrapper -> &mut Inner code paths such as Mutex or, less obviously, Arc

[rkuhn]

Thanks for the PR @danielhenrymantilla and sorry for the long delay! (I now have notifications on for this repo — d’oh)

I don’t yet understand your reasoning why Send shouldn’t be necessary, perhaps we evaluate this question in different contexts. The SyncWrapper only says that &mut is Sync, it doesn’t say anything else; it can be used in any context you like. But for something to be Sync it needs to be safely shared across threads. Therefore, if something cannot be sent to another thread, it may not be safe to access it from that other thread at all (although there certainly are cases where only the ownership transfer is problematic — but Rust doesn’t distinguish these cases). So basically, it boils down to the question of why T isn’t Sync in the first place:

• it could be lacking synchronisation ⇒ SyncWrapper fixes that
• it could be tied to a thread ⇒ SyncWrapper cannot fix that

Since we don’t know, the best we can do is to take T: Send as evidence that the second case can be ruled out.

Does this sound waterproof to you?

[danielhenrymantilla]

But for something to be Sync it needs to be safely shared across threads. Therefore, if something cannot be sent to another thread, it may not be safe to access it from that other thread at all.

Emphasis on that may: the whole point of Sync and Send is to decorrelate/untangle one another. There are two kind of cross-thread access at play here:

• exclusive access: T / &mut T. For it to be safe, T : Send.

• shared access: &T. For it to be safe, T : Sync.

So, if something is not Send, means neither T nor &mut T access ought to be able to cross the thread boundary. Given your new type wrapper around it –let's call it W<T> to keep it short–, the only way to get T or &mut T out of a W<T> is through a W<T> or &mut W<T>. That is, if T is not Send, then W<T> cannot be Send indeed. And this is automatically derived by the compiler by virtue of auto-traits work, and T being one of the fields (the only one!) of W<T>.

But here we are dealing with whether W<T> : Sync. That is, we offer &W<T> across threads. For many wrappers, this means offering &T across threads, this is what requires to transitively forward the Sync bound, but the point of this very wrapper is to forbid such access, thus making it un-necessary. But, as you can see, there is no Send at play here.

The only times where you will see <T : Send> Sync for W<T> impls, is when the wrapper somehow manages to feature a &mut T access out of a &W<T> access,

• almost always because an UnsafeCell is involved, e.g., Mutex<T>, RwLock<T>;

• the other times, because shared ownership is involved: Arc<T>: it makes sense for such a wrapper to conflate Send and Sync since shared ownership conflates sharing and (traditionally unique) ownership.

  • Why the bound for Arc<T>

    

But neither is the case for SyncWrapper: SyncWrapper offers no &_-based API whatsoever, it is thus literally impossible to do anything with a &SyncWrapper<_>, a fortiori, it is impossible to cause unsoundness with it.

Hence why the Send bound is not needed 🙂

[rkuhn]

Ah, I see — which casts SyncWrapper in a new light for me. If there exists code that works with SyncWrapper but not without, then that code places unnecessary Sync restrictions that it never makes use of. So in the end SyncWrapper only exists due to library design bugs.

[rkuhn]

Thanks a lot for the PR and the clear explanation!

[danielhenrymantilla]

If there exists code that works with SyncWrapper but not without, then that code places unnecessary Sync restrictions that it never makes use of. So in the end SyncWrapper only exists due to library design bugs.

Yeah, I'd say that the use case for SyncWrapper is a bit contrived, but does happen in practice:

1. There is some type –let's call it Foo– which is not Sync / safe the share across threads, potentially because it has a non-Sync field such as a Cell<usize> counter.

2. But such a type also happens to feature a standalone useful &mut-based API.

   That's when wrapping the type in a SyncWrapper is useful: by wrapping Foo in it, you can easily create the "subset type" featuring that useful API without losing Sync-ness for a non-threadsafe API you won't use: SyncWrapper<Foo>.

3. You can then define your own type –say, Bar– having SyncWrapper<Foo> as one of its fields, and then some other harmless / simple type that you may wish to feature shared access to, such as a String field –say, name: String.

4. Then, within an async body, if a &Bar happens to cross an .await point because you'll be accessing that .name field, you won't have that "Future is not Send because &Bar is not Sync because foo is not Sync" error 🙂

mod some_lib {
    #[derive(Default)]
    pub
    struct MonotonicCounter {
        value: ::core::cell::Cell<u64>,
    }

    impl MonotonicCounter {
        pub
        fn next_mut (self: &'_ mut Self)
          -> u64
        {
            let at_value = self.value.get_mut();
            let ret = *at_value;
            *at_value = ret.checked_add(1).expect("Overflow");
            ret
        }

        pub
        fn next (self: &'_ Self)
          -> u64
        {
            let ret = self.value.get();
            self.value.set(ret.checked_add(1).expect("Overflow"));
            ret
        }
    }
}

use some_lib::MonotonicCounter;
use sync_wrapper::SyncWrapper;

struct MyType {
    name: String,
    monotonic_counter: SyncWrapper<MonotonicCounter>, // opt-out of `Cell` capabilities
}

async fn print_name (it: &'_ MyType)
{
    /* stuff… */
    println!("{}", it.name);
}

fn _assert_is_send (it: &'_ MyType)
{
    let fut = print_name(it);
    let _: &dyn Send = &fut; // OK
}

fn next (it: &'_ mut MyType)
  -> u64
{
    it.monotonic_counter.get_mut().next_mut()
}

• Playground

---

Now, you may say that a SyncWrapper wrapping a non-Send type is thus pretty useless, thread-wise, since unique references to it cannot cross the thread-boundary, and while shared references can, there is nothing useful you can do with them! 😅 So this whole PR was rather about a theoretical nitpick rather than an actually useful situation… But when unsafe code is concerned, I believe that examples such as this lib can be useful for teaching purposes, and in that case getting the bounds 100% right will help other people understand these nuances 🙂