Skip to content

fix(module-runner): prevent partial-exports race on concurrent imports of in-flight invalidated re-export chains#22369

Open
schiller-manuel wants to merge 1 commit intovitejs:mainfrom
schiller-manuel:fix-server-hmr
Open

fix(module-runner): prevent partial-exports race on concurrent imports of in-flight invalidated re-export chains#22369
schiller-manuel wants to merge 1 commit intovitejs:mainfrom
schiller-manuel:fix-server-hmr

Conversation

@schiller-manuel
Copy link
Copy Markdown

Summary

Fixes an SSR/HMR module runner race where a concurrent import can observe an incomplete namespace from an in-flight module evaluation.

The same shape reproduces in any project where:

  1. Two entry modules share a dependency that does export * from a deeper module
  2. The shared dependency has a real cycle with one of the entries
  3. HMR invalidates the chain and two requests race through it

The runner mis-detects the second request as "circular" and hands it the partial in-flight mod.exports object before export * has populated it, exposing an empty namespace to user code.

Root Cause

ModuleRunner.cachedRequest decides between returning partial mod.exports (cycle break) vs. await mod.promise (wait for full evaluation). The pre-fix logic used three signals; two of them consulted mod.importers:

if (
  callstack.includes(moduleId) ||
  this.isCircularModule(mod) ||                 // mod.imports ∩ mod.importers
  this.isCircularImport(importers, moduleId)    // BFS over mod.importers
) {
  if (mod.exports) return mod.exports
}

mod.importers is a monotonic set, never cleared by invalidateModule. After HMR it carries every historical caller. A new concurrent caller is therefore frequently mis-classified as "in a cycle" with a stale importer, and receives an empty exports object.

Walkthrough

Module graph

entry-a --\
           +--> shared --> core   (core gated on async work; wildcard-reexported by shared)
entry-b --/      ^
                 |
                 +-- import './entry-a.js'  (real cycle: shared <-> entry-a)

Old Behavior

  1. Cold start: both entries import successfully. shared.importers = { entry-a, entry-b }, shared.imports = { entry-a, core }.
  2. HMR invalidates the chain. evaluated/promise/exports reset; importers preserved.
  3. entry-a re-imports -> reaches shared.directRequest -> reaches core -> suspends on async work. shared.promise set, shared.evaluated = false, shared.exports = {} (empty placeholder).
  4. entry-b concurrently re-imports -> reaches cachedRequest(shared) with callstack = [entry-b].
  5. Cycle check runs:
    • callstack.includes(shared) -> false
    • isCircularModule(shared): shared.imports = { entry-a, core } intersects shared.importers = { entry-a, entry-b } -> true (entry-a is in both)
  6. Returns shared.exports, which is empty. entry-b body executes createThing(...) against an empty namespace -> TypeError: createThing is not a function.

New Behavior

Replace the importer-based heuristics with a callstack-driven forward-graph check:

if (mod.promise) {
  if (
    mod.exports &&
    (callstack.includes(moduleId) || this.isCircularRequest(mod, callstack))
  ) {
    return this.processImport(mod.exports, meta, metadata)
  }
  return this.processImport(await mod.promise, meta, metadata)
}

isCircularRequest(mod, callstack) walks mod.imports transitively (bounded by visited, descent halted at already-evaluated modules) and asks the only question that matters for deadlock: does anything mod is waiting on appear in my current callstack?

Replaying the same scenario:

  1. callstack = [entry-b]. Walk shared.imports = { entry-a, core }:
    • entry-a not in [entry-b]; recurse: entry-a.imports = { shared }; shared already visited -> false
    • core not in [entry-b]; core.imports = {} -> false
    • returns false
  2. Falls through to await mod.promise. entry-b waits until shared finishes evaluating, then sees the fully populated namespace.

Real cycles still break correctly: when shared re-enters entry-a during entry-a's own evaluation, callstack = [entry-a, shared] -> callstack.includes(entry-a) -> returns partial exports (ESM spec-correct cycle break).

Why Previous Heuristics Fail

  • isCircularModule: cannot distinguish "I am in a cycle with my caller" from "I have ever been in a cycle with anyone".
  • isCircularImport: mod.importers is a historical reverse-edge set, not a runtime callstack. Across HMR it grows unboundedly. The check is also O(importers^2) in the worst case.

The new check is local to the live request, O(V+E) over the forward subgraph (visited-bounded), and matches the actual deadlock condition.

Performance

Test

Adds does not expose partial exports during concurrent updates in server-hmr.spec.ts with fixtures under __tests__/fixtures/hmr-reexport-race/.

The test:

  1. Cold-imports entry-a and entry-b (both import shared, which export * from core and has a self-cycle to entry-a).
  2. Invalidates the chain.
  3. Installs a global gate that suspends core's top-level await.
  4. Starts entry-a re-import; awaits "gate hit".
  5. Starts entry-b re-import; flushes a macrotask; releases the gate.
  6. Asserts both promises fulfill with the full namespace and HMR listeners are intact.

The test fails on main and passes with the fix.

Refs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SSR HMR: createStartHandler is not a function on every re-evaluation (re-occurrence of #5673, 1.167.50)

1 participant