net: server.maxConnections does not trigger error on some platforms

[Trott]

See #1881 for discussion and some details. Exceeding server.maxConnections on OS X (and probably others) does not trigger ECONNRESET (at least in some situations) but it seemingly should and it does on FreeBSD (and probably others).

[brendanashworth]

FWIW that test also broke ARM, SmartOS and Windows, it isn't just FreeBSD.

[brendanashworth]

Here is a refined test case that illustrates the bug. For me it always fails on FreeBSD 10.0 amd64, and does not on OSX 10.10.3.

var net = require('net');

var server = net.createServer();
// Hack to reject all connections
server.maxConnections = -1;

// Listen, connect (closed asap), but write in between.
server.listen(9876, function() {
  console.error('connecting');

  var connection = net.connect(9876, function() {
    var buffered = this.write('foo');

    console.error('sent message, data', buffered ? 'flushed' : 'queued');
  });

  connection.on('close', function() {
    server.close();

    console.error('this run did not error and die');
  });
});

Non-error run (OSX @ v2.2.1):

connecting
sent message, data flushed
this run did not error and die

Error run (FreeBSD @ 43a82f8):

connecting
sent message, data flushed
events.js:141
      throw er; // Unhandled 'error' event
            ^
Error: read ECONNRESET
    at exports._errnoException (util.js:846:11)
    at TCP.onread (net.js:540:26)

The error is emitted on the client connection. I think what is happening here is, since maxConnections closes a client after it connects, the client tries to write to the server (at which point the socket is open), but the server closes the socket before the data can be read into the server.

I think this is a bug on the platforms that fail with ECONNRESET. I'm fairly sure there is no way to get whether the data was actually sent to the server; socket.write takes a callback, but it doesn't seem to give an error.

[Trott]

I installed FreeBSD 10.1 on VirtualBox on my Mac and am unable to replicate this either with the test code above or by reverting test/parallel/test-net-server-max-connections-close-makes-more-available.js. @santigimeno reported the bug on FreeBSD 10.1, so it's not a 10.0 vs. 10.1 issue. Maybe it's because I am running a VM on top of OS X and that somehow squashes the ECONNRESET?

So, flying somewhat blind because I am unable to replicate the problem (any assistance welcome) even though I have no doubt the problem is very real...

Likely relevant for any archeology on this:

• 8417870 was a change to swallow ECONNRESET. I guess this issue of inconsistent firing across operating systems came up way back when.
• 14a4245 undid that change and provides considerable detail as to the thinking.
• https://github.com/nodejs/io.js/blob/ca9eb718fbf2cd2c60c7aeb3ca33413e17fcbbf0/lib/net.js#L537 last time the line was touched, for what it's worth.

Maybe @brendanashworth or @santigimeno (or someone else running a version of FreeBSD that exhibits this issue) can compile with that line changed to return self._destroy(); and see if the problem goes away? Not saying that's necessarily a good solution. Just trying to confirm that that is where the issue comes up.

Not sure if this is a case of "oh well, different operating systems will behave differently" or if there's a good reason to attempt a workaround.

For what it's worth, it looks like ruby may have implemented a workaround specifically for this.

[brendanashworth]

@Trott fwiw I use the FreeBSD in https://github.com/brendanashworth/vms, just run setup.sh, CC=clang CXX=clang++ ./configure and make in /vagrant. I can reproduce there, even on OSX.

I think you should follow up what you're looking at and try your idea. That ruby patch looks like it's related, or at least feels like it. Too bad it doesn't mention any of the other OSs.

cc: @bnoordhuis for the earlier commit

[Trott]

Thanks, @brendanashworth. That worked for replicating the problem. And the change I suggested "solves" it but, you know, might break other tests. (test-child-process-fork-dgram and others hang and then give EADDRINUSE for me with and without the change, so something is up with the vagrant FreeBSD VM, or at least for my setup.)

[Trott]

This may be getting off the trail here, but hey @brendanashworth, when you set up your FreeBSD 10.0 via Vagrant with OS X as the host OS, does this succeed for you?

./iojs  test/parallel/test-child-process-fork-dgram.js

If not, does applying these two changes fix it?

--- a/test/parallel/test-child-process-fork-dgram.js
+++ b/test/parallel/test-child-process-fork-dgram.js
@@ -34,10 +34,12 @@ if (process.argv[2] === 'child') {
       server.on('message', function() {
         process.send('gotMessage');
       });
+      setInterval(function () {}, 1000);

     } else if (msg === 'stop') {
       server.close();
       process.removeListener('message', removeMe);
+      process.exit();
     }
   });

[brendanashworth]

@Trott it seems to work fine via the test suite but not individually (hangs forever). It doesn't seem to be blocking the CI so maybe it is related to #1876.

[Trott]

Doesn't work for me in the suite or individually. And not temp file related. So not #1876-related, I don't think. Either way, if It's Just Me, then maybe I ought to just blow everything away, start over, and see if it goes away...

[Trott]

Bumping my VM from 1 CPU to 2 CPUs and fixed the test-child-process-fork-dgram issue for me. Hooray. Now back to the main issue at hand...

[Trott]

Er...um...OK...bumping myself from 1 CPU to 2 CPUs also made the test case above start passing for me on FreeBSD. Er...@brendanashworth, do you experience that too? Or is the test still failing for you on FreeBSD if you go from (presumably) a single CPU to 2?