socket.server is not defined

[unusualbob]

Recently myself and @braydonf were upgrading a project from 0.10.28 to 4.x and found something our connection tracking logic broke and was causing our server to never shutdown. When investigating I found that the server would never emit the close event even if all sockets had been destroyed.

When looking at net.js I noticed that the server._emitCloseIfDrained won't be called if the connection is missing its server property:

https://github.com/nodejs/node/blob/v4.2.6/lib/net.js#L482

Socket.prototype._destroy = function(exception, cb) {
  ...
  if (this.server) {
    COUNTER_NET_SERVER_CONNECTION_CLOSE(this);
    debug('has server');
    this.server._connections--;
    if (this.server._emitCloseIfDrained) {
      this.server._emitCloseIfDrained();
    }
  }

I decided to check for this property when creating my server

server.on('connection', function(socket) {
  console.log(socket.server);
});

I found that in 0.10.x socket.server was defined, but in 4.x it was undefined. I tried to make a test case to replicate it but found I was unable to do so. Luckily @braydonf took another look at it and has managed to replicate it, and is currently tracking down exactly when breakage occurred and said he would follow up on this issue when found. Looks like so far he's narrowed it down to io.js 2.2 -> 2.3.

[braydonf]

Okay so I've narrowed it further down and the behavior changed between the releases 2.3.1 and 2.3.2.

Here is code to reproduce:

var https = require('https');
var fs = require('fs');

var options = {
  key: fs.readFileSync('./server.key'),
  cert: fs.readFileSync('./server.crt')
};

var server = https.createServer(options, function(req, res) {
  res.writeHead(200);
  res.end('hello world\n');
}).listen(8111);


server.on('connection', function(connection) {
  console.log('connection.server', connection.server);
  // undefined in 2.3.2, and defined in 2.3.1
});

Looks like the behavior changed between commit 6c61ca5 and 9180140 .

[braydonf]

And specifically here: 9180140#diff-04c3a6bcf355f2e05e7700c1b253d475R377

[braydonf]

Should also be noted that the with "http" connection.server is defined, so this only affects "https".

[mscdex]

/cc @indutny

[braydonf]

Okay @unusualbob and I have worked out a test case for this here: #5106

[indutny]

Looking.

[indutny]

Should be fixed by #5262, thank you!