tl;dr
Never, ever forget that node.js is single threaded and expects every callback to return to the event loop. Otherwise it will never be garbage collected.
Background
This behavior was experienced in the Softwerkskammer platform. In order to show small avatars next to member’s names we load them from gravatar or take an uploaded pic. These are then binhexed and inlined in the page’s content. We started doing this every time the memberlist is requested. This is very slow and additionally causes unnecessary load to gravatar’s site. We then introduced a local cache using node-cache. I wanted to get rid of this caching and persist the gravatar along with other profile data in our DB.
Of course, we do not know, if a gravatar has been updated. Therefor we need to regularly check gravatar’s services.
Code
We had code like that (no caching anymore):
function imageDataFromGravatar (url, callback) {
request.get(url, function (error, response, body) {
if (error) { return callback({image: null}); }
var image = 'data:' + response.headers['content-type'] + ';base64,' + new Buffer(body).toString('base64');
callback({image: image});
});
}
module.exports = {
getImage: function (member, callback) {
var url = this.avatarUrl(member.email(), 16);
imageDataFromGravatar(url, function (data) {
member.setAvatarData(data);
callback(data);
});
}
};
The function “getImage” was called like that (for each member in a loop):
if (!member.getPersistedAvatarData()) {
avatarProvider.getImage(member, function (imageData) {
member.persistAvatarData(imageData);
store.saveMember(member, callback);
});
} else {
if (/*actuality check*/) {
avatarProvider.getImage(member, function (imageData) {
var oldAvatar = member.getPersistedAvatarData();
member.persistAvatarData(imageData);
if (member.getPersistedAvatarData() !== oldAvatar) {
store.saveMember(member, function () { /* background op */ });
}
});
}
callback();
}
Do you spot the horror?
store.saveMember(member, function () { /* background op */ });
I really thought, if I don’t give the “saveMember” function a callback method (which is expected by that method, it eventually calls a mongodb function that expects a callback), it just performs its stuff in the background. – *haha* background, *haha* background, *haha* background – today I am laughing at that. A few weeks ago I have been completely naiv.
So what did I do?
quite simple, changing the last few lines to:
if (/*actuality check*/) {
avatarProvider.getImage(member, function (imageData) {
member.setAvatarData(imageData);
return store.saveMember(member, callback); // never, ever "fork" stuff in node by not having return values *I AM IDIOT*
});
} else {
callback();
}
(please note the comment after the important line)
That’s it. I hope it helps you to not do the same mistakes.
P.S.: Finding this took me multiple days, spending lots of hours of effort.
Pair Blog about OO-Programming 