From 0f7251fc78db5c774f82285bcd1eed8e73564bb7 Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Thu, 30 Jan 2025 08:48:50 -0800 Subject: [PATCH] fix(dev-server): Terminate turbopack HMR websocket connections during exit (#75344) Without this change, an open browser tab with an HMR websocket subscription (which is very common) can prevent next.js's dev server from exiting cleanly (falls back to a SIGKILL after a 100ms timeout). That's bad (aside from the extra 100ms delay) because it prevents ``` await Promise.all([ nextServer?.close().catch(console.error), cleanupListeners?.runAll().catch(console.error), ]) ``` from running, which can prevent us from flushing task statistics (https://github.com/vercel/next.js/pull/67164). You can test this with: ``` NEXT_EXIT_TIMEOUT_MS=200000 node_modules/.bin/next dev ``` or ``` NEXT_EXIT_TIMEOUT_MS=200000 node_modules/.bin/next dev --turbo ``` Prior to this PR, after hitting ctrl+c, the processes will hang if there's a connected browser window open. After this PR, the processes exit instantly. --- .../next/src/server/dev/hot-middleware.ts | 19 +++++++------------ .../src/server/dev/hot-reloader-turbopack.ts | 7 +++++++ .../next/src/server/dev/hot-reloader-types.ts | 1 + .../src/server/dev/hot-reloader-webpack.ts | 4 ++++ .../src/server/lib/dev-bundler-service.ts | 4 ++++ packages/next/src/server/lib/render-server.ts | 5 +++++ packages/next/src/server/lib/router-server.ts | 9 ++++++++- packages/next/src/server/lib/start-server.ts | 10 ++++++++-- 8 files changed, 44 insertions(+), 15 deletions(-) diff --git a/packages/next/src/server/dev/hot-middleware.ts b/packages/next/src/server/dev/hot-middleware.ts index b3af17caa1823..952bf60c27772 100644 --- a/packages/next/src/server/dev/hot-middleware.ts +++ b/packages/next/src/server/dev/hot-middleware.ts @@ -72,16 +72,11 @@ class EventStream { this.clients = new Set() } - everyClient(fn: (client: ws) => void) { - for (const client of this.clients) { - fn(client) - } - } - close() { - this.everyClient((client) => { - client.close() - }) + for (const wsClient of this.clients) { + // it's okay to not cleanly close these websocket connections, this is dev + wsClient.terminate() + } this.clients.clear() } @@ -93,9 +88,9 @@ class EventStream { } publish(payload: any) { - this.everyClient((client) => { - client.send(JSON.stringify(payload)) - }) + for (const wsClient of this.clients) { + wsClient.send(JSON.stringify(payload)) + } } } diff --git a/packages/next/src/server/dev/hot-reloader-turbopack.ts b/packages/next/src/server/dev/hot-reloader-turbopack.ts index 2b224932a9ae4..721bb9d31627b 100644 --- a/packages/next/src/server/dev/hot-reloader-turbopack.ts +++ b/packages/next/src/server/dev/hot-reloader-turbopack.ts @@ -1059,6 +1059,13 @@ export async function createHotReloaderTurbopack( } }) }, + close() { + for (const wsClient of clients) { + // it's okay to not cleanly close these websocket connections, this is dev + wsClient.terminate() + } + clients.clear() + }, } handleEntrypointsSubscription().catch((err) => { diff --git a/packages/next/src/server/dev/hot-reloader-types.ts b/packages/next/src/server/dev/hot-reloader-types.ts index f7e014f331e96..e8e27661a3847 100644 --- a/packages/next/src/server/dev/hot-reloader-types.ts +++ b/packages/next/src/server/dev/hot-reloader-types.ts @@ -182,4 +182,5 @@ export interface NextJsHotReloaderInterface { definition: RouteDefinition | undefined url?: string }): Promise + close(): void } diff --git a/packages/next/src/server/dev/hot-reloader-webpack.ts b/packages/next/src/server/dev/hot-reloader-webpack.ts index 27772d9127a77..7d0e16d04061a 100644 --- a/packages/next/src/server/dev/hot-reloader-webpack.ts +++ b/packages/next/src/server/dev/hot-reloader-webpack.ts @@ -1595,4 +1595,8 @@ export default class HotReloaderWebpack implements NextJsHotReloaderInterface { }) }) } + + public close() { + this.webpackHotMiddleware?.close() + } } diff --git a/packages/next/src/server/lib/dev-bundler-service.ts b/packages/next/src/server/lib/dev-bundler-service.ts index 3cd148df9d7a5..6b2b35296468a 100644 --- a/packages/next/src/server/lib/dev-bundler-service.ts +++ b/packages/next/src/server/lib/dev-bundler-service.ts @@ -103,4 +103,8 @@ export class DevBundlerService { data: this.appIsrManifest, }) } + + public close() { + this.bundler.hotReloader.close() + } } diff --git a/packages/next/src/server/lib/render-server.ts b/packages/next/src/server/lib/render-server.ts index e9834930ba523..c50a73d5c106a 100644 --- a/packages/next/src/server/lib/render-server.ts +++ b/packages/next/src/server/lib/render-server.ts @@ -9,6 +9,8 @@ export type ServerInitResult = { requestHandler: RequestHandler upgradeHandler: UpgradeHandler server: NextServer + // Make an effort to close upgraded HTTP requests (e.g. Turbopack HMR websockets) + closeUpgraded: () => void } let initializations: Record | undefined> = {} @@ -109,6 +111,9 @@ async function initializeImpl(opts: { requestHandler, upgradeHandler, server, + closeUpgraded() { + opts.bundlerService?.close() + }, } } diff --git a/packages/next/src/server/lib/router-server.ts b/packages/next/src/server/lib/router-server.ts index 5c1750d2a1c89..3ac0474555b6e 100644 --- a/packages/next/src/server/lib/router-server.ts +++ b/packages/next/src/server/lib/router-server.ts @@ -751,5 +751,12 @@ export async function initialize(opts: { } } - return { requestHandler, upgradeHandler, server: handlers.server } + return { + requestHandler, + upgradeHandler, + server: handlers.server, + closeUpgraded() { + developmentBundler?.hotReloader?.close() + }, + } } diff --git a/packages/next/src/server/lib/start-server.ts b/packages/next/src/server/lib/start-server.ts index 87cd0440fa252..8ec1ad1b671d1 100644 --- a/packages/next/src/server/lib/start-server.ts +++ b/packages/next/src/server/lib/start-server.ts @@ -289,6 +289,7 @@ export async function startServer( try { let cleanupStarted = false + let closeUpgraded: (() => void) | null = null const cleanup = () => { if (cleanupStarted) { // We can get duplicate signals, e.g. when `ctrl+c` is used in an @@ -303,12 +304,16 @@ export async function startServer( // first, stop accepting new connections and finish pending requests, // because they might affect `nextServer.close()` (e.g. by scheduling an `after`) - await new Promise((res) => + await new Promise((res) => { server.close((err) => { if (err) console.error(err) res() }) - ) + if (isDev) { + server.closeAllConnections() + closeUpgraded?.() + } + }) // now that no new requests can come in, clean up the rest await Promise.all([ @@ -360,6 +365,7 @@ export async function startServer( requestHandler = initResult.requestHandler upgradeHandler = initResult.upgradeHandler nextServer = initResult.server + closeUpgraded = initResult.closeUpgraded const startServerProcessDuration = performance.mark('next-start-end') &&