@yoshuawuyts The problem I was talking about was that, due to AsyncRead's poll-based design, each AsyncRead instance should hold one waker which it sets on poll_read and wakes when the resource becomes readable. However, an &TcpStream cannot hold a waker - the internal TcpStream can, but there may be more references and thus more wakers that need to be registered than the one waker the TcpStream itself holds. And because different &TcpStreams cannot be differentiated, it's not like each stream can store a map of different references to wakers.

Previously I assumed that async-std would just lose wakeups here (hence my comment), but it turns out async-std does avoid that. Because Async::poll_readable will wake the old waker if a new waker is seen but the resource has not yet become readable, two tasks with &TcpStreams (or cloned TcpStreams) competing for the waker slot will get caught spinnning in a cycle of continually waking each other up. This does avoid the wakups getting lost issue, but isn't an ideal solution.

One option is to remove the Clone and "I/O traits on references" APIs altogether, but that is a severe loss of functionality. Another option is to not implement the I/O traits on references, but still keep the Clone based API. This would involve changing Async::readable and Async::writable to return named 'static futures - if we're lucky, we could implement this by just cloning the inner Arc<Source> to make them 'static, but that may cause bugs if readable/writable assume the I/O resource has not been dropped, so that'll have to be investigated. Then TcpStream could store both an Arc<Async<std::net::TcpStream>> and a Readable and Writable. AsyncRead and AsyncWrite would poll the Readable and Writable, and cloning the TcpStream would generate new Readable and Writable futures with their respective methods on Async.

Edit: Ah, I see the async-net does do basically the solution I mentioned above, but it boxes and type-erases the futures. I was sort of considering that to not be an option when writing the above paragraph due to the overhead it would have, but if we switch to async-net anyway it will be fine. And we can always optimize later.

Migrating the net library to async-net? #969

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions