unix: set non-block mode in uv_{pipe,tcp,udp}_open #134
Conversation
The contract specifies that the file descriptor should already be in non-blocking mode before passing it to libuv. However, node users don't really have an opportunity to do so, never mind the fact that the call to uv_pipe_open() or uv_tcp_open() is an implementation detail that most users won't be aware of. Let's be nice and set the non-blocking flag explicitly. It's a cheap operation anyway. Fixes: #124
The mode argument is an enum now and the signedness of an enum is implementation-defined when it doesn't have negative members. Cast it to int in the comparison to tty->mode because the latter is still an int.
|
Thanks, Ben! I'll add some versionchanged thing to the docs and update some tests a bit later then. |
|
@bnoordhuis can you PTAL at this? saghul/libuv@libuv:v1.x...do_you_even_open Pay special attention to the change to test-spawn.c, we used to set the fd to non-blocking before spawning a child process, it would now be after. Also, I wonder if we should do the same for poll handles, WDYT? Windows doesn't do it either. /cc @piscisaureus |
|
For windows:
|
|
Why doesn't it matter for poll handles? I see we do put the fd as
|
The mode argument is an enum now and the signedness of an enum is implementation-defined when it doesn't have negative members. Cast it to int in the comparison to tty->mode because the latter is still an int. PR: #134 Reviewed-by: Saúl Ibarra Corretgé <[email protected]>
|
I added another patch for poll handles. PTAL: saghul/libuv@libuv:v1.x...do_you_even_open |
The problem with uv_pipe_open() arises because libuv itself tries to do a nonblocking operation, but the fd is not in nonblocking mode. With uv_poll() we don't actually do any nonblocking stuff, we just do epoll-like things for the user. The test must set SOCK_NONLBLOCK because it does nonblocking sends and receives. |
I'm a bit confused :-/ AFAIK, all uv_{pipe, tcp, udp}_open work correctly on Windows. Apparently there are tricks on uv_pipe_open but It Just Works (TM), right? Now, on poll handles the user needs to set the fd to non-blocking mode manually, which is what one of the patches here solves, for consistency with the other functions where the user doesn't need to do it. Is there anything I'm missing? Thanks! |
|
Had some discussion with Saùl on IRC. |
The contract specifies that the file descriptor should already be in non-blocking mode before passing it to libuv. However, node users don't really have an opportunity to do so, never mind the fact that the call to uv_pipe_open() or uv_tcp_open() is an implementation detail that most users won't be aware of. Let's be nice and set the non-blocking flag explicitly. It's a cheap operation anyway. Fixes: #124 PR: #134 Reviewed-by: Saúl Ibarra Corretgé <[email protected]>
|
As discussed with @piscisaureus on IRC, the PR landed, I'll take the poll handles thing to another PR. |
R=@saghul
I didn't have time to write a test. Feel free to steal this PR and put your own changes in.