Eugeny / russh

Rust SSH client & server library
https://docs.rs/russh
950 stars 106 forks source link

Feature-gate `async_trait` in lieu of new, standardized `async_fn_in_trait` feature. #236

Open Qix- opened 9 months ago

Qix- commented 9 months ago

Would it be possible to put the async-trait stuff behind a feature of the same name, and then use a #[cfg_attr(feature = "async-trait", ::async_trait::async_trait)] for all occurrences, such that we can use the newly stabilized async-in-trait functionality?

Sherlock-Holo commented 9 months ago

for now trait-variant can't handle default methods, but I think if it support, russh can use AFIT directly instead of the async-trait crate

Eugeny commented 9 months ago

Sadly this doesn't work yet as native async trait futures aren't Send and can't be used to spawn tokio tasks: https://blog.rust-lang.org/inside-rust/2022/11/17/async-fn-in-trait-nightly.html#limitation-spawning-from-generics

indietyp commented 9 months ago

That isn't completely true, if you specify the desurgared form in a trait definition you can specify additional bounds (like Send + 'static) in implementations you can still use async fn.

You can see this in action here: https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=ec570d1da8499bb14e80bac0827e233b

Comment out the Rc implementation to see a compilation error. (with RPITIT normal future rules apply, here just using Rc to illustrate the point more clearly)

Eugeny commented 9 months ago

Thanks for the tip! I gave it a try and unfortunately I'm running into https://github.com/rust-lang/rust/issues/110338

If you want to give it a try: https://github.com/warp-tech/russh/pull/244

Qix- commented 9 months ago

As a complete alternative (a large, hairy alternative), it might be a good evolution to implement proper Future types, given that russh is a library. Then all of these issues go away. But that's a massive change, for sure.