Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Cluster: fix shared handles on Windows #8020

Closed
wants to merge 1 commit into from
Closed

Cluster: fix shared handles on Windows #8020

wants to merge 1 commit into from

Conversation

orangemocha
Copy link
Contributor

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.

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

cc @piscisaureus

@bnoordhuis
Copy link
Member

Seems like an uncontroversial change.

@indutny
Copy link
Member

indutny commented Jul 31, 2014

Indeed, landed in 4e68a28 ! Thank you @orangemocha

@orangemocha
Copy link
Contributor Author

This change was reverted. It needs a libuv update.

@orangemocha orangemocha reopened this Aug 2, 2014
@orangemocha
Copy link
Contributor Author

Landed in 7ca4fa5

@orangemocha orangemocha closed this Aug 8, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants