Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Readable includes hup, but writable doesn't #274

Open
cramertj opened this issue Oct 30, 2017 · 10 comments
Open

Readable includes hup, but writable doesn't #274

cramertj opened this issue Oct 30, 2017 · 10 comments

Comments

@cramertj
Copy link
Contributor

cramertj commented Oct 30, 2017

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?

@alexcrichton
Copy link
Contributor

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.

@cramertj
Copy link
Contributor Author

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.

@alexcrichton
Copy link
Contributor

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?

@cramertj
Copy link
Contributor Author

cramertj commented Nov 1, 2017

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.

@alexcrichton
Copy link
Contributor

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

@cramertj
Copy link
Contributor Author

cramertj commented Nov 2, 2017

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.

@alexcrichton
Copy link
Contributor

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.

@cramertj
Copy link
Contributor Author

cramertj commented Nov 9, 2017

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
Copy link
Contributor Author

cramertj commented Nov 9, 2017

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

@alexcrichton
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants