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

Co-authored-by: bluwy <bjornlu.dev@gmail.com>

Author: shYkiSto

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()
+}