Skip to content

Commit fa3a970

Browse files
ForbesLindesayForbes Lindesay
authored andcommitted
Fix memory leak
1 parent cd153d0 commit fa3a970

File tree

3 files changed

+28
-4
lines changed

3 files changed

+28
-4
lines changed

lib/core.js

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,16 @@
22

33
var asap = require('asap')
44

5+
// Use Symbol if it's available, but otherwise just
6+
// generate a random string as this is probably going
7+
// to be unique
8+
var handleSymbol = typeof Symbol === 'function' ?
9+
Symbol() :
10+
(
11+
'_promise_internal_key_handle_' +
12+
Math.random().toString(35).substr(2, 10) + '_'
13+
)
14+
515
module.exports = Promise;
616
function Promise(fn) {
717
if (typeof this !== 'object') throw new TypeError('Promises must be constructed via new')
@@ -16,6 +26,7 @@ function Promise(fn) {
1626
handle(new Handler(onFulfilled, onRejected, resolve, reject))
1727
})
1828
}
29+
this.then[handleSymbol] = handle;
1930

2031
function handle(deferred) {
2132
if (state === null) {
@@ -46,7 +57,18 @@ function Promise(fn) {
4657
if (newValue && (typeof newValue === 'object' || typeof newValue === 'function')) {
4758
var then = newValue.then
4859
if (typeof then === 'function') {
49-
doResolve(then.bind(newValue), resolve, reject)
60+
if (typeof then[handleSymbol] === 'function') {
61+
// to prevent a memory leak, we adopt the value of the other promise
62+
// allowing this promise to be garbage collected as soon as nobody
63+
// has a reference to it
64+
handle = (self[handleSymbol] = then[handleSymbol]);
65+
self.then = then;
66+
deferreds.forEach(function (deferred) {
67+
handle(deferred);
68+
});
69+
} else {
70+
doResolve(then.bind(newValue), resolve, reject)
71+
}
5072
return
5173
}
5274
}

package.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44
"description": "Bare bones Promises/A+ implementation",
55
"main": "index.js",
66
"scripts": {
7-
"test": "mocha --timeout 200 --slow 99999 && npm run test-memory-leak",
8-
"test-resolve": "mocha test/resolver-tests.js -R spec --timeout 200 --slow 999999",
9-
"test-extensions": "mocha test/extensions-tests.js -R spec --timeout 200 --slow 999999",
7+
"test": "mocha --timeout 200 --slow 99999 -R dot && npm run test-memory-leak",
8+
"test-resolve": "mocha test/resolver-tests.js --timeout 200 --slow 999999",
9+
"test-extensions": "mocha test/extensions-tests.js --timeout 200 --slow 999999",
1010
"test-memory-leak": "node --expose-gc test/memory-leak.js"
1111
},
1212
"repository": {

test/memory-leak.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,9 @@ function next() {
2323
if (typeof global.gc === 'function') {
2424
global.gc()
2525
sampleB = process.memoryUsage()
26+
console.log('Memory usage at start:');
2627
console.dir(sampleA)
28+
console.log('Memory usage at end:');
2729
console.dir(sampleB)
2830
assert(sampleA.rss * 1.2 > sampleB.rss, 'RSS should not grow by more than 20%')
2931
assert(sampleA.heapTotal * 1.2 > sampleB.heapTotal, 'heapTotal should not grow by more than 20%')

0 commit comments

Comments
 (0)