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

Thin Wrapper for Owned Traits #70

Draft
wants to merge 11 commits into
base: main
Choose a base branch
from
Draft

Conversation

parastrom
Copy link
Contributor

Status: WIP, not yet ready for merging.
Request: Generally just wanted other eyes on it
Summary of Changes:

  • Introduction of futures Feature: This new feature introduces support for async-std and any other runtime that relies on futures_io::AsyncRead/Write.
  • New Traits: rt::Read and rt::Write: These traits serve as thin wrappers around tokio::io::AsyncRead/Write and any type that implements futures_io::AsyncRead/Write. Maintains tokio as a first class citizen + support for other runtimes
  • Custom ReadBuf Future Implementation for futures_io::AsyncRead: Due to differences in API surface between futures_io::AsyncRead and tokio::io::AsyncRead, I made a custom impl - (Soundness/Safety guarantees are still up for debate)
  • Async fn: The new traits return an impl std::future::Future<Output = _>. Initial attempts to declare them as async fn were unsuccessful, but this allows them to be used across runtimes.
  • Testing and Examples: All tests and examples within the tests directory and examples have been successfully executed, both with and without the feature flag via cargo run --feature futures --example <example> or cargo test --feature futures. The only exception were the documentation tests, which I didn't want to change but don't see why they wouldn't work.
  • Dependency Update: The only new dependency introduced is [email protected], for [dev-dependencies] we tested with async-std and futures_rustls . async_std needed the unstable feature flag for async-std for task::spawn_local and the atrributes flag for async_std::main.
  • Also bumped webpki-roots from 0.23.0 -> 0.25.2

Troubles

In src/handshake ran into a problem where I couldn't change the trait bounds to rt::Read/Write - believe the full solution there would be to implement in full a AsyncRead/Write interface - similar to what hyper did with their traits here, but I just wanted to get something up and running. Something like hyper's is probably more ideal, and should be the target. ATM I'm implicitly targeting Glommio as io_uring library of choice here since it impls future_io, so easier to work with + they've shown that they're capable of working with hyper here - but if this PR ends up with something similar to Hyper's traits then it'll probably end up with the main library changing to runtime agnostic interfaces and then writing runtime specific compat layers gated by feature flags so extensions would be possible.

Want to test with Glommio' since their IO impl futures_io::AsyncRead/Write since async-std works okay so far

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant