fix(ssr): resolve interlocking circular dependency issues (#15395)

Co-authored-by: bluwy <[email protected]>

packages/vite/src/node/server/index.ts

@@ -563,13 +563,7 @@ export async function _createServer(
  return devHtmlTransformFn(server, url, html, originalUrl)
  },
  async ssrLoadModule(url, opts?: { fixStacktrace?: boolean }) {
- return ssrLoadModule(
- url,
- server,
- undefined,
- undefined,
- opts?.fixStacktrace,
- )
+ return ssrLoadModule(url, server, undefined, opts?.fixStacktrace)
  },
  async ssrFetchModule(url: string, importer?: string) {
  return ssrFetchModule(server, url, importer)

packages/vite/src/node/ssr/ssrModuleLoader.ts

@@ -39,14 +39,13 @@ interface NodeImportResolveOptions
 }
 
 const pendingModules = new Map<string, Promise<SSRModule>>()
-const pendingImports = new Map<string, string[]>()
+const pendingModuleDependencyGraph = new Map<string, Set<string>>()
 const importErrors = new WeakMap<Error, { importee: string }>()
 
 export async function ssrLoadModule(
  url: string,
  server: ViteDevServer,
  context: SSRContext = { global },
- urlStack: string[] = [],
  fixStacktrace?: boolean,
 ): Promise<SSRModule> {
  url = unwrapId(url)
@@ -60,17 +59,11 @@ export async function ssrLoadModule(
  return pending
  }
 
- const modulePromise = instantiateModule(
- url,
- server,
- context,
- urlStack,
- fixStacktrace,
- )
+ const modulePromise = instantiateModule(url, server, context, fixStacktrace)
  pendingModules.set(url, modulePromise)
  modulePromise
  .catch(() => {
- pendingImports.delete(url)
+ /* prevent unhandled promise rejection error from bubbling up */
  })
  .finally(() => {
  pendingModules.delete(url)
@@ -82,7 +75,6 @@ async function instantiateModule(
  url: string,
  server: ViteDevServer,
  context: SSRContext = { global },
- urlStack: string[] = [],
  fixStacktrace?: boolean,
 ): Promise<SSRModule> {
  const { moduleGraph } = server
@@ -122,9 +114,6 @@ async function instantiateModule(
  url: pathToFileURL(mod.file!).toString(),
  }
 
- urlStack = urlStack.concat(url)
- const isCircular = (url: string) => urlStack.includes(url)
-
  const {
  isProduction,
  resolve: { dedupe, preserveSymlinks },
@@ -150,38 +139,37 @@ async function instantiateModule(
  packageCache: server.config.packageCache,
  }
 
- // Since dynamic imports can happen in parallel, we need to
- // account for multiple pending deps and duplicate imports.
- const pendingDeps: string[] = []
-
  const ssrImport = async (dep: string, metadata?: SSRImportBaseMetadata) => {
  try {
  if (dep[0] !== '.' && dep[0] !== '/') {
  return await nodeImport(dep, mod.file!, resolveOptions, metadata)
  }
  // convert to rollup URL because `pendingImports`, `moduleGraph.urlToModuleMap` requires that
  dep = unwrapId(dep)
- if (!isCircular(dep) && !pendingImports.get(dep)?.some(isCircular)) {
- pendingDeps.push(dep)
- if (pendingDeps.length === 1) {
- pendingImports.set(url, pendingDeps)
- }
- const mod = await ssrLoadModule(
- dep,
- server,
- context,
- urlStack,
- fixStacktrace,
- )
- if (pendingDeps.length === 1) {
- pendingImports.delete(url)
- } else {
- pendingDeps.splice(pendingDeps.indexOf(dep), 1)
+
+ // Handle any potential circular dependencies for static imports, preventing
+ // deadlock scenarios when two modules are indirectly waiting on one another
+ // to finish initializing. Dynamic imports are resolved at runtime, hence do
+ // not contribute to the static module dependency graph in the same way
+ if (!metadata?.isDynamicImport) {
+ addPendingModuleDependency(url, dep)
+
+ // If there's a circular dependency formed as a result of the dep import,
+ // return the current state of the dependent module being initialized, in
+ // order to avoid interlocking circular dependencies hanging indefinitely
+ if (checkModuleDependencyExists(dep, url)) {
+ const depSsrModule = moduleGraph.urlToModuleMap.get(dep)?.ssrModule
+ if (!depSsrModule) {
+ // Technically, this should never happen under normal circumstances
+ throw new Error(
+ '[vite] The dependency module is not yet fully initialized due to circular dependency. This is a bug in Vite SSR',
+ )
+ }
+ return depSsrModule
  }
- // return local module to avoid race condition #5470
- return mod
  }
- return moduleGraph.urlToModuleMap.get(dep)?.ssrModule
+
+ return ssrLoadModule(dep, server, context, fixStacktrace)
  } catch (err) {
  // tell external error handler which mod was imported with error
  importErrors.set(err, { importee: dep })
@@ -267,11 +255,52 @@ async function instantiateModule(
  )
 
  throw e
+ } finally {
+ pendingModuleDependencyGraph.delete(url)
  }
 
  return Object.freeze(ssrModule)
 }
 
+function addPendingModuleDependency(originUrl: string, depUrl: string): void {
+ if (pendingModuleDependencyGraph.has(originUrl)) {
+ pendingModuleDependencyGraph.get(originUrl)!.add(depUrl)
+ } else {
+ pendingModuleDependencyGraph.set(originUrl, new Set([depUrl]))
+ }
+}
+
+function checkModuleDependencyExists(
+ originUrl: string,
+ targetUrl: string,
+): boolean {
+ const visited = new Set()
+ const stack = [originUrl]
+
+ while (stack.length) {
+ const currentUrl = stack.pop()!
+
+ if (currentUrl === targetUrl) {
+ return true
+ }
+
+ if (!visited.has(currentUrl)) {
+ visited.add(currentUrl)
+
+ const dependencies = pendingModuleDependencyGraph.get(currentUrl)
+ if (dependencies) {
+ for (const depUrl of dependencies) {
+ if (!visited.has(depUrl)) {
+ stack.push(depUrl)
+ }
+ }
+ }
+ }
+ }
+
+ return false
+}
+
 // In node@12+ we can use dynamic import to load CJS and ESM
 async function nodeImport(
  id: string,

playground/ssr/__tests__/ssr.spec.ts

@@ -20,10 +20,20 @@ test(`circular import doesn't throw`, async () => {
  )
 })
 
-test(`deadlock doesn't happen`, async () => {
- await page.goto(`${url}/forked-deadlock`)
+test(`deadlock doesn't happen for static imports`, async () => {
+ await page.goto(`${url}/forked-deadlock-static-imports`)
 
- expect(await page.textContent('.forked-deadlock')).toMatch('rendered')
+ expect(await page.textContent('.forked-deadlock-static-imports')).toMatch(
+ 'rendered',
+ )
+})
+
+test(`deadlock doesn't happen for dynamic imports`, async () => {
+ await page.goto(`${url}/forked-deadlock-dynamic-imports`)
+
+ expect(await page.textContent('.forked-deadlock-dynamic-imports')).toMatch(
+ 'rendered',
+ )
 })
 
 test('should restart ssr', async () => {

playground/ssr/src/app.js

@@ -4,7 +4,8 @@ const pathRenderers = {
  '/': renderRoot,
  '/circular-dep': renderCircularDep,
  '/circular-import': renderCircularImport,
- '/forked-deadlock': renderForkedDeadlock,
+ '/forked-deadlock-static-imports': renderForkedDeadlockStaticImports,
+ '/forked-deadlock-dynamic-imports': renderForkedDeadlockDynamicImports,
 }
 
 export async function render(url, rootDir) {
@@ -40,8 +41,16 @@ async function renderCircularImport(rootDir) {
  return `<div class="circ-import">${escapeHtml(logA())}</div>`
 }
 
-async function renderForkedDeadlock(rootDir) {
+async function renderForkedDeadlockStaticImports(rootDir) {
  const { commonModuleExport } = await import('./forked-deadlock/common-module')
  commonModuleExport()
- return `<div class="forked-deadlock">rendered</div>`
+ return `<div class="forked-deadlock-static-imports">rendered</div>`
+}
+
+async function renderForkedDeadlockDynamicImports(rootDir) {
+ const { commonModuleExport } = await import(
+ './forked-deadlock/dynamic-imports/common-module'
+ )
+ await commonModuleExport()
+ return `<div class="forked-deadlock-dynamic-imports">rendered</div>`
 }

playground/ssr/src/forked-deadlock/dynamic-imports/common-module.js

@@ -0,0 +1,13 @@
+/**
+ * module H
+ */
+export async function commonModuleExport() {
+ const [{ stuckModuleExport }, { deadlockfuseModuleExport }] =
+ await Promise.all([
+ import('./stuck-module'),
+ import('./deadlock-fuse-module'),
+ ])
+
+ stuckModuleExport()
+ deadlockfuseModuleExport()
+}

playground/ssr/src/forked-deadlock/dynamic-imports/deadlock-fuse-module.js

@@ -0,0 +1,8 @@
+import { fuseStuckBridgeModuleExport } from './fuse-stuck-bridge-module'
+
+/**
+ * module A
+ */
+export function deadlockfuseModuleExport() {
+ fuseStuckBridgeModuleExport()
+}

playground/ssr/src/forked-deadlock/dynamic-imports/fuse-stuck-bridge-module.js

@@ -0,0 +1,8 @@
+import { stuckModuleExport } from './stuck-module'
+
+/**
+ * module C
+ */
+export function fuseStuckBridgeModuleExport() {
+ stuckModuleExport()
+}

playground/ssr/src/forked-deadlock/dynamic-imports/middle-module.js

@@ -0,0 +1,8 @@
+import { deadlockfuseModuleExport } from './deadlock-fuse-module'
+
+/**
+ * module Y
+ */
+export function middleModuleExport() {
+ void deadlockfuseModuleExport
+}

playground/ssr/src/forked-deadlock/dynamic-imports/stuck-module.js

@@ -0,0 +1,8 @@
+import { middleModuleExport } from './middle-module'
+
+/**
+ * module X
+ */
+export function stuckModuleExport() {
+ middleModuleExport()
+}