Skip to content
Open
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
6103e8c
feat(dev): detect 'use cache' module-scope deadlocks early in dev (#1…
Divkix May 11, 2026
f0ba0b9
ci: fix benchmarks/vinext tsconfig missing vinext path and remove red…
Divkix May 11, 2026
2c7dde8
chore: remove unused tinypool dependency causing knip failure
Divkix May 11, 2026
4b2c506
fix(tests): remove unused use-cache deadlock fixtures causing static …
Divkix May 11, 2026
5662e1a
fix(dev): address use-cache probe review issues
Divkix May 11, 2026
b4886d1
fix(use-cache): address all review issues for deadlock probe
Divkix May 11, 2026
b543d3a
fix(use-cache): address v3 review feedback on deadlock probe
Divkix May 11, 2026
c2e053f
fix(use-cache): address v4 review feedback on deadlock probe
Divkix May 11, 2026
445a0d0
fix(use-cache): address v5 review feedback on deadlock probe
Divkix May 11, 2026
41895ed
Merge branch 'main' into fix/issue-1009
Divkix May 12, 2026
fe5ac07
Merge branch 'main' into fix/issue-1009
Divkix May 12, 2026
9dc5254
refactor(server): use performance.now() in use-cache probe pool
Divkix May 12, 2026
bc998e6
fix(shims/cache-runtime): review round 6 cleanups for use-cache probe
Divkix May 12, 2026
0b10e4f
refactor(server): lazy-load use-cache probe pool and document variant…
Divkix May 12, 2026
3223445
fix(server): surface use-cache probe failures
Divkix May 14, 2026
f0a6c58
Merge branch 'main' into fix/issue-1009
Divkix May 14, 2026
f15c467
fix(shims): address use-cache probe review nits
Divkix May 14, 2026
73d4f15
Merge branch 'main' into fix/issue-1009
Divkix May 16, 2026
86678d8
fix(shims): address ask-bonk review nits for use-cache probe
Divkix May 16, 2026
242b27e
fix(tests): use vi.stubEnv instead of direct process.env.NODE_ENV ass…
Divkix May 16, 2026
42fb5cd
fix: address review feedback on use-cache probe
Divkix May 18, 2026
fe0d953
fix(shims): address ask-bonk parity review (#2, #3)
Divkix May 19, 2026
9d71da4
fix(shims): address probe review retry path
Divkix May 19, 2026
fea883d
Merge branch 'main' into fix/issue-1009
Divkix May 20, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion benchmarks/vinext/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
"resolveJsonModule": true,
"isolatedModules": true,
"paths": {
"@/*": ["./*"]
"@/*": ["./*"],
"vinext": ["../../packages/vinext/src/index.ts"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is unrelated to the deadlock probe feature. If the benchmarks tsconfig needs a vinext path mapping, that should be a separate commit or PR.

}
},
"include": ["**/*.ts", "**/*.tsx"],
Expand Down
12 changes: 12 additions & 0 deletions packages/vinext/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { handleApiRoute } from "./server/api-handler.js";
import { installSocketErrorBackstop } from "./server/socket-error-backstop.js";
import { shouldInvalidateAppRouteFile } from "./server/dev-route-files.js";
import { createDirectRunner } from "./server/dev-module-runner.js";
import { initUseCacheProbePool, tearDownUseCacheProbePool } from "./server/use-cache-probe-pool.js";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: static import loads probe pool at plugin registration time. initUseCacheProbePool and tearDownUseCacheProbePool are only used inside configureServer() (dev only). A dynamic import() inside configureServer would avoid loading use-cache-probe-pool.js (and its transitive imports of use-cache-probe-globals and use-cache-errors) during vite build.

Non-blocking — Vite plugins are dev-time artifacts and the module loading cost is trivial. But if bundle size of the plugin itself ever matters, this is low-hanging fruit.

import { generateRscEntry } from "./entries/app-rsc-entry.js";
import { generateSsrEntry } from "./entries/app-ssr-entry.js";
import { generateBrowserEntry } from "./entries/app-browser-entry.js";
Expand Down Expand Up @@ -2041,6 +2042,9 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
invalidateAppRouteCache();
invalidateRscEntryModule();
invalidateRootParamsModule();
// Tear down the use-cache probe pool so the next probe starts with
// fresh code after HMR invalidation.
tearDownUseCacheProbePool();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical: probe permanently disabled after first HMR. tearDownUseCacheProbePool() nulls _probeEnvironment and clears the probe function. But initUseCacheProbePool() is only called once (line 2146), not after each teardown. After the first file change, the probe is permanently gone.

Either re-initialize here:

Suggested change
tearDownUseCacheProbePool();
tearDownUseCacheProbePool();
// Re-initialize so probes continue working after HMR.
const rscEnv = server.environments["rsc"];
if (rscEnv) {
initUseCacheProbePool(rscEnv);
}

Or restructure tearDownUseCacheProbePool so it doesn't unset the probe function — since the environment reference is the same, you could just trust that fresh runners (created per-probe) already give you fresh code.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The teardown+reinit pattern looks correct now. tearDownUseCacheProbePool() nulls _probeEnvironment and clears the probe, then initUseCacheProbePool() sees _probeEnvironment === null, re-assigns it, and re-installs the probe closure.

One thing to watch: if invalidateAppRoutingModules() is called frequently during rapid saves, each call creates a new probe closure. The old closure is garbage-collected since setUseCacheProbe() overwrites the globalThis symbol. Any in-flight probe from the old closure will still hold a reference to _probeEnvironment via the closure capture (the _probeEnvironment! on line 47 of use-cache-probe-pool.ts). Since _probeEnvironment is reassigned to null then immediately back to the same rscEnv, this is fine — but if the environment could change, the captured null would be a problem.

}

// Node throws on unhandled 'error' events on sockets. When a browser
Expand Down Expand Up @@ -2134,6 +2138,14 @@ export default function vinext(options: VinextOptions = {}): PluginOption[] {
// it is flushed to the client.
// 2. Logs the full request after res finishes, using those timings.
if (hasAppDir) {
// Initialize the "use cache" deadlock probe pool for the App
// Router dev server. We bind to the RSC environment because
// "use cache" functions run inside the RSC module graph.
const rscEnv = server.environments["rsc"];
if (rscEnv) {
initUseCacheProbePool(rscEnv);
}

server.middlewares.use((req, res, next) => {
const url = req.url ?? "/";
// Skip Vite internals, HMR, and static assets.
Expand Down
50 changes: 49 additions & 1 deletion packages/vinext/src/server/dev-module-runner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ import type { DevEnvironment } from "vite";
* environment types — including Cloudflare's custom environments that don't
* support the hot-channel-based transport.
*/
type DevEnvironmentLike = {
export type DevEnvironmentLike = {
fetchModule: (
id: string,
importer?: string,
Expand Down Expand Up @@ -129,3 +129,51 @@ export function createDirectRunner(environment: DevEnvironmentLike | DevEnvironm
new ESModulesEvaluator(),
);
}

/**
* Create a fresh ModuleRunner with its own isolated module graph.
*
* Used by the "use cache" deadlock probe to re-import a cache function
* with zero shared module-scope state from the main request pipeline.
* Each probe gets a brand-new `EvaluatedModules` instance so top-level
* `Map`s, `Promise`s, etc. are recreated from scratch.
*
* The transport delegates to the same `environment.fetchModule()`, so
* Vite transforms and HMR still work, but the *evaluated* module cache
* is completely separate.
*/
export function createProbeRunner(environment: DevEnvironmentLike | DevEnvironment): ModuleRunner {
return new ModuleRunner(
{
transport: {
// oxlint-disable-next-line @typescript-eslint/no-explicit-any
invoke: async (payload: any) => {
const { name, data: args } = payload.data;
if (name === "fetchModule") {
const [id, importer, options] = args as [
string,
string | undefined,
{ cached?: boolean; startOffset?: number } | undefined,
];
return {
result: await environment.fetchModule(id, importer, options),
};
}
if (name === "getBuiltins") {
return { result: [] };
}
return {
error: {
name: "Error",
message: `[vinext] Unexpected ModuleRunner invoke: ${name}`,
},
};
},
},
createImportMeta: createNodeImportMeta,
sourcemapInterceptor: false,
hmr: false,
},
new ESModulesEvaluator(),
);
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moderate: exact duplicate of createDirectRunner. This function is identical to createDirectRunner above — same transport, same evaluator, same options. Each call already creates a fresh ModuleRunner with its own EvaluatedModules, so there's no need for a separate function.

Just reuse createDirectRunner in use-cache-probe-pool.ts. The JSDoc distinction ("for probes") can be a comment at the call site.

2 changes: 1 addition & 1 deletion packages/vinext/src/server/request-pipeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -601,7 +601,7 @@ export function filterInternalHeaders(headers: Headers): Headers {
return filtered;
}

function getRequestCf(request: Request): unknown | undefined {
function getRequestCf(request: Request): unknown {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This type change (unknown | undefinedunknown) is unrelated to the deadlock probe feature. unknown | undefined does simplify to unknown so this is technically a no-op, but it should be in a separate commit.

const cf = Reflect.get(request, "cf");
return cf === undefined ? undefined : cf;
}
Expand Down
215 changes: 215 additions & 0 deletions packages/vinext/src/server/use-cache-probe-pool.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,215 @@
/**
* use-cache-probe-pool.ts
*
* Manages a pool of isolated ModuleRunners for "use cache" deadlock probes.
*
* In dev mode, when a cache fill appears stuck, we re-run the same cache
* function in a fresh module graph. If it completes there but the main fill
* is still hung, the hang is attributable to module-scope shared state
* (e.g. a top-level Map used to dedupe fetches) from the outer render.
*
* Unlike Next.js (which uses jest-worker with real OS processes), vinext
* creates a fresh Vite ModuleRunner per probe. Each runner has its own
* EvaluatedModules instance, so top-level module state is recreated from
* scratch while still using the same Vite transform pipeline.
*
* The pool is torn down on HMR / file invalidation so the next probe
* starts with fresh transformed code.
*/

import type { DevEnvironment } from "vite";
import { createProbeRunner, type DevEnvironmentLike } from "./dev-module-runner.js";
import { ModuleRunner } from "vite/module-runner";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ModuleRunner is only used as a type annotation (line 152). Use a type-only import to avoid pulling the module at runtime:

Suggested change
import { ModuleRunner } from "vite/module-runner";
import type { ModuleRunner } from "vite/module-runner";

import { setUseCacheProbe } from "vinext/shims/use-cache-probe-globals";
import { UseCacheTimeoutError } from "vinext/shims/use-cache-errors";

let _activeProbeRunners: ModuleRunner[] | null = null;
let _environment: DevEnvironmentLike | DevEnvironment | null = null;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead store: _environment is assigned on line 52 and nulled on line 164, but is never read anywhere. Remove it.

const MAX_RUNNERS = 4;

function getProbeRunner(): ModuleRunner {
if (!_activeProbeRunners || _activeProbeRunners.length === 0) {
throw new Error("[vinext] use cache probe pool not initialized");
}
// Round-robin across runners for basic load distribution.
const runner = _activeProbeRunners.shift()!;
_activeProbeRunners.push(runner);
return runner;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stale module state on reuse: The pool round-robins across 4 pre-created runners, but ModuleRunner caches evaluated modules internally. The second time a runner is used for the same function, the module's top-level scope has already been evaluated — the Maps, Promises, etc. that the probe is supposed to test with fresh state are already initialized from the previous probe run.

This means the first probe for a given function works correctly, but subsequent probes on the same runner reuse stale module state and may not detect the deadlock.

Either:

  1. Create a fresh runner per probe and close() it after, or
  2. Call runner.close() and replace the runner in the pool after each use

}

/**
* Initialize the probe pool with a set of fresh ModuleRunners bound to the
* given Vite dev environment.
*
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: JSDoc says "Called once during configureServer()" but the function is also called on every HMR file change (via invalidateAppRoutingModulestearDownUseCacheProbePool()initUseCacheProbePool()). The guard on line 34 correctly handles re-init, but the doc is misleading.

Suggested change
*
* Called during configureServer() when the App Router dev server starts,
* and re-called after each HMR teardown cycle.

* Called once during configureServer() when the App Router dev server starts.
*/
export function initUseCacheProbePool(environment: DevEnvironmentLike | DevEnvironment): void {
if (_activeProbeRunners) {
// Already initialized — no-op. The environment is the same for the
// lifetime of the dev server.
return;
Comment on lines +36 to +39
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Misleading comment. This says "Already initialized — no-op" and "the environment is the same for the lifetime of the dev server", but invalidateAppRoutingModules() explicitly calls tearDownUseCacheProbePool() then initUseCacheProbePool() on every file change. The guard works correctly (teardown nulls _probeEnvironment, so reinit proceeds), but the comment is wrong about this being a one-time init.

Suggested change
if (_probeEnvironment) {
// Already initialized — no-op. The environment is the same for the
// lifetime of the dev server.
return;
// Guard against double-init within the same cycle (e.g., if
// initUseCacheProbePool is called without a preceding teardown).
return;

}
_environment = environment;
_activeProbeRunners = [];
for (let i = 0; i < MAX_RUNNERS; i++) {
_activeProbeRunners.push(createProbeRunner(environment));
}

setUseCacheProbe(async (msg) => {
const runner = getProbeRunner();
const { id, kind, encodedArguments, request, timeoutMs } = msg;

// Internal timeout so the probe aborts before the outer render timeout.
const deadline = Date.now() + timeoutMs;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Date.now() vs performance.now() inconsistency. The deadline here is computed with Date.now(), but the remaining-time check in cache-runtime.ts:443 uses performance.now() (fillDeadlineAt - performance.now() - 1_000). These two clocks have different origins — Date.now() is epoch-based, performance.now() is monotonic from process start. They can't be meaningfully subtracted from each other.

The deadline on this line is only used on line 121 as deadline - Date.now(), which is self-consistent. But if someone later tries to compare this deadline with fillDeadlineAt (which is performance.now()-based), it'll be wrong. Consider using performance.now() here too for consistency with the caller:

Suggested change
const deadline = Date.now() + timeoutMs;
const deadline = performance.now() + timeoutMs;

Then line 121 becomes deadline - performance.now(). The monotonic clock is also more correct for timeout math (immune to wall-clock adjustments).


try {
// Import the cache-runtime shim in the isolated runner.
// The shim's registerCachedFunction will create fresh module-scope state.
const cacheRuntime = (await runner.import("vinext/shims/cache-runtime")) as Record<
string,
unknown
>;
Comment on lines +60 to +66
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: The runner.import("vinext/shims/cache-runtime") call loads the full cache-runtime module in the isolated graph — including the isDev check, the getProbeModules() lazy loader, and the 54s timeout setup. When the probe subsequently calls wrapped(...args) at line 121, the wrapped function enters cachedFn in the probe runner's cache-runtime. Since the probe runner also runs with NODE_ENV=development, it enters the if (isDev) block — but the _probeDepth guard at line 479 correctly short-circuits to executeWithContext before any timer allocation.

This works correctly because the _probeDepth check (line 479) happens before module loading and timer setup in the probe's cachedFn. Good — this addresses the "double dev-timeout" concern from earlier review rounds.

const registerCachedFunction = cacheRuntime.registerCachedFunction as
| (<T extends (...args: unknown[]) => Promise<unknown>>(
fn: T,
id: string,
variant?: string,
) => T)
| undefined;

if (!registerCachedFunction) {
return false;
}

// We need to locate the original cached function module in the isolated
// runner. The function id is "<modulePath>:<exportName>". We split it
// to find the module and the export.
const lastColon = id.lastIndexOf(":");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Edge case: function IDs containing colons in the module path. id.lastIndexOf(":") works for most Unix paths, but on Windows, absolute paths contain colons (e.g., C:\foo\bar.ts:myExport). lastIndexOf(":") would split at the right colon here, which is correct. But if the export name itself contains a colon (unlikely but not impossible with computed exports), this would split incorrectly.

This is fine for now — just flagging it as a known limitation. A comment noting the assumption ("export names don't contain colons") would be helpful.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The lastIndexOf(":") approach for splitting "<modulePath>:<exportName>" works correctly for Unix paths and even Windows paths (where C:\ has a colon before the backslash — lastIndexOf finds the export separator, not the drive letter).

The comment on line 81 documents the assumption that export names don't contain colons. Worth noting that on Windows, module IDs like C:/foo/bar.ts:myExport would also work correctly since lastIndexOf returns the rightmost colon. Good.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Windows path edge case. On Windows, Vite module IDs use forward slashes internally, so C:/foo/bar.ts:myExport would split correctly at the rightmost colon. But if someone uses a non-standard module ID format or the transform emits colons in the path (e.g., virtual modules like virtual:vinext-foo:bar), lastIndexOf(":") would split at the wrong boundary.

Not blocking — the assumption is reasonable for file-system modules. But consider adding a brief comment noting this relies on the ID format being <path>:<export> with no colons in the export name.

const modulePath = lastColon >= 0 ? id.slice(0, lastColon) : id;
const exportName = lastColon >= 0 ? id.slice(lastColon + 1) : "default";

// Import the module containing the original "use cache" function.
const mod = (await runner.import(modulePath)) as Record<string, unknown>;
const originalFn = mod[exportName];
if (typeof originalFn !== "function") {
return false;
}

// Wrap it with registerCachedFunction so the probe runs through the
// same cache-runtime path (fresh ALS, no shared state).
const variant = kind === "private" ? "private" : "";
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dead code. kind can never be "private" here because private cache functions (cacheVariant === "private") return at cache-runtime.ts:396 before reaching the isDev probe-scheduling block at line 401. The probe is never scheduled for private caches.

This should either be removed (simplify to const variant = "";) or documented with a comment explaining it's a safety net for future changes.

const wrapped = registerCachedFunction(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When wrapped(...args) is called, the probe runner's cache-runtime evaluates with NODE_ENV=development and enters the if (isDev) block. The _probeDepth: 1 guard prevents nested probe scheduling, but the inner 54s timeout timer and probe module loading still execute (they run before the _probeDepth check in the current code).

If the _probeDepth check is moved above the module loading / timeout setup (see my comment on cache-runtime.ts), this becomes a non-issue — the probe's inner cachedFn will early-return via executeWithContext without any timer allocation.

If that reordering isn't done, consider adding a brief comment here noting the double-timeout is expected and harmless (outer pool timeout preempts inner).

originalFn as (...args: unknown[]) => Promise<unknown>,
id,
variant,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Double dev-timeout in probe. wrapped(...) enters cachedFn in the probe runner's cache-runtime, which (since NODE_ENV=development) enters the if (isDev) block and sets up another 54s timeout + probe module loading. The _probeDepth: 1 guard correctly prevents a nested probe, but the 54s timer still runs unnecessarily.

Next.js avoids this: when workStore.useCacheProbeMode is set, the wrapper takes a different code path that skips the dev-timeout/probe setup entirely.

Consider either:

  1. Checking _probeDepth > 0 earlier (before the timeout setup, not just before probe scheduling), or
  2. Adding a TODO noting this is a known overhead.

Not blocking since the outer probe pool timeout preempts the inner one, but it's wasteful.

);

// Decode the arguments (simple JSON fallback; RSC encodeReply is
// not available in the probe because we lack the client environment).
// For deadlock detection, the exact argument values matter less than
// the fact that the function body executes with a fresh module scope.
let args: unknown[] = [];
if (typeof encodedArguments === "string") {
try {
args = JSON.parse(encodedArguments);
if (!Array.isArray(args)) args = [args];
} catch {
args = [];
}
}

// Run the function with a reconstructed request store so private caches
// that read cookies()/headers()/draftMode() see the same values.
const result = await runWithProbeRequestStore(request, async () => wrapped(...args));

// Wait for the result, but enforce the internal timeout.
const remaining = deadline - Date.now();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the suggestion to use performance.now() for the deadline is accepted, this line should also change:

Suggested change
const remaining = deadline - Date.now();
const remaining = performance.now() >= deadline ? 0 : deadline - performance.now();

Or more simply:

const remaining = deadline - performance.now();

since the <= 0 check on the next line already handles the expired case.

if (remaining <= 0) {
return false;
}

// If we got here, the probe completed.
await Promise.race([
Promise.resolve(result),
new Promise<never>((_, reject) => {
const t = setTimeout(() => reject(new UseCacheTimeoutError()), remaining);
// Ensure timer is cleaned up on success via unref if available.
if (typeof (t as NodeJS.Timeout).unref === "function") {
(t as NodeJS.Timeout).unref();
}
}),
]);
Comment on lines +110 to +128
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Redundant await + race: runWithProbeRequestStore is awaited on line 123, which means result is already a resolved value by line 125. The Promise.race on lines 132-141 is racing an already-resolved Promise.resolve(result) against a timeout — the resolved promise always wins instantly. This timeout never fires.

The timeout should wrap the un-awaited call:

const result = await Promise.race([
  runWithProbeRequestStore(request, async () => wrapped(...args)),
  new Promise<never>((_, reject) => {
    const t = setTimeout(() => reject(new UseCacheTimeoutError()), remaining);
    if (typeof t.unref === 'function') t.unref();
  }),
]);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor timer leak inside the probe. When runWithProbeRequestStore resolves successfully, the Promise.race completes but the timeout setTimeout on line 121 is never cleared — it's only .unref()'d. The timer callback will fire after remaining ms and create a UseCacheTimeoutError that rejects into a detached promise (the catch on line 130 won't see it since execution already left the try block via the return true on line 129).

The .unref() prevents it from keeping the process alive, so this is not a blocker, but it's wasteful and will produce an unhandled rejection in some Node versions. Store the timer ID and clear it in the finally block:

Suggested change
]);
let probeTimeoutTimer: ReturnType<typeof setTimeout> | undefined;
await Promise.race([
runWithProbeRequestStore(runner, request, async () => wrapped(...args)),
new Promise<never>((_, reject) => {
probeTimeoutTimer = setTimeout(() => reject(new UseCacheTimeoutError()), remaining);
if (typeof (probeTimeoutTimer as NodeJS.Timeout).unref === "function") {
(probeTimeoutTimer as NodeJS.Timeout).unref();
}
}),
]);

Then in the finally block:

if (probeTimeoutTimer !== undefined) clearTimeout(probeTimeoutTimer);


return true;
} catch {
// Probe failure is inconclusive — the function might genuinely hang
// even in isolation, or the module import failed. Fall back to the
// regular timeout.
return false;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A function that throws is not deadlocked. The catch here handles two distinct failure modes identically:

  1. The probe's internal timeout fires (UseCacheTimeoutError) — genuinely inconclusive, false is correct.
  2. The wrapped function throws an error — the function actually ran and completed (with a failure). If the main fill is still stuck but the probe's copy threw, that's evidence the main fill is deadlocked on shared state, not that the function itself can't run.

Consider distinguishing these:

Suggested change
return false;
} catch (err) {
// If the probe timed out, the result is inconclusive.
if (err instanceof UseCacheTimeoutError) {
return false;
}
// The function threw — it ran to completion (with an error).
// If the main fill is still stuck, this is evidence of a deadlock.
return true;

}
});
}

/**
* Tear down the probe pool. Called on HMR / file invalidation so the next
* probe starts with fresh code.
*/
export function tearDownUseCacheProbePool(): void {
if (_activeProbeRunners) {
for (const runner of _activeProbeRunners) {
runner.close().catch(() => {});
}
_activeProbeRunners = null;
}
_environment = null;
setUseCacheProbe(undefined);
}

/**
* Reconstruct a minimal request store in the probe runner so that
* cookies(), headers(), and draftMode() behave correctly.
*/
async function runWithProbeRequestStore<T>(
requestSnapshot: {
headers: [string, string][];
cookieHeader: string | undefined;
urlPathname: string;
urlSearch: string;
rootParams: Record<string, unknown>;
isDraftMode: boolean;
isHmrRefresh: boolean;
},
fn: () => Promise<T>,
): Promise<T> {
// Import the ALS-backed request-context modules in the isolated runner.
const unifiedCtx = (async () => {
// These imports run inside the probe runner's module graph.
// We dynamic-import them because the probe runner doesn't share
// module state with the main runner.
const { createRequestContext, runWithRequestContext } =
(await import("vinext/shims/unified-request-context")) as typeof import("vinext/shims/unified-request-context");
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical: These import() calls use the host process's native module resolution — they load the main runner's module instances, not the probe runner's isolated graph. The comment says "These imports run inside the probe runner's module graph" but that's not what happens with native import().

To actually run in the probe's isolated graph, these would need to go through runner.import("vinext/shims/unified-request-context") and runner.import("vinext/shims/headers"). But the function doesn't have access to the runner — it would need to be passed in or this logic restructured.

As-is, the ALS context is set on the main runner's modules, so cookies()/headers() inside the probe function will read from the main runner's ALS, not the isolated one. This defeats the purpose of request-store reconstruction.


const { headersContextFromRequest } =
(await import("vinext/shims/headers")) as typeof import("vinext/shims/headers");

// Build a Request from the snapshot so headersContextFromRequest works.
const url = new URL(
requestSnapshot.urlPathname + requestSnapshot.urlSearch,
"http://localhost",
);
const request = new Request(url, {
headers: new Headers(requestSnapshot.headers),
});

const headersContext = headersContextFromRequest(request);
const ctx = createRequestContext({
headersContext,
executionContext: null,
rootParams: requestSnapshot.rootParams as Record<string, string | string[]>,
});

return runWithRequestContext(ctx, fn);
})();

return unifiedCtx;
}
Loading
Loading