Merge pull request then#67 from then/fix-memory-leak

Fix memory leak

Author: ForbesLindesay

lib/core.js

@@ -2,6 +2,16 @@
 
 var asap = require('asap')
 
+// Use Symbol if it's available, but otherwise just
+// generate a random string as this is probably going
+// to be unique
+var handleSymbol = typeof Symbol === 'function' ?
+    Symbol() :
+    (
+      '_promise_internal_key_handle_' +
+      Math.random().toString(35).substr(2, 10) + '_'
+    )
+
 module.exports = Promise;
 function Promise(fn) {
   if (typeof this !== 'object') throw new TypeError('Promises must be constructed via new')
@@ -16,8 +26,11 @@ function Promise(fn) {
       handle(new Handler(onFulfilled, onRejected, resolve, reject))
     })
   }
-
+  this.then[handleSymbol] = handle;
   function handle(deferred) {
+    return internalHandle(deferred);
+  }
+  function internalHandle(deferred) {
     if (state === null) {
       deferreds.push(deferred)
       return
@@ -46,7 +59,17 @@ function Promise(fn) {
       if (newValue && (typeof newValue === 'object' || typeof newValue === 'function')) {
         var then = newValue.then
         if (typeof then === 'function') {
-          doResolve(then.bind(newValue), resolve, reject)
+          if (typeof then[handleSymbol] === 'function') {
+            // to prevent a memory leak, we adopt the value of the other promise
+            // allowing this promise to be garbage collected as soon as nobody
+            // has a reference to it
+            internalHandle = then[handleSymbol];
+            deferreds.forEach(function (deferred) {
+              internalHandle(deferred);
+            });
+          } else {
+            doResolve(then.bind(newValue), resolve, reject)
+          }
           return
         }
       }

package.json

@@ -4,9 +4,10 @@
   "description": "Bare bones Promises/A+ implementation",
   "main": "index.js",
   "scripts": {
-    "test": "mocha --timeout 200 --slow 99999",
-    "test-resolve": "mocha test/resolver-tests.js -R spec --timeout 200 --slow 999999",
-    "test-extensions": "mocha test/extensions-tests.js -R spec --timeout 200 --slow 999999"
+    "test": "mocha --timeout 200 --slow 99999 -R dot && npm run test-memory-leak",
+    "test-resolve": "mocha test/resolver-tests.js --timeout 200 --slow 999999",
+    "test-extensions": "mocha test/extensions-tests.js --timeout 200 --slow 999999",
+    "test-memory-leak": "node --expose-gc test/memory-leak.js"
   },
   "repository": {
     "type": "git",

test/memory-leak.js

@@ -0,0 +1,43 @@
+'use strict';
+
+var assert = require('assert')
+var Promise = require('../')
+
+var i = 0
+var sampleA, sampleB
+
+function next() {
+  return new Promise(function (resolve) {
+    i++
+    /*
+    if (i % 100000 === 0) {
+      global.gc()
+      console.dir(process.memoryUsage())
+    }
+    */
+    if (i === 100000 && typeof global.gc === 'function') {
+      global.gc()
+      sampleA = process.memoryUsage()
+    }
+    if (i > 100000 * 5) {
+      if (typeof global.gc === 'function') {
+        global.gc()
+        sampleB = process.memoryUsage()
+        console.log('Memory usage at start:');
+        console.dir(sampleA)
+        console.log('Memory usage at end:');
+        console.dir(sampleB)
+        assert(sampleA.rss * 1.2 > sampleB.rss, 'RSS should not grow by more than 20%')
+        assert(sampleA.heapTotal * 1.2 > sampleB.heapTotal, 'heapTotal should not grow by more than 20%')
+        assert(sampleA.heapUsed * 1.2 > sampleB.heapUsed, 'heapUsed should not grow by more than 20%')
+      }
+    } else {
+      setImmediate(resolve)
+    }
+  }).then(next)
+}
+
+if (typeof global.gc !== 'function') {
+  console.warn('You must run with --expose-gc to test for memory leak.')
+}
+next().done()

test/nested-promises.js

@@ -0,0 +1,67 @@
+'use strict';
+
+var assert = require('assert');
+var Promise = require('../');
+
+describe('nested promises', function () {
+  it('does not result in any wierd behaviour - 1', function (done) {
+    var resolveA, resolveB, resolveC;
+    var A = new Promise(function (resolve, reject) {
+      resolveA = resolve;
+    });
+    var B = new Promise(function (resolve, reject) {
+      resolveB = resolve;
+    });
+    var C = new Promise(function (resolve, reject) {
+      resolveC = resolve;
+    });
+    resolveA(B);
+    resolveB(C);
+    resolveC('foo');
+    A.done(function (result) {
+      assert(result === 'foo');
+      done();
+    });
+  });
+  it('does not result in any wierd behaviour - 2', function (done) {
+    var resolveA, resolveB, resolveC, resolveD;
+    var A = new Promise(function (resolve, reject) {
+      resolveA = resolve;
+    });
+    var B = new Promise(function (resolve, reject) {
+      resolveB = resolve;
+    });
+    var C = new Promise(function (resolve, reject) {
+      resolveC = resolve;
+    });
+    var D = new Promise(function (resolve, reject) {
+      resolveD = resolve;
+    });
+    var Athen = A.then, Bthen = B.then, Cthen = C.then, Dthen = D.then;
+    resolveA(B);
+    resolveB(C);
+    resolveC(D);
+    resolveD('foo');
+    A.done(function (result) {
+      assert(result === 'foo');
+      done();
+    });
+  });
+  it('does not result in any wierd behaviour - 2', function (done) {
+    var promises = [];
+    var resolveFns = [];
+    for (var i = 0; i < 100; i++) {
+      promises.push(new Promise(function (resolve) {
+        resolveFns.push(resolve);
+      }));
+    }
+    for (var i = 0; i < 99; i++) {
+      resolveFns[i](promises[i + 1]);
+    }
+    resolveFns[99]('foo');
+    promises[0].done(function (result) {
+      assert(result === 'foo');
+      done();
+    });
+  });
+});