Add thread-safety where possible

[dureuill]

trafficstars

@steffahn wrote:

With the new implementation, thread-safety shouldn't really be any problem - essentially adding the right manual Send and Sync implementations should be all that's necessary.1

However, the API currently contains API for the erased dyn Future<Output = Never> type, and with thread-safety implemented, dyn Future<Output = Never> + Send would become a viable alternative, too. (With relaxations for Sync, a + Sync would never be useful, I believe.)

This is both in the form of the current default F = dyn Future<Output = Never>, and with the new_erased method that produces dyn Future, not dyn Future + Send.

Solutions to this include

• one could change the default to F = dyn Future<…> + Send or keep it as-is, or remove it and offer two type synonyms, say DynScope<T> = BoxScope<T, dyn Future<Output = Never>> and DynScopeSend<T> = BoxScope<T, dyn Future<Output = Never> + Send>. (I don't like these names, they're just place-holders for now.)

• one could add an additional method so we have new_erased and new_erased_send producing the respective erased types

  • alternatively, there could be a single method to produce these different types, using a trait and type inference at the use-site
  • alternatively and/or additionally, it's also possible to just create From implementations. These could offer all kinds of conversions, most notably from F: TopScope<Output = Never> or F: TopScope<Output = Never, Future: Send> to the respective erased BoxScope types. I have not yet checked how Rust’s coherence checks would like additional conversions, e.g. also from BoxScope<T, F> to the erased BoxScope<T, dyn Future<…>+…> types, and from BoxScope<T, dyn Futuer<…> + Send> to BoxScope<T, dyn Future<…>>.

Footnotes

1. For BoxScope<T, F>, the Send and Sync bound would in principle need to consider the two constituent types F and (for<'a>) <T as Family<'a>>::Family. However, Sync might be further relaxed (à la SyncWrapper), as the future (and with current API also the &mut <T as Family<'_>>::Family cannot be accessed unless through &mut selfAPI. ↩

This issue has the purpose of adding some kind of thread-safety to nolife, continuing the discussion initiated in #15

[dureuill]

My take on thread-safety from an API perspective

1. I don't think relaxing Sync is very interesting, and it might force otherwise avoidable breakage in the future if we ever add an operation that gets the value from a shared reference. Interested parties can make use of SyncWrapper (thank you for the link btw, I didn't know there was a crate for that)
2. I think that the general case is probably to have a scope that is Send, and so the default dyn version should add + Send + Sync to the bound. We could then add an additional new_local_dyn constructor (blanket name) to relax the additional bounds.

alternatively, there could be a single method to produce these different types, using a trait and type inference at the use-site

Something like:

    pub fn new_dyn<B: ErasedBoxScope, S: TopScope<Family = T>>(scope: S) -> B
    where
        S::Future: 'static,
    {
        let this = mem::ManuallyDrop::new(BoxScope::new(scope));
        ErasedBoxScope::new(this.0)
    }

I feel like the additional trait might confuse users, same about the From implementations.

Current proposal I feel most comfortable with:

1. Add the send and sync impl to BoxScope
2. Change the default parameter for BoxScope to add the Send + Sync bounds on the erased future type
3. Add a new_local_dyn method that returns an erased BoxScope without the bounds on the erased future type

[dureuill]

Implemented as a draft PR (#19), but I get a weird issue when trying to get the type from a function returning an impl Scope: the Send bound on Scope::Future is lost.

[steffahn]

I'll try to look into it today or tomorrow ^^

[dureuill]

Thank you, take your time[^1], I find the soundness proofs to be actually complicated

[^1]: (I myself have been somewhat slowed down by the early access release of a certain game :grinning: )