async-std and futures-rs

[kpp]

Hi all,

There are a lot of combinators copypasted from https://github.com/rust-lang-nursery/futures-rs/.

I would like to know your thoughts on further reducing the code in async-std in order to ease the maintenance and mitigate potential security risks.

[skade]

Hi @kpp, I'm sympathetic to de-duplicating that at some point, but we currently don't re-export from futures-rs in general because their stability guarantees are different and things are in flux. Once they commit on interfaces, that might change.

I'd be interested which security risks you see in combinator implementations?

[kpp]

I'd be interested which security risks you see in combinator implementations?

async-std/src/io/copy.rs

Line 56 in 46f0fb1

fn project(self: Pin<&mut Self>) -> (Pin<&mut R>, Pin<&mut W>, &mut u64) {

By copying not only combinators but the unsafe code generated by pin_project macros.

[laizy]

@kpp, I think this function is safe, since W has Unpin constraint, and the field reader owns R. if R is !Unpin, then CopyFuture is !Unpin. So we can not get &mut CopyFuture from Pin<&mut CopyFuture> to move out the reader field, we also can not get &mut R from Pin<&mut R>. please correct me if I am wrong.

[yoshuawuyts]

@kpp our position on this hasn't changed since you last raised this a few weeks ago. You can read back the discussion here: #123 (comment)

To re-iterate: async-std currently doesn't use the futures-preview crate, and we don't intend to either. We rely on futures-core-preview and futures-io-preview to provide ecosystem compatibility, and have no intention of changing our approach at this time.

Going to go ahead and close this since we've made our position clear on this.

[kpp]

@yoshuawuyts actually your position is not clear to me. I can try to wrap the existent combinators into your stable api. So your api won’t change but it will reduce the inner code and copypaste a lot by using combinators from futures-rs. Would it be acceptable to you?

[yoshuawuyts]

@kpp No thank you. async-std currently doesn't use the futures-preview crate, and we don't intend to either. We rely on futures-core-preview and futures-io-preview to provide ecosystem compatibility, and have no intention of changing our approach at this time.

[kpp]

May I ask the reason why you don't intend to use futures-preview?

[skade]

Because the name of the library is preview and the communicated state is alpha. The stability guarantees of futures-preview don't match ours and re-exporting futures-preview types is a stability hazard. This is both to allows the futures-preview team to do decisions without us, as much as the other way around. We are in close touch and track changes on both sides.

That may change when futures-rs stabilised, in which moment we'll also reconsider our position.

[kpp]

Thanks! That’s what I wanted to know.