Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

async_hooks: fix leak in AsyncLocalStorage exit #35779

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 11 additions & 7 deletions lib/async_hooks.js
Expand Up @@ -271,6 +271,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 @@ -280,11 +288,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 @@ -308,11 +312,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
@@ -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');
Qard marked this conversation as resolved.
Show resolved Hide resolved

const done = common.mustCall();

function run(count) {
if (count === 0) return done();
als.run({}, () => {
als.exit(run, --count);
Qard marked this conversation as resolved.
Show resolved Hide resolved
});
}
run(100);