On Sun, Sep 15, 2024 at 3:49 PM Gary Guo <gary@xxxxxxxxxxx> wrote: > > On Fri, 13 Sep 2024 23:28:37 -0700 > Boqun Feng <boqun.feng@xxxxxxxxx> wrote: > > > Hmm.. I think it makes more sense to make `access()` requires `where T: > > Sync` instead of the current fix? I.e. I propose we do: > > > > impl<T, U> LockedBy<T, U> { > > pub fn access<'a>(&'a self, owner: &'a U) -> &'a T > > where T: Sync { > > ... > > } > > } > > > > The current fix in this patch disallows the case where a user has a > > `Foo: !Sync`, but want to have multiple `&LockedBy<Foo, X>` in different > > threads (they would use `access_mut()` to gain unique accesses), which > > seems to me is a valid use case. > > > > The where-clause fix disallows the case where a user has a `Foo: !Sync`, > > a `&LockedBy<Foo, X>` and a `&X`, and is trying to get a `&Foo` with > > `access()`, this doesn't seems to be a common usage, but maybe I'm > > missing something? > > +1 on this. Our `LockedBy` type only works with `Lock` -- which > provides mutual exclusion rather than `RwLock`-like semantics, so I > think it should be perfectly valid for people to want to use `LockedBy` > for `Send + !Sync` types and only use `access_mut`. So placing `Sync` > bound on `access` sounds better. I will add the `where` bound to `access`. > There's even a way to not requiring `Sync` bound at all, which is to > ensure that the owner itself is a `!Sync` type: > > impl<T, U> LockedBy<T, U> { > pub fn access<'a, B: Backend>(&'a self, owner: &'a Guard<U, B>) -> &'a T { > ... > } > } > > Because there's no way for `Guard<U, B>` to be sent across threads, we > can also deduce that all caller of `access` must be from a single > thread and thus the `Sync` bound is unnecessary. Isn't Guard Sync? Either way, it's inconvenient to make Guard part of the interface. That prevents you from using it from within `&self`/`&mut self` methods on the owner. Alice