windows: Failing simple/test-cluster-bind-twice-v2

[isaacs]

In this test, two child processes are set up. Each of them in turn do cluster.fork().

In the first child process, the worker listens on common.PORT. Once it is listening, it sends a message to its parent, which is relayed to its grandparent (the top-level node process).

When the grandparent receives that message, it sends a message to the second child. The second child then relays it to its cluster worker, which creates a new server, and attempts to listen on the same port.

The intended behavior is that second listen() call gets an EADDRINUSE. However, what's happening instead is that, when the second master process tries to send the created and bound server handle to its cluster worker, uv_write2 returns -1, and thus the worker.send() raises an EINVAL on the worker object in the second parent process.

Why is uv_write2 returning -1, I wonder?

I will continue investigating. If I don't make it back, perhaps @piscisaureus or @sblom will have an idea of where to keep searching.

[isaacs]

It seems like this might be related to what @sblom found in #4959, that we're doing the tcp_endgame before it's actually time to do so. That could be shutting down the child process IPC channel, which would totally explain this.

[tjfontaine]

After further investigation, the call stack for uv_write2 returning -1 is

• uv_pipe_write2
• uv_pipe_write_impl
• uv_tcp_duplicate_socket

where if (listen(handle->socket, SOMAXCONN) == SOCKET_ERROR) { is hitting

[tjfontaine]

ok, so master2 creates the socket and attempts to bind it fails with EADDRINUSE and sets handle->bind_error but we delay the handling of that explicitly

If we set the sys_error to the bind_error this won't fix the test, because the EADDRINUSE will raise in the parent, not in the child -- you won't be able to pass the socket to the child at all.

[tjfontaine]

The following makes the test pass but is less than ideal as a socket simply isn't being sent to the child, and relying on an failure in tcp_wrap for getsockname

diff --git a/deps/uv/src/win/tcp.c b/deps/uv/src/win/tcp.c
index c3ef653..3714a1e 100644
--- a/deps/uv/src/win/tcp.c
+++ b/deps/uv/src/win/tcp.c
@@ -1223,7 +1223,7 @@ int uv_tcp_duplicate_socket(uv_tcp_t* handle, int pid,
      * with another process, we call listen here in the parent process.
      */

-    if (!(handle->flags & UV_HANDLE_LISTENING)) {
+    if (!(handle->flags & UV_HANDLE_LISTENING) && !(handle->flags & UV_HANDLE_BIND_ERROR)) {
       if (!(handle->flags & UV_HANDLE_BOUND)) {
         uv__set_artificial_error(handle->loop, UV_EINVAL);
         return -1;
diff --git a/lib/net.js b/lib/net.js
index 6174b40..b4565f2 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -1046,9 +1046,12 @@ function listen(self, address, port, addressType, backlog, fd) {
     // not actually bound. That's why we check if the actual port matches what
     // we requested and if not, raise an error. The exception is when port == 0
     // because that means "any random port".
-    if (port && handle.getsockname && port != handle.getsockname().port) {
-      self.emit('error', errnoException('EADDRINUSE', 'bind'));
-      return;
+    if (port && handle.getsockname) {
+      var sname = handle.getsockname();
+      if (!sname || port != sname.port) {
+        self.emit('error', errnoException('EADDRINUSE', 'bind'));
+        return;
+      }
     }

     self._handle = handle

[bnoordhuis]

FYI, simple/test-cluster-bind-twice-v2 has been renamed to simple/test-cluster-bind-twice in master as of 872e720, with some unhiding of child process errors in 45ed546.

/cc @piscisaureus and @sblom

[justinddunlap]

Is this ever going to be fixed?

[orangemocha]

#6603

[jasnell]

Appears to be resolved