Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking Issue for Exclusive #98407

Open
3 of 5 tasks
guswynn opened this issue Jun 22, 2022 · 17 comments
Open
3 of 5 tasks

Tracking Issue for Exclusive #98407

guswynn opened this issue Jun 22, 2022 · 17 comments
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@guswynn
Copy link
Contributor

guswynn commented Jun 22, 2022

Feature gate: #![feature(exclusive_wrapper)]

This is a tracking issue for the Exclusive wrapper struct.

This structure can be used to upgrade non-Sync objects into Sync ones, by only offering mutable access to the underlying object. This can be useful with Send-but-not-Sync futures (like BoxFutures)

Public API

// core::sync
#[derive(Default)]
struct Exclusive<T: ?Sized> { ... }

impl<T: ?Sized> Sync for Exclusive {}

impl<T> Exclusive<T> {
    pub const fn new(t: T) -> Self;
    pub const fn into_inner(self) -> T;
}

impl<T: ?Sized> Exclusive<T> {
    pub const fn get_mut(&mut self) -> &mut T;
    pub const fn get_pin_mut(Pin<&mut self>) -> Pin<&mut T>;
    pub const fn from_mut(&mut T) -> &mut Exclusive<T>;
    pub const fn from_pin_mut(Pin<&mut T>) -> Pin<&mut Exclusive<T>>;
}

impl<T: Future> Future for Exclusive { ... }

impl<T> From<T> for Exclusive<T> { ... }
impl<T: ?Sized> Debug for Exclusive { ... }

Steps / History

Unresolved Questions

  • None yet.
@guswynn guswynn added C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Jun 22, 2022
@clarfonthey
Copy link
Contributor

clarfonthey commented Jun 24, 2022

This is probably a bikeshed, but perhaps the get_pin_mut method could be directly implemented on Pin<Exclusive<T>>, so that it can share the same name as the regular Exclusive<T> version? Then maybe both could be named get_mut, or even get since you'll have to use it to access non-mutable methods, just with exclusive access.

This does make it asymmetric with from_pin_mut, though, unless there were a Pin::exclusive method.

@guswynn
Copy link
Contributor Author

guswynn commented Jun 29, 2022

@clarfonthey that would conflict with https://doc.rust-lang.org/std/pin/struct.Pin.html#method.get_mut

@Nemo157
Copy link
Member

Nemo157 commented Oct 20, 2022

To provide some motivation I have been using a more limited internal version of this for about 3 years in async-compression. This is the sole bit of unsafety in that crate so I would love to be able to offload that onto std.

@guswynn
Copy link
Contributor Author

guswynn commented Oct 29, 2022

@Nemo157 I think we should try to get this in at 1.66, next week ill take a look at writing a stabilization report

@ghost ghost mentioned this issue Nov 6, 2022
@ghost
Copy link

ghost commented Nov 6, 2022

Hi! I've submitted PR #104057, with a few small changes:

  • Added rustc_const_unstable attributes on the const fns.
    • into_inner is gated under const_exclusive_into_inner, and is blocked on const drop.
    • {get,from}_{mut,pin_mut} is gated under const_exclusive_get_mut, and is blocked on const_mut_refs.
  • The From impl is now unstably const under const_convert, for consistency with the rest of the standard library.
  • Exclusive<G> now implements Generator, gated under generator_trait.
  • Exclusive<F> now implements FnOnce & FnMut.

I would be happy to write a stabilization PR when this passes an FCP.

@zachs18
Copy link
Contributor

zachs18 commented Nov 13, 2022

For the same reason that Exclusive can unconditionally implement Sync, I believe it can also unconditionally implement core::panic::RefUnwindSafe (which claims that a shared reference to the type is unwind-safe (i.e. can cross a panic boundary without introducing logic errors), which &Exclusive is, because it has no API).

I don't know if it would be that useful (even ignoring that there's been some disucssion about deprecating (Ref)UnwindSafe), but I believe it could be added after stabilization if it is wanted, so it's not urgent or blocking.

@harudagondi
Copy link
Contributor

Interestingly, in bevyengine/bevy#7718, they are adding a method called read that allows shared access to their Exclusive analogue as long as the inner type implements Sync.

@alice-i-cecile
Copy link

