Created
March 3, 2014 09:20
-
-
Save spadgos/9321318 to your computer and use it in GitHub Desktop.
Promises causing memory leaks
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
function bad() { | |
var promise = somethingAsync(); | |
promise.finally(function () { | |
logSomeThings(); | |
}); | |
promise = promise.then(function (result) { | |
return transform(result); | |
}); | |
return promise; | |
} | |
function good() { | |
var promise = somethingAsync(); | |
promise = promise.finally(function () { // <-- only changed line | |
logSomeThings(); | |
}); | |
promise = promise.then(function (result) { | |
return transform(result); | |
}); | |
return promise; | |
} |
I am not sure either, but the difference is that the original promise would not be retained. The same could be accomplished, I suspect, by writing promise = null
in the handler. Does this pattern leak?
function maybe() {
return somethingAsync()
.finally(logSomeThings)
.then(transform);
}
That should be equivalent, is the common usage, and doesn’t retain any intermediates. If that leaks, it’s bad news. (please ping on Twitter again; gist will not inform me if you reply).
@kriskowal: having some difficulty repro'ing the error right now (the original leak was very slow and was only visible under production load -- fun!). I'll trying boiling it down to the raw test case above and update tomorrow.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
This is using Q. The
somethingAsync
method essentially returnsQ.defer().promise