Skip to content

Commit

Permalink
fix(ssr): resolve interlocking circular dependency issues (#15395)
Browse files Browse the repository at this point in the history
Co-authored-by: bluwy <bjornlu.dev@gmail.com>
  • Loading branch information
shYkiSto and bluwy authored Jun 6, 2024
1 parent 67ff94b commit 687c38b
Show file tree
Hide file tree
Showing 9 changed files with 137 additions and 50 deletions.
8 changes: 1 addition & 7 deletions packages/vite/src/node/server/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
103 changes: 66 additions & 37 deletions packages/vite/src/node/ssr/ssrModuleLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -82,7 +75,6 @@ async function instantiateModule(
url: string,
server: ViteDevServer,
context: SSRContext = { global },
urlStack: string[] = [],
fixStacktrace?: boolean,
): Promise<SSRModule> {
const { moduleGraph } = server
Expand Down Expand Up @@ -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 },
Expand All @@ -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 })
Expand Down Expand Up @@ -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,
Expand Down
16 changes: 13 additions & 3 deletions playground/ssr/__tests__/ssr.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 () => {
Expand Down
15 changes: 12 additions & 3 deletions playground/ssr/src/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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>`
}
Original file line number Diff line number Diff line change
@@ -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()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { fuseStuckBridgeModuleExport } from './fuse-stuck-bridge-module'

/**
* module A
*/
export function deadlockfuseModuleExport() {
fuseStuckBridgeModuleExport()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { stuckModuleExport } from './stuck-module'

/**
* module C
*/
export function fuseStuckBridgeModuleExport() {
stuckModuleExport()
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { deadlockfuseModuleExport } from './deadlock-fuse-module'

/**
* module Y
*/
export function middleModuleExport() {
void deadlockfuseModuleExport
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { middleModuleExport } from './middle-module'

/**
* module X
*/
export function stuckModuleExport() {
middleModuleExport()
}

0 comments on commit 687c38b

Please sign in to comment.