Future API improvement ideas (thread-safety, async-support, and more)

[steffahn]

trafficstars

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]

[^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.

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<…>>.

---

For async support, really I think all that’s needed is an alternative to the enter method that polls the future with a given cx from outside, and then determines whether or not state was set to either propagate Pending or indicate that an “item” is available. However, this needs a two-step API. The part that awaits the next “item” to become available would be an async fn, but the callback that receives the &mut <T as Family<'_>>::Family via a HRTB needs to be a normal, synchronous callback (for soundness, I believe, and in any case also because HRTB “async closures” in Rust don’t yet really work at all).

The main concern, in terms of API-breakage, then is that the synchronous API might possibly want to mirror the async-supporting API for consistency (and flexibility).

One possible API I’m imagining is to just have a fn go_to_next(&mut self) and a with_mut(&'borrow mut self, impl for<'a> FnOnce(&'borrow mut <T as Family<'a>>::Family) -> Output) -> Output one. (Once again, I’m not sure about naming, these are placeholders.) Where go_to_next executes the contained future to get to the next “item” and with_mut accesses the contained state without advancing. This would the probably panic in case with_mut is called before the first call to go_to_next.

The former could then be amended by an async-supporting alternative async fn async_go_to_next(&mut self). The latter could also support an additional immutable-access with(&'borrow self, impl for<'a> FnOnce(&'borrow <T as Family<'a>>::Family) -> Output) -> Output method.

With such an API change, one might also question the pattern of using freeze_forever. For use-cases that only contain a single freeze_forever, it might be less overhead (polling the Future again, etc..) to instead just call go_to_next once and then work by re-borrowing the same mutable reference with the with_mut method.

API flexibility here is large though, anyways. Additional possibilities include

• allow non-Never return values R and make go_to_next return Option<R> or Result<(), R> or something like that to indicate some “done” state.
• avoid panics in the API

  • either by making with_mut/with skip the callback and return some indication of whether or not a value was present

  • or by making go_to_next return some handle that re-borrows from Self and offers panic-free with_mut/with API

    So: go_to_next(&'s mut self) -> Handle<'s, T> and Handle has non-panicking with_mut and with methods with the usual API, e.g. with_mut(&'borrow mut self, impl for<'a> FnOnce(&'borrow mut <T as Family<'a>>::Family) -> Output) -> Output.

  • these could also be combined for maximal flexibility and no panics, so that next to go_to_next, there's also a view_current(&'a mut self) -> Option<Handle<'a>> API.

---

I also think that possibly the Scope trait could become “nicer” to work with if it had a convenience alias trait that doesn't use associated type but type parameters. This way, signatures like -> impl Scope<Family = ZipReaderFamily, Output = ZipResult<Never>> could become a bit shorter to write: -> impl Scope<ZipReaderFamily, ZipResult<Never>>.

[steffahn]

To be clear, I have not tried implementing any of this yet, so there may possibly be oversights in things that could turn out not possible. Also, feel free to ask for more concrete code examples or signatures for parts of plain text descriptions that turn out not being clear enough.

I'll gladly try to implement these ideas myself and make some PRs; but of course only if (/ only those that) you deem them something you consider possibly interesting to merge in the first place.

[dureuill]

Hello,

sorry for not coming back to you earlier, the week has been very packed.

I saw your suggestions, and we need to discuss them. I don't have the time to go into details right now, but the gist of it as I saw it:

1. We should probably release 0.4 first so as to have a version with the soundness holes plugged
2. I think we should separate the ideas you have here as much as possible:
   1. Thread-safe Scope
   2. async scope
   3. scope you can get_mut without advancing
   4. scope you can get_ref
   5. scope that can be finalized (not returning Never)
3. I think any API separating get_mut without advancing should avoid any possibility for panics. Avoiding the runtime panic that occurs when attempting to poll a "finished" async fn is why the scopes must return Never. I'd like to avoid introducing the same kind of "state machine failure", if possible.
4. I'll need convincing that the scope can be made thread-safe and we'll need to tread carefully. Same for get_ref (variance stuff gets in the way, they are avoided with a &mut T)

Thanks as always for your interest in this crate, it is great to cooperate with you :-)

[steffahn]

Sounds good to me ^^