Design decisions behind having poll_close in AsyncWrite?

[sirhcel]

TL;DR

What are the design decisions behind having poll_close in AsyncWrite and not in a separate trait like AsyncClose? I'm wondering because close is not part of std::io::Write which does not mandate making closing explicit and allows "close on drop" like std::fs::File

Background Story

First and foremost, thank you very much for this crate! We are currently evaluating adding async support to serialport-rs with futures-rs and I really like how this bridges to smol and tokio which we use as initial targets for our PoC.

Today, I started the discussion on why the async variant of a serial port needs to be closable where the already existing sync variant handles this as "close on drop" like for example std::io::File. I learned that closing is part of AsyncWrite which requires to deal with the concept of closing in the async serial port as well.

Having only little work done with async I/O, smol, and tokio so far, I would have it expected to be separate from AsyncWrite. The AsyncRead trait does not support closing and so I would naively have it expected in a trait like AsyncClose. The latter would have allowed to keep the async variant of a serialport "close on drop" as well. So I'm really wondering and interested in the design decisions behind having poll_close in AsyncWrite?

[stbuehler]

This is just my interpretation of why things are this way, but maybe that is already good enough for you :)

The "close on drop" shouldn't block, at least not with async resources; and certain proper shutdown operations (e.g. TLS connections, but even normal TCP connections) need to wait for network events and can't be done in "close on drop". (It still should release resources on drop of course, but those aren't proper shutdowns. Some might spawn separate shutdown tasks on drop, but I wouldn't do that.)

While shutdown supports "closing read end" with SHUT_RD and SHUT_RDWR (which doesn't do anything useful for TCP afaik, you could just close the socket; unless multiple dups of the socket still exist and you can't close them all reliably), closing is usually associated with "send EOF and hope the other end does too" - and even if a proper "close connection now" is supported by some protocol this is usually associated with the "writing" side of it.

This has deeper meaning as well (and I don't know whether this is documented somewhere): if you have something that supports AsyncWrite and AsyncRead, you really should support that one task does the writing and one does the reading (using shared+locked ownership, e.g. futures::lock::BiLock<...> or Arc<futures::lock::Mutex<...>>) - that is your underlying IO needs to be able to register two waiting tasks - one for reading, one for writing (but not more than one per operation! The last registered task is used, i.e. the last one that polled, usually implemented with something like futures::task::AtomicWaker). Using a separate task for closing would be overkill, so you need to put it into one of the existing categories, and it makes sense to add it to the corresponding trait as well.

In simple protocols your "wrapper" can usually use the reader/writer task registration from some underlying primitive (e.g. tokio::net::TcpSocket or tokio::io::unix::AsyncFd), but if a send might require reading first you need to do your own task registration and handling (and you might have to spawn additional background tasks to handle the IO in the background).

tokio::io::unix::AsyncFd actually has some words on tasks:

• When using the poll-based API only one reader and one writer task are supported: https://docs.rs/tokio/latest/tokio/io/unix/struct.AsyncFd.html#use-with-to-a-poll-based-api
• Using readable() and writable() you can create new futures, where each of the returned futures can wakeup a single task, and you can have multiple tasks waiting for one event. (The event registration is dropped when the future is dropped, which can't be done in a poll-based API.)

Other implementations:

• tokio uses AsyncWrite::poll_shutdown for this - i.e. part of AsyncWrite
• smol imports AsyncWrite from futures

There is also the async_drop (nightly) feature - if this actually comes one day you'd probably just call your close/shutdown from there too (it wouldn't replace the existing API). (But to be honest: I don't like implicit await points, not sure I actually want that feature.)