Skip to content

Commit 351d8fb

Browse files
perf(ext/node): gate node:http async resource entry (#34608)
## Summary - Skips the per-request async-resource enter path in node:http request handling when no async hooks are active and no async context mapping is present. - Still restores the request-entry async context snapshot on exit, so `AsyncLocalStorage.enterWith()` inside a gated request cannot leak into later request handlers. - Preserves the existing async hooks and AsyncLocalStorage behavior by falling back to the original request-resource path whenever those observability mechanisms are active. - Adds focused coverage for async context propagation, `enterWith()` isolation in the gated path, and async resource observability through node:http requests. --------- Co-authored-by: Nathan Whitaker <nathan@deno.com>
1 parent 61f96bf commit 351d8fb

4 files changed

Lines changed: 179 additions & 6 deletions

File tree

ext/node/polyfills/_http_server.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,8 @@ const {
8585
} = core.loadExtScript("ext:deno_node/internal/validators.mjs");
8686
const { nextTick } = core.loadExtScript("ext:deno_node/_next_tick.ts");
8787
const {
88-
enterAsyncResource,
89-
exitAsyncResource,
88+
enterAsyncResourceIfActive,
89+
exitAsyncResourceIfActive,
9090
} = core.loadExtScript("ext:deno_node/internal/async_hooks.ts");
9191
const { enqueueNodePerformanceEntry, hasNodeObserverForType } = core
9292
.loadExtScript(
@@ -923,7 +923,7 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
923923
// across timers, await transitions, etc. Without this, every request would
924924
// share the top-level resource and concurrent requests would race on any
925925
// state stashed there.
926-
const prevAsyncResource = enterAsyncResource(req);
926+
const prevAsyncResource = enterAsyncResourceIfActive(req);
927927
try {
928928
let handled = false;
929929

@@ -979,7 +979,7 @@ function parserOnIncoming(server, socket, state, req, keepAlive) {
979979
server.emit("request", req, res);
980980
}
981981
} finally {
982-
exitAsyncResource(prevAsyncResource);
982+
exitAsyncResourceIfActive(prevAsyncResource);
983983
}
984984

985985
return 0;

ext/node/polyfills/internal/async_hooks.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,13 @@ const {
2121
ArrayPrototypeSlice,
2222
ArrayPrototypeSplice,
2323
FunctionPrototypeApply,
24+
ObjectKeys,
2425
Symbol,
2526
} = primordials;
2627
const {
2728
AsyncVariable,
29+
getAsyncContext,
30+
kNoAsyncContextRestore,
2831
setAsyncContext,
2932
} = core;
3033

@@ -111,6 +114,31 @@ function exitAsyncResource(previousContext: any): void {
111114
setAsyncContext(previousContext);
112115
}
113116

117+
// deno-lint-ignore no-explicit-any
118+
function enterAsyncResourceIfActive(resource: any): any {
119+
if (active_hooks.array.length > 0) {
120+
return executionResourceVariable.enter(resource);
121+
}
122+
return executionResourceVariable.enterIfActive(resource);
123+
}
124+
125+
// deno-lint-ignore no-explicit-any
126+
function exitAsyncResourceIfActive(previousContext: any): void {
127+
if (previousContext !== kNoAsyncContextRestore) {
128+
setAsyncContext(previousContext);
129+
return;
130+
}
131+
132+
const currentContext = getAsyncContext();
133+
if (
134+
currentContext !== null &&
135+
currentContext !== undefined &&
136+
ObjectKeys(currentContext).length > 0
137+
) {
138+
setAsyncContext(undefined);
139+
}
140+
}
141+
114142
// Emit functions that work with the internal hook system
115143
function emitBefore(asyncId: number): void {
116144
ArrayPrototypePush(executionAsyncIdStack, asyncId);
@@ -495,6 +523,8 @@ return {
495523
executionAsyncResource,
496524
enterAsyncResource,
497525
exitAsyncResource,
526+
enterAsyncResourceIfActive,
527+
exitAsyncResourceIfActive,
498528
emitBefore,
499529
emitAfter,
500530
emitDestroy,

libs/core/01_core.js

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -958,6 +958,7 @@
958958

959959
const getAsyncContext = getContinuationPreservedEmbedderData;
960960
const setAsyncContext = setContinuationPreservedEmbedderData;
961+
const kNoAsyncContextRestore = Symbol("Deno.core.noAsyncContextRestore");
961962

962963
function scopeAsyncContext(ctx) {
963964
const old = getAsyncContext();
@@ -977,6 +978,24 @@
977978

978979
enter(value) {
979980
const previousContextMapping = getAsyncContext();
981+
this.#enterWithPreviousContext(value, previousContextMapping);
982+
return previousContextMapping;
983+
}
984+
985+
enterIfActive(value) {
986+
const previousContextMapping = getAsyncContext();
987+
if (
988+
previousContextMapping === null ||
989+
previousContextMapping === undefined ||
990+
ObjectKeys(previousContextMapping).length === 0
991+
) {
992+
return kNoAsyncContextRestore;
993+
}
994+
this.#enterWithPreviousContext(value, previousContextMapping);
995+
return previousContextMapping;
996+
}
997+
998+
#enterWithPreviousContext(value, previousContextMapping) {
980999
const entry = { id: this.#id };
9811000
const asyncContextMapping = {
9821001
__proto__: null,
@@ -985,7 +1004,6 @@
9851004
};
9861005
this.#data.set(entry, value);
9871006
setAsyncContext(asyncContextMapping);
988-
return previousContextMapping;
9891007
}
9901008

9911009
get() {
@@ -1238,6 +1256,7 @@
12381256
setAsyncContext,
12391257
scopeAsyncContext,
12401258
AsyncVariable,
1259+
kNoAsyncContextRestore,
12411260
});
12421261

12431262
const internals = {};

tests/unit_node/http_test.ts

Lines changed: 125 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,11 @@
33
// deno-lint-ignore-file no-console
44

55
import { EventEmitter, once } from "node:events";
6-
import { createHook } from "node:async_hooks";
6+
import {
7+
AsyncLocalStorage,
8+
createHook,
9+
executionAsyncResource,
10+
} from "node:async_hooks";
711
import http, {
812
IncomingMessage,
913
type RequestOptions,
@@ -16,6 +20,7 @@ import net, { type AddressInfo, Socket } from "node:net";
1620
import fs from "node:fs";
1721
import type { Duplex } from "node:stream";
1822
import { text } from "node:stream/consumers";
23+
import { channel } from "node:diagnostics_channel";
1924

2025
import { assert, assertEquals, assertStringIncludes, fail } from "@std/assert";
2126
import { assertSpyCalls, spy } from "@std/testing/mock";
@@ -2653,6 +2658,125 @@ Deno.test("[node/http] keep-alive timer is suspended during active request", asy
26532658
}
26542659
});
26552660

2661+
Deno.test("[node/http] AsyncLocalStorage propagates into request handler", async () => {
2662+
const storage = new AsyncLocalStorage<string>();
2663+
const { promise, resolve, reject } = Promise.withResolvers<void>();
2664+
const responseDone = Promise.withResolvers<void>();
2665+
const requestStart = channel("http.server.request.start");
2666+
const subscriber = () => storage.enterWith("request-context");
2667+
const server = http.createServer((_req, res) => {
2668+
try {
2669+
assertEquals(storage.getStore(), "request-context");
2670+
resolve();
2671+
} catch (err) {
2672+
reject(err);
2673+
} finally {
2674+
res.end("ok");
2675+
}
2676+
});
2677+
2678+
requestStart.subscribe(subscriber);
2679+
await new Promise<void>((resolve) => server.listen(0, resolve));
2680+
try {
2681+
const port = (server.address() as AddressInfo).port;
2682+
const req = http.get(`http://127.0.0.1:${port}`, (res) => {
2683+
res.resume();
2684+
res.on("end", responseDone.resolve);
2685+
res.on("error", responseDone.reject);
2686+
});
2687+
req.on("error", reject);
2688+
await Promise.all([promise, responseDone.promise]);
2689+
} finally {
2690+
requestStart.unsubscribe(subscriber);
2691+
storage.disable();
2692+
await new Promise<void>((resolve) => server.close(() => resolve()));
2693+
}
2694+
});
2695+
2696+
Deno.test("[node/http] AsyncLocalStorage enterWith in request handler is isolated", async () => {
2697+
const storage = new AsyncLocalStorage<string>();
2698+
const firstDone = Promise.withResolvers<void>();
2699+
const secondDone = Promise.withResolvers<void>();
2700+
let requests = 0;
2701+
const server = http.createServer((_req, res) => {
2702+
try {
2703+
requests++;
2704+
if (requests === 1) {
2705+
assertEquals(storage.getStore(), undefined);
2706+
storage.enterWith("first-request");
2707+
assertEquals(storage.getStore(), "first-request");
2708+
} else {
2709+
assertEquals(storage.getStore(), undefined);
2710+
}
2711+
res.end("ok");
2712+
if (requests === 1) {
2713+
firstDone.resolve();
2714+
} else {
2715+
secondDone.resolve();
2716+
}
2717+
} catch (err) {
2718+
firstDone.reject(err);
2719+
secondDone.reject(err);
2720+
res.destroy(err as Error);
2721+
}
2722+
});
2723+
2724+
await new Promise<void>((resolve) => server.listen(0, resolve));
2725+
try {
2726+
const port = (server.address() as AddressInfo).port;
2727+
const request = () =>
2728+
new Promise<void>((resolve, reject) => {
2729+
const req = http.get(`http://127.0.0.1:${port}`, (res) => {
2730+
res.resume();
2731+
res.on("end", resolve);
2732+
res.on("error", reject);
2733+
});
2734+
req.on("error", reject);
2735+
});
2736+
2737+
await request();
2738+
await firstDone.promise;
2739+
await request();
2740+
await secondDone.promise;
2741+
} finally {
2742+
storage.disable();
2743+
await new Promise<void>((resolve) => server.close(() => resolve()));
2744+
}
2745+
});
2746+
2747+
Deno.test("[node/http] async_hooks observes request execution resource", async () => {
2748+
const { promise, resolve, reject } = Promise.withResolvers<void>();
2749+
const responseDone = Promise.withResolvers<void>();
2750+
const hook = createHook({
2751+
before() {},
2752+
});
2753+
const server = http.createServer((req, res) => {
2754+
try {
2755+
assertEquals(executionAsyncResource(), req);
2756+
res.end("ok");
2757+
resolve();
2758+
} catch (err) {
2759+
reject(err);
2760+
}
2761+
});
2762+
2763+
hook.enable();
2764+
try {
2765+
await new Promise<void>((resolve) => server.listen(0, resolve));
2766+
const port = (server.address() as AddressInfo).port;
2767+
const req = http.get(`http://127.0.0.1:${port}`, (res) => {
2768+
res.resume();
2769+
res.on("end", responseDone.resolve);
2770+
res.on("error", responseDone.reject);
2771+
});
2772+
req.on("error", reject);
2773+
await Promise.all([promise, responseDone.promise]);
2774+
} finally {
2775+
hook.disable();
2776+
await new Promise<void>((resolve) => server.close(() => resolve()));
2777+
}
2778+
});
2779+
26562780
Deno.test("[node/http] abandoned suspended keep-alive timer emits async_hooks destroy", async () => {
26572781
const server = http.createServer(
26582782
{ keepAliveTimeout: 10 },

0 commit comments

Comments
 (0)