Q: What's the use case for a SyncCell when the type is already Sync?
A: Generics, mostly. When designing an API you might not know if a type parameter is Sync or not, so you'd put it in a SyncCell. Then when you use that API with a concrete type, you can bypass the exclusivity if that type is Sync

The method above is quite useful for us, but is a contradiction with the Exclusive name of this type. As a result, we'd like to suggest renaming the type to something compatible, and adding the method linked in the PR above.

This is quite a useful type, and it would be a shame to see something limited get locked in via stabilization.

(summarized from conversation in the Bevy Discord).

@zachs18
Copy link
Contributor

zachs18 commented Feb 18, 2023

I agree that SyncCell or something like it could be a better name than Exclusive in the prescence of a fn read(&self) -> &T where T: Sync-like method, but I don't necessarily think that Exclusive being a contradictory name/having an exception for Exclusive<T: Sync> would be too bad. There's somewhat of precendent with Pin<Pointer>, which has an "exception" when Pointer::Target: Unpin, and indeed has additional API in that case, e.g. Pin::get_mut.

@JoJoJet
Copy link
Contributor

JoJoJet commented Feb 19, 2023

I don't think Pin is a the best comparison here. It makes total sense that the Unpin trait would negate Pin, for obvious reasons. However, it does not make sense that Sync would negate Exclusive, since those concepts are orthogonal. If shared access is going to be supported for this type, I don't think it makes sense to keep the current name.

@clarfonthey
Copy link
Contributor

The main issue with using the Cell terminology I can see is that Exclusive doesn't use UnsafeCell internally, whereas all other forms of Cell do. In some sense, this actually makes it a sort of anti-cell; cells let you perform writes with immutable references, whereas Exclusive prevents you from performing reads with immutable references.

What we have is some sort of static mutex, but I'm not sure if using that terminology is helpful either.

@JoJoJet
Copy link
Contributor

JoJoJet commented Feb 20, 2023

I agree that the Cell terminology isn't great, since this type doesn't use interior mutability.

I think the name should emphasize the fact that the purpose of this type is to gurantee Sync-ness. A name along the lines of SyncSafe<T> would be ideal IMO -- this describes the purpose of this type, and doesn't make shared access contradictory.

@Nikita240
Copy link

We should just name it Iceberg since the type is used to Sync the Unsyncable.

@Xuanwo
Copy link
Contributor

Xuanwo commented Dec 8, 2023

Is it possible (and how) to make Exclusive implement traits that only have &mut self APIs? There are two useful cases:

  • Trait in std or 3rd crates, like Read and Write.
  • Trait in users own crates, like MyMutOnlyTrait.

@clarfonthey
Copy link
Contributor

You could also "cheat" by implementing traits that have read-only methods specifically for &mut Exclusive, like for example Iterator could be done with this approach.

@zachs18
Copy link
Contributor

zachs18 commented Dec 8, 2023

You could also "cheat" by implementing traits that have read-only methods specifically for &mut Exclusive, like for example Iterator could be done with this approach.

I don't think that would work? If you have a &self method, where Self = &mut Exclusive, then you have &&mut Exclusive, which you cannot get a &mut Exclusive from.

Well, Iterator specifically could technically be implemented (even for Exclusive, not just &mut Exclusive), since the only &self method size_hint can be implemented conservatively as (0, None) without accessing the iterator (likewise with io::Read/io::Write and is_read_vectored/is_write_vectored which can just return false), but in general this is not true.

Trying to implement <&mut Exclusive as Iterator>::size_hint normally

(Playground link)

impl<I: ?Sized + Iterator> Iterator for &mut Exclusive<I> {
    type Item = I::Item;
    
    fn next(&mut self) -> Option<I::Item> {
        self.get_mut().next() // this is fine
    }
    
    fn size_hint(&self) -> (usize, Option<usize>) {
        self.get_mut().size_hint()
        // cannot borrow `**self` as mutable, as it is behind a `&` reference
    }
}

@clarfonthey
Copy link
Contributor

…right, initially I thought that reborrows of the initial &mut would not violate the exclusive access guarantees, but in hindsight, they do. You're right that we'd have to rely on naïve implementations of size_hint.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-tracking-issue Category: An issue tracking the progress of sth. like the implementation of an RFC T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

9 participants