Skip to content

Commit

Permalink
async_hooks: fix leak in AsyncLocalStorage exit
Browse files Browse the repository at this point in the history
If exit is called and then run or enterWith are called within the
exit function, the als instace should not be added to the storageList
additional times. The correct behaviour is to remove the instance
from the storageList before executing the exit handler and then to
restore it after.

PR-URL: #35779
Reviewed-By: Vladimir de Turckheim <vlad2t@hotmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Gerhard Stöbich <deb2001-github@yahoo.de>
Reviewed-By: Andrey Pechkurov <apechkurov@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
  • Loading branch information
Qard authored and ruyadorno committed Feb 10, 2021
1 parent 427968d commit e8a4e56
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 7 deletions.
18 changes: 11 additions & 7 deletions lib/async_hooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,14 @@ class AsyncLocalStorage {
}
}

_enable() {
if (!this.enabled) {
this.enabled = true;
storageList.push(this);
storageHook.enable();
}
}

// Propagate the context from a parent resource to a child one
_propagate(resource, triggerResource) {
const store = triggerResource[this.kResourceStore];
Expand All @@ -277,11 +285,7 @@ class AsyncLocalStorage {
}

enterWith(store) {
if (!this.enabled) {
this.enabled = true;
storageList.push(this);
storageHook.enable();
}
this._enable();
const resource = executionAsyncResource();
resource[this.kResourceStore] = store;
}
Expand All @@ -305,11 +309,11 @@ class AsyncLocalStorage {
if (!this.enabled) {
return callback(...args);
}
this.enabled = false;
this.disable();
try {
return callback(...args);
} finally {
this.enabled = true;
this._enable();
}
}

Expand Down
25 changes: 25 additions & 0 deletions test/parallel/test-async-local-storage-exit-does-not-leak.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
'use strict';
const common = require('../common');
const assert = require('assert');
const { AsyncLocalStorage } = require('async_hooks');

const als = new AsyncLocalStorage();

// Make sure _propagate function exists.
assert.ok(typeof als._propagate === 'function');

// The als instance should be getting removed from the storageList in
// lib/async_hooks.js when exit(...) is called, therefore when the nested runs
// are called there should be no copy of the als in the storageList to run the
// _propagate method on.
als._propagate = common.mustNotCall('_propagate() should not be called');

const done = common.mustCall();

function run(count) {
if (count === 0) return done();
als.run({}, () => {
als.exit(run, --count);
});
}
run(100);

0 comments on commit e8a4e56

Please sign in to comment.