Skip to content

Commit

Permalink
Fix memory leak
Browse files Browse the repository at this point in the history
  • Loading branch information
ForbesLindesay authored and Forbes Lindesay committed Apr 2, 2015
1 parent cd153d0 commit 5c2f0e4
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 4 deletions.
35 changes: 34 additions & 1 deletion lib/core.js
Expand Up @@ -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')
Expand All @@ -16,6 +26,7 @@ function Promise(fn) {
handle(new Handler(onFulfilled, onRejected, resolve, reject))
})
}
this.then[handleSymbol] = handle;

function handle(deferred) {
if (state === null) {
Expand Down Expand Up @@ -43,10 +54,32 @@ function Promise(fn) {
function resolve(newValue) {
try { //Promise Resolution Procedure: https://github.com/promises-aplus/promises-spec#the-promise-resolution-procedure
if (newValue === self) throw new TypeError('A promise cannot be resolved with itself.')
if (newValue instanceof Promise && newValue.handle && typeof newValue.handle === '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
self.handle = newValue.handle;
self.then = newValue.then;
deferreds.forEach(function (deferred) {
self.handle(deferred);
});
return
}
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
handle = (self[handleSymbol] = then[handleSymbol]);
self.then = then;
deferreds.forEach(function (deferred) {
handle(deferred);
});
} else {
doResolve(then.bind(newValue), resolve, reject)
}
return
}
}
Expand Down
6 changes: 3 additions & 3 deletions package.json
Expand Up @@ -4,9 +4,9 @@
"description": "Bare bones Promises/A+ implementation",
"main": "index.js",
"scripts": {
"test": "mocha --timeout 200 --slow 99999 && npm run test-memory-leak",
"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": {
Expand Down
2 changes: 2 additions & 0 deletions test/memory-leak.js
Expand Up @@ -23,7 +23,9 @@ function next() {
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%')
Expand Down

0 comments on commit 5c2f0e4

Please sign in to comment.