This repository has been archived by the owner on May 4, 2018. It is now read-only.
windows: relay TCP bind errors via ipc #1384
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Can we have a libuv test case for this? |
| typedef struct { | ||
| WSAPROTOCOL_INFOW socket_info; | ||
| int bind_error; | ||
| } uv_ipc_socket_info_ex; |
There was a problem hiding this comment.
Maybe call it uv__ipc_socket_info_ex as it's an internal thing and we lean towards that naming.
orangemocha
added a commit
to nodejs/node-v0.x-archive
that referenced
this pull request
Jul 31, 2014
This is the Node side of the fix for Node's cluster module on Windows. #7691 The other required part is joyent/libuv#1384 Windows and Unix return certain socket errors (i.e. EADDRINUSE) at different times: bind on Windows, and listen on Unix. In an effort to hide this difference, libuv on Windows stores such errors in the bind_error field of uv_tcp_t, to defer raising it at listen time. This worked fine except for the case in which a socket is shared in a Node cluster and a bind error occurs. A previous attempt to fix this ( joyent/libuv@d1e6be1 3da36fe ) was flawed becaused in an attempt to relay the error at the JS level it caused the master to start accepting connections. With this new approach, libuv itself is relaying the bind errors, providing for a uniform behavior of uv_tcp_listen. Reviewed-By: Fedor Indutny <[email protected]>
|
Added test case (win32-only), and cleaned up per your suggestions. Thanks! |
| @@ -205,6 +204,70 @@ static void on_read(uv_stream_t* handle, | |||
| free(buf->base); | |||
| } | |||
|
|
|||
| #ifdef _WIN32 | |||
| static void on_read_listen_after_bound_twice(uv_stream_t* handle, | |||
There was a problem hiding this comment.
style: align arguments with the first one.
|
Some style nits, otherwise LGTM. I'll run tests later. |
This is the libuv side of the fix for Node's cluster module on Windows. nodejs/node-v0.x-archive#7691 Windows and Unix return certain socket errors (i.e. EADDRINUSE) at different times: bind on Windows, and listen on Unix. In an effort to hide this difference, libuv on Windows stores such errors in the bind_error field of uv_tcp_t, to defer raising it at listen time. This worked fine except for the case in which a socket is shared in a Node cluster and a bind error occurs. A previous attempt to fix this ( d1e6be1 nodejs/node-v0.x-archive@3da36fe ) was flawed becaused in an attempt to relay the error at the JS level it caused the master to start accepting connections. With this new approach, libuv itself is relaying the bind errors, providing for a uniform behavior of uv_tcp_listen.
|
Amended to address your latest feedback. Thanks! |
|
Thanks Alexis! Extra karma for refactoring the |
orangemocha
added a commit
to nodejs/node-v0.x-archive
that referenced
this pull request
Aug 7, 2014
This is the Node side of the fix for Node's cluster module on Windows. #7691 The other required part is joyent/libuv#1384 Windows and Unix return certain socket errors (i.e. EADDRINUSE) at different times: bind on Windows, and listen on Unix. In an effort to hide this difference, libuv on Windows stores such errors in the bind_error field of uv_tcp_t, to defer raising it at listen time. This worked fine except for the case in which a socket is shared in a Node cluster and a bind error occurs. A previous attempt to fix this ( joyent/libuv@d1e6be1 3da36fe ) was flawed becaused in an attempt to relay the error at the JS level it caused the master to start accepting connections. With this new approach, libuv itself is relaying the bind errors, providing for a uniform behavior of uv_tcp_listen. Reviewed-By: Fedor Indutny <[email protected]>
fdgonthier
pushed a commit
to opersys/node
that referenced
this pull request
Apr 1, 2015
This is the Node side of the fix for Node's cluster module on Windows. nodejs/node-v0.x-archive#7691 The other required part is joyent/libuv#1384 Windows and Unix return certain socket errors (i.e. EADDRINUSE) at different times: bind on Windows, and listen on Unix. In an effort to hide this difference, libuv on Windows stores such errors in the bind_error field of uv_tcp_t, to defer raising it at listen time. This worked fine except for the case in which a socket is shared in a Node cluster and a bind error occurs. A previous attempt to fix this ( joyent/libuv@d1e6be1 nodejs/node-v0.x-archive@3da36fe ) was flawed becaused in an attempt to relay the error at the JS level it caused the master to start accepting connections. With this new approach, libuv itself is relaying the bind errors, providing for a uniform behavior of uv_tcp_listen. Reviewed-By: Fedor Indutny <[email protected]>
fdgonthier
pushed a commit
to opersys/node
that referenced
this pull request
Apr 1, 2015
This is the Node side of the fix for Node's cluster module on Windows. nodejs/node-v0.x-archive#7691 The other required part is joyent/libuv#1384 Windows and Unix return certain socket errors (i.e. EADDRINUSE) at different times: bind on Windows, and listen on Unix. In an effort to hide this difference, libuv on Windows stores such errors in the bind_error field of uv_tcp_t, to defer raising it at listen time. This worked fine except for the case in which a socket is shared in a Node cluster and a bind error occurs. A previous attempt to fix this ( joyent/libuv@d1e6be1 nodejs/node-v0.x-archive@3da36fe ) was flawed becaused in an attempt to relay the error at the JS level it caused the master to start accepting connections. With this new approach, libuv itself is relaying the bind errors, providing for a uniform behavior of uv_tcp_listen. Reviewed-By: Fedor Indutny <[email protected]>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is the libuv side of the fix for Node's cluster module on Windows.
nodejs/node-v0.x-archive#7691
Windows and Unix return certain socket errors (i.e. EADDRINUSE) at
different times: bind on Windows, and listen on Unix.
In an effort to hide this difference, libuv on Windows stores such
errors in the bind_error field of uv_tcp_t, to defer raising it at
listen time.
This worked fine except for the case in which a socket is shared in
a Node cluster and a bind error occurs.
A previous attempt to fix this (
d1e6be1
nodejs/node-v0.x-archive@3da36fe
) was flawed becaused in an attempt to relay the error at the JS level
it caused the master to start accepting connections.
With this new approach, libuv itself is relaying the bind errors,
providing for a uniform behavior of uv_tcp_listen.