Fix client memory leaks

lib/client/client.js

@@ -1,11 +1,12 @@
 // Copyright 2011 Mark Cavage, Inc.  All rights reserved.
 
-var assert = require('assert');
 var EventEmitter = require('events').EventEmitter;
 var net = require('net');
 var tls = require('tls');
 var util = require('util');
 
+var assert = require('assert-plus');
+
 var Attribute = require('../attribute');
 var Change = require('../change');
 var Control = require('../controls/index').Control;
@@ -104,6 +105,11 @@ function setupSocket(socket, opts) {
   // On close we have to walk the outstanding messages and go invoke their
   // callback with an error
   socket.on('close', function onClose(had_err) {
+    socket.removeAllListeners('data');
+    socket.removeAllListeners('end');
+    socket.removeAllListeners('error');
+    socket.removeAllListeners('timeout');
+
     if (log.trace())
       log.trace('close event had_err=%s', had_err ? 'yes' : 'no');
 
@@ -243,16 +249,14 @@ module.exports = Client;
  * @throws {TypeError} on invalid input.
  */
 Client.prototype.abandon = function abandon(messageID, controls, callback) {
-  if (typeof (messageID) !== 'number')
-    throw new TypeError('messageID (number) required');
+  assert.number(messageID, 'messageID');
   if (typeof (controls) === 'function') {
     callback = controls;
     controls = [];
   } else {
     controls = validateControls(controls);
   }
-  if (typeof (callback) !== 'function')
-    throw new TypeError('callback (function) required');
+  assert.func(callback, 'callback');
 
   var req = new AbandonRequest({
     abandonID: messageID,
@@ -277,18 +281,15 @@ Client.prototype.abandon = function abandon(messageID, controls, callback) {
  * @throws {TypeError} on invalid input.
  */
 Client.prototype.add = function add(name, entry, controls, callback) {
-  if (typeof (name) !== 'string')
-    throw new TypeError('name (string) required');
-  if (typeof (entry) !== 'object')
-    throw new TypeError('entry (object) required');
+  assert.string(name, 'name');
+  assert.object(entry, 'entry');
   if (typeof (controls) === 'function') {
     callback = controls;
     controls = [];
   } else {
     controls = validateControls(controls);
   }
-  if (typeof (callback) !== 'function')
-    throw new TypeError('callback (function) required');
+  assert.func(callback, 'callback');
 
   if (Array.isArray(entry)) {
     entry.forEach(function (a) {
@@ -334,16 +335,14 @@ Client.prototype.add = function add(name, entry, controls, callback) {
 Client.prototype.bind = function bind(name, credentials, controls, callback) {
   if (typeof (name) !== 'string' && !(name instanceof dn.DN))
     throw new TypeError('name (string) required');
-  if (typeof (credentials) !== 'string')
-    throw new TypeError('credentials (string) required');
+  assert.string(credentials, 'credentials');
   if (typeof (controls) === 'function') {
     callback = controls;
     controls = [];
   } else {
     controls = validateControls(controls);
   }
-  if (typeof (callback) !== 'function')
-    throw new TypeError('callback (function) required');
+  assert.func(callback, 'callback');
 
   var req = new BindRequest({
     name: name || '',
@@ -371,20 +370,16 @@ Client.prototype.compare = function compare(name,
                                             value,
                                             controls,
                                             callback) {
-  if (typeof (name) !== 'string')
-    throw new TypeError('name (string) required');
-  if (typeof (attr) !== 'string')
-    throw new TypeError('attribute (string) required');
-  if (typeof (value) !== 'string')
-    throw new TypeError('value (string) required');
+  assert.string(name, 'name');
+  assert.string(attr, 'attr');
+  assert.string(value, 'value');
   if (typeof (controls) === 'function') {
     callback = controls;
     controls = [];
   } else {
     controls = validateControls(controls);
   }
-  if (typeof (callback) !== 'function')
-    throw new TypeError('callback (function) required');
+  assert.func(callback, 'callback');
 
   var req = new CompareRequest({
     entry: dn.parse(name),
@@ -411,16 +406,14 @@ Client.prototype.compare = function compare(name,
  * @throws {TypeError} on invalid input.
  */
 Client.prototype.del = function del(name, controls, callback) {
-  if (typeof (name) !== 'string')
-    throw new TypeError('name (string) required');
+  assert.string(name, 'name');
   if (typeof (controls) === 'function') {
     callback = controls;
     controls = [];
   } else {
     controls = validateControls(controls);
   }
-  if (typeof (callback) !== 'function')
-    throw new TypeError('callback (function) required');
+  assert.func(callback, 'callback');
 
   var req = new DeleteRequest({
     entry: dn.parse(name),
@@ -445,8 +438,7 @@ Client.prototype.del = function del(name, controls, callback) {
  * @throws {TypeError} on invalid input.
  */
 Client.prototype.exop = function exop(name, value, controls, callback) {
-  if (typeof (name) !== 'string')
-    throw new TypeError('name (string) required');
+  assert.string(name, 'name');
   if (typeof (value) === 'function') {
     callback = value;
     controls = [];
@@ -460,8 +452,7 @@ Client.prototype.exop = function exop(name, value, controls, callback) {
   } else {
     controls = validateControls(controls);
   }
-  if (typeof (callback) !== 'function')
-    throw new TypeError('callback (function) required');
+  assert.func(callback, 'callback');
 
   var req = new ExtendedRequest({
     requestName: name,
@@ -488,10 +479,8 @@ Client.prototype.exop = function exop(name, value, controls, callback) {
  * @throws {TypeError} on invalid input.
  */
 Client.prototype.modify = function modify(name, change, controls, callback) {
-  if (typeof (name) !== 'string')
-    throw new TypeError('name (string) required');
-  if (typeof (change) !== 'object')
-    throw new TypeError('change (Change) required');
+  assert.string(name, 'name');
+  assert.object(change, 'change');
 
   var changes = [];
 
@@ -531,8 +520,7 @@ Client.prototype.modify = function modify(name, change, controls, callback) {
   } else {
     controls = validateControls(controls);
   }
-  if (typeof (callback) !== 'function')
-    throw new TypeError('callback (function) required');
+  assert.func(callback, 'callback');
 
   var req = new ModifyRequest({
     object: dn.parse(name),
@@ -761,8 +749,14 @@ Client.prototype._send = function _send(message, expect, emitter, callback) {
     return callback(new ConnectionError('no socket'));
 
   function _done(event, obj) {
-    if (emitter)
+    if (emitter) {
+      if (event === 'error')
+        emitter.removeAllListeners('end');
+      if (event === 'end')
+        emitter.removeAllListeners('error');
+
       return emitter.emit(event, obj);
+    }
 
     if (event === 'error')
       return callback(obj);

lib/client/index.js

@@ -54,7 +54,7 @@ module.exports = {
     if (typeof (options.log) !== 'object')
       throw new TypeError('options.log must be an object');
 
-    if (options.maxConnections > 1)
+    if (options.maxConnections >= 1)
       return new ClientPool(options);
 
     return new Client(options);

lib/client/pool.js

@@ -44,7 +44,8 @@ function createPool(options) {
     create: function createClient(callback) {
       var client = new Client(options);
 
-      client.once('error', function (err) {
+      client.on('error', function (err) {
+        client.removeAllListeners('connect');
         return callback(err);
       });
 

lib/messages/parser.js

@@ -101,11 +101,11 @@ Parser.prototype.write = function (data) {
 
   try {
     message.parse(ber);
-    this.emit('message', message);
   } catch (e) {
     this.emit('error', e, message);
     return false;
   }
+  this.emit('message', message);
 
   return end();
 };

package.json

@@ -10,7 +10,7 @@
   "name": "ldapjs",
   "homepage": "http://ldapjs.org",
   "description": "LDAP client and server APIs",
-  "version": "0.5.0",
+  "version": "0.5.1",
   "repository": {
     "type": "git",
     "url": "git://github.com/mcavage/node-ldapjs.git"
@@ -21,10 +21,11 @@
     "lib": "./lib"
   },
   "engines": {
-    "node": ">=0.6"
+    "node": "~0.6"
   },
   "dependencies": {
     "asn1": "0.1.11",
+    "assert-plus": "0.1.0",
     "buffertools": "1.1.0",
     "bunyan": "0.10.0",
     "dtrace-provider": "0.0.9",

test/client.test.js

@@ -130,6 +130,7 @@ test('setup', function (t) {
 
   server.listen(SOCKET, function () {
     client = ldap.createClient({
+      connectTimeout: 100,
       socketPath: SOCKET,
       maxConnections: process.env.LDAP_MAX_CONNS || 5,
       idleTimeoutMillis: 10,
@@ -148,6 +149,7 @@ test('setup', function (t) {
 
 
 test('simple bind failure', function (t) {
+  try {
   client.bind(BIND_DN, uuid(), function (err, res) {
     t.ok(err);
     t.notOk(res);
@@ -160,6 +162,10 @@ test('simple bind failure', function (t) {
 
     t.end();
   });
+  } catch (e) {
+    console.log(e.stack);
+    process.exit(1);
+  }
 });