LearnBoost / cluster

Node.JS multi-core server manager with plugins support.
http://learnboost.github.com/cluster
MIT License
2.29k stars 159 forks source link

Fix for leaking socketpair descriptors when worker dies. #106

Closed Nomon closed 13 years ago

Nomon commented 13 years ago

Fix for bug that left open file descriptors after worker has died. Saving fds to worker on spawn and closing both on remove. The leaked descriptors are the ones created with socketpair through net bindings.

Demonstration of the leaked socketpair handles with error.js example:

matti:examples no$ nohup node error.js & [1] 12598 matti:examples no$ appending output to nohup.out nohup node error.js matti:examples no$ lsof -p 12598 | grep unix -c 8 matti:examples no$ ab -v 0 -d -S -c 10 -n 10000 http://127.0.0.1:3000/ & for((i=0;i<=10;i++)) do sleep 1; lsof -p 12598 | grep unix -c; done; [2] 12608 Benchmarking 127.0.0.1 (be patient) 34 58 85 110 136 Completed 1000 requests 160 188 212 apr_socket_recv: Connection reset by peer (54) Total of 1597 requests completed [1]- Exit 1 nohup node error.js [2]+ Exit 54 ab -v 0 -d -S -c 10 -n 10000 http://127.0.0.1:3000/ matti:examples no$ ulimit -n 256 matti:examples no$ tail -n 20 nohup.out info - worker 1 spawned info - worker 0 connected error - worker 0 uncaught exception failed! warning - worker 0 died pipe(): Too many open files

child_process.js:243 var fds = this._internal.spawn(path, ^ Error: Error spawning at ChildProcess.spawn (child_process.js:243:28) at child_process.js:31:15 at Worker.spawn (/Users/no/Development/cluster/lib/worker.js:204:15) at Master.spawnWorker (/Users/no/Development/cluster/lib/master.js:545:31) at Master.workerKilled (/Users/no/Development/cluster/lib/master.js:836:12) at Master. (/Users/no/Development/cluster/lib/master.js:479:20) at Array.forEach (native) at Master.maintainWorkerCount (/Users/no/Development/cluster/lib/master.js:477:17) at EventEmitter. (native) at EventEmitter.emit (events.js:61:17) matti:examples no$

tj commented 13 years ago

thanks, I will merge and tweak the style a bit to conform

tj commented 13 years ago

merged