Do not generate artificial "broken pipe" errors

[link2xt]

closed flag is set when there is no more data to read from the stream. It does not mean that no more requests can be written into the stream. If the stream is closed for writing, write_all call will return this error, there is no need to emulate it.

[link2xt]

@flub GitHub suggests you as the reviewer and you have some lines in git blame in this file. The closed variable is introduced by you in 9a7097d

[link2xt]

My logic here is that it should never be forbidden to send another request into the underlying stream. If write() syscall succeeds, then you are allowed to do this.

[link2xt]

This is essentially reverting 68f21e5. So maybe there is a reason not to send requests when we cannot receive responses, then maybe I should write a documentation comment with a reason instead.

[flub]

What's the motivation for this? IIUC TCP closes both directions of the stream at the same time, it is not possible to only close one half. So once you read 0 bytes the stream is closed in all directions. I suspect there's not much harm to do this, but what's the expected gain? I thought this was purely a fast-path for the expected behaviour.

Even if you were able to still write, you'd not be able to read any response since there is already this closed marker forcing the return of EOF (effectively this closed marker makes the ImapStream "fused").

[link2xt]

IIUC TCP closes both directions of the stream at the same time, it is not possible to only close one half.

It is possible to call shutdown(fd, SHUT_WR) on the server. In this case the server will send FIN and the client will interpret it as EOF, but the server can still accept bytes, i.e. it will send ACK rather than RST in response.

There is no guarantee though that async-imap runs over TCP. It can already run over TLS or SOCKS5 connection, and there is nothing preventing usage of the library over QUIC, unix sockets and all kinds of tunnels, so TCP should not be assumed. There are definitely types of streams that can be closed one-way, and there are even streams that can be reopened as described in std::io::Read documentation.

The original reason I made this PR is that during debugging of #66 closed flag was erroneously set, but resulted in a misleading write error while in fact the socket was not closed in any direction. Even with the bug fixed, it may be not the only case where this happens, because there may be other bugs or middleboxes inserting FIN flag.

I will probably redo this PR trying to remove the closed variable completely, because having a variable that tries to track the state of the underlying stream naturally leads to bugs where the state of the variable does not match the state of the underlying stream.

[flub]

I'm still not sure why you'd want to send requests if you can't send responses anymore. But all valid points.

Anyway, the second half of what you say makes a lot of sense: to not manually track if the stream is usable or not so completely removing the closed flag.

I don't really mind this PR itself I guess, the original PR in which I introduce this commit already mentioned that it was optional as it shouldn't really change things (but yes, does change things in the face of bugs).

[flub]

I guess the one reason to maybe keep it in, for now, is that it makes things symmetric: if you stop reading you also stop writing.

[link2xt]

The bug may even be not in our code, but in a middlebox injecting FIN flags. This may make things hard to debug if the user reports a "broken pipe" error, while in fact it is a bug on the server or a network-level attack.

The closed flag was introduced in the parent commit 9a7097d
in attempt to fix some infinite loop, so need to be careful not to reintroduce it when removing the closed variable completely.

I will merge this PR as is now, and complete closed removal can come in later commits because it is more dangerous.

[link2xt]

I am also not sure it is possible to receive EPIPE from a write() syscall on a TCP socket, normally you would get an ECONNRESET once the server gets your request and sends you back an RST.

[link2xt]

I will merge this PR as is now, and complete closed removal can come in later commits because it is more dangerous.

I have investigated further why closed was erroneously set on a non-closed inner stream, and found a bug in ensure_capacity(): #77
So now I want to fix the bug first in the next minor release, then proceed with removing closed variable in next releases.