fd leak

[rlidwka]

Something isn't right here:

var http = require('http');
var send = require('send');

var app = http.createServer(function(req, res){
  send(req, __filename).pipe(res);
}).listen(3000);


var net = require('net');
var headers = [
    'GET / HTTP/1.1',
    'Host: localhost:3000',
    '',
    ''
  ].join('\r\n');

function sendCanceledRequest () {
  var socket = new net.Socket();
  socket.connect(3000, 'localhost', function() {
    socket.write(headers, 'utf-8', function () {
        socket.destroy();
    });
  });
}

setInterval(sendCanceledRequest, 1000);

$ lsof -p `pidof node`
COMMAND  PID USER   FD   TYPE DEVICE SIZE/OFF    NODE NAME
node    6327 alex  cwd    DIR   0,20      358     276 /tmp
node    6327 alex  rtd    DIR   0,20      226     256 /
node    6327 alex  txt    REG   0,20  1462424  185784 /usr/bin/nodejs
[... tons of system stuff skipped ...]
node    6327 alex    9u  0000    0,9        0    7445 anon_inode
node    6327 alex   10r   DIR   0,20      226     256 /
node    6327 alex   11u  IPv4  83502      0t0     TCP *:3000 (LISTEN)
node    6327 alex   12r   REG   0,20      540 1042412 /tmp/test1.js
node    6327 alex   13r   REG   0,20      540 1042412 /tmp/test1.js
node    6327 alex   14r   REG   0,20      540 1042412 /tmp/test1.js
node    6327 alex   15r   REG   0,20      540 1042412 /tmp/test1.js
node    6327 alex   16r   REG   0,20      540 1042412 /tmp/test1.js
node    6327 alex   17r   REG   0,20      540 1042412 /tmp/test1.js
node    6327 alex   18r   REG   0,20      540 1042412 /tmp/test1.js
node    6327 alex   19r   REG   0,20      540 1042412 /tmp/test1.js
[... tons of leaking fds skipped ...]
node    6327 alex   43r   REG   0,20      540 1042412 /tmp/test1.js

[ritch]

socket.destroy();

This won't close the connection (which will eventually emit close / finish) on the server's socket.

[dougwilson]

This won't close the connection (which will eventually emit close / finish) on the server's socket.

yes it does. you have to read the core source code; the api documentation is extremely lacking in node.js core

[dougwilson]

i can confirm this is a problem and a fix is about to go out to EVERYTHING

[dougwilson]

btw, the reason we have this issue is because i never got around to jshttp/on-finished#3 :)

[rlidwka]

@dougwilson ,

I reduced this thing to:

setInterval(function() {
  var stream = require('fs').createReadStream(__filename)
  stream.destroy() // won't work because stream.fd == null
}, 1000)

Core bug, or destroy can't be used on those streams?

[dougwilson]

That looks like a race condition here: https://github.com/joyent/node/blob/912b5e05811fd24f09f9d65200a1561a4482f166/lib/fs.js#L1515 which is a core bug; it should probably check if it was destroyed after the open returned so it can be closed right away and not emit the open event

[rlidwka]

Cool. As a temporary patch, we could replace stream.destroy() with stream.close(), that seem to work. :)

[dougwilson]

Cool. As a temporary patch, we could replace stream.destroy() with stream.close(), that seem to work. :)

good to hear; does that work with the original as well? if you look in core, .destroy for readstream just calls close if there is an fd; we'll probably want to do both or something.

[dougwilson]

specifically, this conditional needs to be removed: https://github.com/joyent/node/blob/912b5e05811fd24f09f9d65200a1561a4482f166/lib/fs.js#L1592 and the core bug is fixed.

[rlidwka]

good to hear; does that work with the original as well?

yep

if you look in core, .destroy for readstream just calls close if there is an fd;

nope

  if (util.isNumber(this.fd)) // <-- null
    this.close();

[dougwilson]

p.s. i feel like i wrote the same dumb work-around in some other module...

[dougwilson]

@rlidwka would you be willing to confirm if the issue is fixed with npm install visionmedia/send#fd-leak?