wait_socket_*, notify_socket_close now accept raw socket descriptors #610
Conversation
njsmith
force-pushed
the
fix-400
branch
2 times, most recently
from
42f9e45
to
c6c43df
Compare
Codecov Report
@@ Coverage Diff @@
## master #610 +/- ##
==========================================
- Coverage 99.31% 99.31% -0.01%
==========================================
Files 91 91
Lines 10822 10800 -22
Branches 753 751 -2
==========================================
- Hits 10748 10726 -22
Misses 56 56
Partials 18 18
Continue to review full report at Codecov.
|
| raise TypeError("need a socket") | ||
| notify_fd_close(sock) | ||
|
|
||
| wait_socket_readable = wait_readable |
There was a problem hiding this comment.
Is there any reason for these to be under a different name now?
There was a problem hiding this comment.
excellent question :-)
The reason I did it this way in the first place is that on Windows, there are two totally different things: handles, and file descriptors. They're both integers that refer to OS objects via some entry in a process-local table, but it's two separate tables, so you have to know which kind of integer you have :-). Because of Python's unix heritage, it's sloppy about this distinction – e.g. in the stdlib, file.fileno() returns a file descriptor, but socket.fileno() returns a handle.
Python gets away with this because each operation either interprets arguments as fds (os.read), or as handles (select.select). And in Windows there is no wait to wait for an arbitrary fd to be readable or writable, or even an arbitrary handle to be readable or writable; it's only defined for socket handles.
So... my default for the low-level APIs is to expose the system's capabilities, whatever those are, as directly as possible. And on Unix there's a generic concept of "wait for a thing to be readable/writable", so I defined wait_readable and wait_writable functions, and on Windows there's a socket-specific concept of "wait for socket to be readable/writable", so I defined wait_socket_readable and wait_socket_writable. And then when I implemented trio.socket, I was like hey it's very annoying to have to call one function on Window and a different one on Unix, given that when you're working on sockets they're exactly the same. So I added the *_socket_* functions to Unix too, as a tiny little platform abstraction layer.
Another approach would be to follow Python's lead, and say eh, no-one's ever going to want to wait for readability/writability on an fd on Windows because Windows just doesn't do that, or notify that an fd is closed, so let's pretend that Windows handles are the same as Unix fds and that Windows fds don't exist, and have a single set of functions wait_readable, wait_writable, notify_closed. (And document that on Windows, they only work on sockets.) I'm not super excited about this though because (1) it feels inelegant, (2) there are ways to check if other objects are readable on Windows that wouldn't use wait_readable, e.g. you can check whether the console is readable by doing WaitForSingleObject on a special handle.
At some point it might also make sense at some point to have notify_handle_closed on Windows, that wakes up WaitForSingleObject as well as wait_socket_*? And then make notify_socket_closed an alias for it?
So... you're right that the difference is smaller now. Having written all this out, I guess I'm still vaguely slightly in favor of having the different functions, and if we do decide to deprecate the *_socket_* functions there's no particular reason to do it in this PR, so in conclusion I guess I'll leave this PR as it is for now.
|
Whoops, just realized there's a bug here: on Windows, we store and look up sockets by Python object id, not by fileno, so if one task does |
|
Oh, and that bug is why the appveyor tests aren't running... it's causing the windows tests to freeze, so every appveyor job takes an hour to timeout, so I've totally killed the appveyor queue. Whoops. |
|
Okay, fixed that bug and tests are green. |
Fixes gh-400
CC @Fuyukai