tokio-core icon indicating copy to clipboard operation
tokio-core copied to clipboard

Readable includes hup, but writable doesn't

Open cramertj opened this issue 8 years ago • 10 comments

I've spent quite a while looking at this code and trying to understand it, but I'm failing. poll_read checks the bits included in read_ready, which includes hup. poll_write, however, doesn't include hup-- it just checks the writable signal. Why is this? IMO it's weird that poll_read is ready on hup, but I understand the idea is to push users to attempt a read and then read the remaining data before discovering that the FD is closed. Additionally, since mio makes no guarantees about sending incorrect signals, you can't guarantee that seeing a hup means that the FD is closed. However, I'd expect the same things to apply to writing (not just reading). What was the logic here?

cramertj avatar Oct 30 '17 20:10 cramertj

My own personal understanding of the hup signal is that it sort of happens sometimes only on Unix. IIRC The "sometimes" is that not all types of fds (files, sockets, pipes, etc) will generate a hup signal. I then am only aware of hup translating to "the remote end has hung up, there is no more data to read". That, I believe, is basically a signal for EOF (read returning 0)

I'm unaware of how hup signals might affect write readiness. (or more notably the write function). the general idea here was that if you're blocked on waiting for an object to become readable then it's our responsibility to forward that information to you, and it's not just "has read readiness" apparently but soemtimes this includes "hup readiness" to unblocked the reading task.

alexcrichton avatar Oct 31 '17 17:10 alexcrichton

The issue I have is that you almost never only want to be awoken only on a read signal or a write signal-- in every case I can imagine, you'd always want to be awoken if the thing you were waiting to read from or write to has closed so that you can perform some garbage collection, issue an error, etc.

cramertj avatar Oct 31 '17 17:10 cramertj

Is there a situation where something blocks today when it should wake up? If so that's probably just a bug that needs to be fixed?

alexcrichton avatar Nov 01 '17 15:11 alexcrichton

Well, in order for Fuchsia to get notifications of closed signals I've had this section patched in our tree for a while to return CLOSED signals from the hup function. However, this means that poll_read returns Ready(()) when the channel is closed, not just when it is readable. Additionally, need_read unsets the CLOSED bit, but need_write doesn't. The fix in order to get the signals working is to use poll_ready manully and be aware of all of these different rules. It's not insurmountable by any means, but it does seem like there's a good deal of extra logic going on here that I don't understand the motivation for. My primary goal with this issue is to understand why "read" and "write" are asymmetric.

cramertj avatar Nov 01 '17 16:11 cramertj

Hm ok so today the only reason read/write are asymmetric are so that you can split and then use the two halves as you would normally otherwise do. Handling of hup/closed/etc are then just enabling those use cases

alexcrichton avatar Nov 01 '17 22:11 alexcrichton

Sorry, I don't follow why split makes it so that you only want poll_read to see the closed signal, but not poll_write. Can you explain more? BTW, I'm happy to close this issue and switch to IRC if there's good motivation here I'm just not understanding. I will say, though, that as someone trying to use the tokio-core API to build their own futures types, this inconsistency has caused confusion and led me to make mistakes.

cramertj avatar Nov 02 '17 03:11 cramertj

Er sorry I think this is all being read into a little too much. We have a split function and we have general I/O objects and the goal is to make them "just work". That is, when you Read and get WouldBlock (or write) it's tokio's job to wake you up when you're otherwise ready to go. Everything after that is just an implementation detail. On Unix it just so happens (I believe) that hup wakes up readers and not writers. If that's not true for Fuchsia though then it's fine to tweak.

alexcrichton avatar Nov 06 '17 15:11 alexcrichton

hup wakes up readers and not writers

Right, I'm asking why a hup makes poll_read return Async::Ready-- why does poll_read check for readable | hup, rather than readable? The answer that I concluded from reading usages of poll_read is that it's used to signal when a socket it closed, so that a read will be attempted and the closed socket will be discovered. Is that not correct? If it is correct, why does that signal not also result in poll_write returning Async::Ready?

cramertj avatar Nov 09 '17 02:11 cramertj

TL; DR: why do this and this not match?

cramertj avatar Nov 09 '17 02:11 cramertj

This may be a case where it's good to play around? You can try removing hup from the readable bit and I believe a test should fail.

alexcrichton avatar Nov 10 '17 09:11 alexcrichton