next-dev: change cluster usage to child process + stabilise inspect port (#45745

feedthejim · web-flow · commit be5fd6cee003 · 2023-02-10T11:20:50.000+01:00
Fixes #45122 and #44817 NEXT-447 This PR: - changes the usage of the cluster module in `next dev` to use `child_process` instead. This is mostly the same thing and helps alleviate issues with libraries that rely on `Cluster.isMaster`. - fixes the implementation also to check if the `--inspect` option is used to then manually attribute the correct port, which is whatever the current port is, plus one. With the previous cluster implementation, it always increased automatically every time the server died. - adds back the warning log I had removed by mistake to inform the user that the server is restarting.  ## Bug - [ ] Related issues linked using `fixes #number` - [ ] Integration tests added - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Feature - [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR. - [ ] Related issues linked using `fixes #number` - [ ] [e2e](https://github.com/vercel/next.js/blob/canary/contributing/core/testing.md#writing-tests-for-nextjs) tests added - [ ] Documentation added - [ ] Telemetry added. In case of a feature if it's used or not. - [ ] Errors have a helpful link attached, see [`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md) ## Documentation / Examples - [ ] Make sure the linting passes by running `pnpm build && pnpm lint` - [ ] The "examples guidelines" are followed from [our contributing doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)
diff --git a/packages/next/src/cli/next-dev.ts b/packages/next/src/cli/next-dev.ts
@@ -1,6 +1,6 @@
 #!/usr/bin/env node
 import arg from 'next/dist/compiled/arg/index.js'
-import { startServer } from '../server/lib/start-server'
+import { startServer, WORKER_SELF_EXIT_CODE } from '../server/lib/start-server'
 import { getPort, printAndExit } from '../server/lib/utils'
 import * as Log from '../build/output/log'
 import { startedDevelopmentServer } from '../build/output'
@@ -13,20 +13,23 @@ import type { NextConfig } from '../../types'
 import type { NextConfigComplete } from '../server/config-shared'
 import { traceGlobals } from '../trace/shared'
 import { isIPv6 } from 'net'
-import cluster from 'cluster'
+import { ChildProcess, fork } from 'child_process'
 import { Telemetry } from '../telemetry/storage'
 import loadConfig from '../server/config'
 import { findPagesDir } from '../lib/find-pages-dir'
 import { fileExists } from '../lib/file-exists'
 import Watchpack from 'next/dist/compiled/watchpack'
 import stripAnsi from 'next/dist/compiled/strip-ansi'
+import { warn } from '../build/output/log'
 
 let isTurboSession = false
 let sessionStopHandled = false
 let sessionStarted = Date.now()
 let dir: string
 let unwatchConfigFiles: () => void
 
+const isChildProcess = !!process.env.__NEXT_DEV_CHILD_PROCESS
+
 const handleSessionStop = async () => {
   if (sessionStopHandled) return
   sessionStopHandled = true
@@ -81,7 +84,7 @@ const handleSessionStop = async () => {
   process.exit(0)
 }
 
-if (cluster.isMaster) {
+if (!isChildProcess) {
   process.on('SIGINT', handleSessionStop)
   process.on('SIGTERM', handleSessionStop)
 } else {
@@ -432,16 +435,75 @@ If you cannot make the changes above, but still want to try out\nNext.js v13 wit
     // we're using a sub worker to avoid memory leaks. When memory usage exceeds 90%, we kill the worker and restart it.
     // this is a temporary solution until we can fix the memory leaks.
     // the logic for the worker killing itself is in `packages/next/server/lib/start-server.ts`
-    if (!process.env.__NEXT_DISABLE_MEMORY_WATCHER && cluster.isMaster) {
+    if (!process.env.__NEXT_DISABLE_MEMORY_WATCHER && !isChildProcess) {
       let config: NextConfig
+      let childProcess: ChildProcess | null = null
+
+      const isDebugging = process.execArgv.some((localArg) =>
+        localArg.startsWith('--inspect')
+      )
+
+      const isDebuggingWithBrk = process.execArgv.some((localArg) =>
+        localArg.startsWith('--inspect-brk')
+      )
+
+      const debugPort = (() => {
+        const debugPortStr = process.execArgv
+          .find(
+            (localArg) =>
+              localArg.startsWith('--inspect') ||
+              localArg.startsWith('--inspect-brk')
+          )
+          ?.split('=')[1]
+        return debugPortStr ? parseInt(debugPortStr, 10) : 9229
+      })()
+
+      if (isDebugging || isDebuggingWithBrk) {
+        warn(
+          `the --inspect${
+            isDebuggingWithBrk ? '-brk' : ''
+          } option was detected, the Next.js server should be inspected at port ${
+            debugPort + 1
+          }.`
+        )
+      }
+
+      const genExecArgv = () => {
+        const execArgv = process.execArgv.filter((localArg) => {
+          return (
+            !localArg.startsWith('--inspect') &&
+            !localArg.startsWith('--inspect-brk')
+          )
+        })
 
-      const setupFork = (env?: Parameters<typeof cluster.fork>[0]) => {
+        if (isDebugging || isDebuggingWithBrk) {
+          execArgv.push(
+            `--inspect${isDebuggingWithBrk ? '-brk' : ''}=${debugPort + 1}`
+          )
+        }
+
+        return execArgv
+      }
+
+      const setupFork = (env?: NodeJS.ProcessEnv, newDir?: string) => {
         const startDir = dir
+        const [, script, ...nodeArgs] = process.argv
         let shouldFilter = false
-        cluster.fork({
-          ...env,
-          FORCE_COLOR: '1',
-        })
+        childProcess = fork(
+          newDir ? script.replace(startDir, newDir) : script,
+          nodeArgs,
+          {
+            env: {
+              ...(env ? env : process.env),
+              FORCE_COLOR: '1',
+              __NEXT_DEV_CHILD_PROCESS: '1',
+            },
+            // @ts-ignore TODO: remove ignore when types are updated
+            windowsHide: true,
+            stdio: ['ipc', 'pipe', 'pipe'],
+            execArgv: genExecArgv(),
+          }
+        )
 
         // since errors can start being logged from the fork
         // before we detect the project directory rename
@@ -473,40 +535,27 @@ If you cannot make the changes above, but still want to try out\nNext.js v13 wit
           process[fd].write(chunk)
         }
 
-        for (const workerId in cluster.workers) {
-          cluster.workers[workerId]?.process.stdout?.on('data', (chunk) => {
-            filterForkErrors(chunk, 'stdout')
-          })
-          cluster.workers[workerId]?.process.stderr?.on('data', (chunk) => {
-            filterForkErrors(chunk, 'stderr')
-          })
-        }
-      }
-
-      const handleClusterExit = () => {
-        const callback = async (worker: cluster.Worker) => {
-          // ignore if we killed the worker
-          if ((worker as any).killed) return
+        childProcess?.stdout?.on('data', (chunk) => {
+          filterForkErrors(chunk, 'stdout')
+        })
+        childProcess?.stderr?.on('data', (chunk) => {
+          filterForkErrors(chunk, 'stderr')
+        })
 
-          // TODO: we should track how many restarts are
-          // occurring and how long in-between them
-          if (worker.exitedAfterDisconnect) {
+        const callback = async (code: number | null) => {
+          if (code === WORKER_SELF_EXIT_CODE) {
             setupFork()
           } else if (!sessionStopHandled) {
             await handleSessionStop()
             process.exit(1)
           }
         }
-        cluster.addListener('exit', callback)
-        return () => cluster.removeListener('exit', callback)
+        childProcess?.addListener('exit', callback)
+        return () => childProcess?.removeListener('exit', callback)
       }
-      let clusterExitUnsub = handleClusterExit()
-      // x-ref: https://nodejs.org/api/cluster.html#clustersettings
-      // @ts-expect-error type is incorrect
-      cluster.settings.windowsHide = true
-      cluster.settings.stdio = ['ipc', 'pipe', 'pipe']
 
-      setupFork()
+      let childProcessExitUnsub = setupFork()
+
       config = await loadConfig(
         PHASE_DEVELOPMENT_SERVER,
         dir,
@@ -516,27 +565,19 @@ If you cannot make the changes above, but still want to try out\nNext.js v13 wit
       )
 
       const handleProjectDirRename = (newDir: string) => {
-        clusterExitUnsub()
-
-        for (const workerId in cluster.workers) {
-          try {
-            // @ts-expect-error custom field
-            cluster.workers[workerId].killed = true
-            cluster.workers[workerId]!.process.kill('SIGKILL')
-          } catch (_) {}
-        }
+        childProcessExitUnsub()
+        childProcess?.kill()
         process.chdir(newDir)
-        // @ts-expect-error type is incorrect
-        cluster.settings.cwd = newDir
-        cluster.settings.exec = cluster.settings.exec?.replace(dir, newDir)
-        setupFork({
-          ...Object.keys(process.env).reduce((newEnv, key) => {
-            newEnv[key] = process.env[key]?.replace(dir, newDir)
-            return newEnv
-          }, {} as typeof process.env),
-          NEXT_PRIVATE_DEV_DIR: newDir,
-        })
-        clusterExitUnsub = handleClusterExit()
+        childProcessExitUnsub = setupFork(
+          {
+            ...Object.keys(process.env).reduce((newEnv, key) => {
+              newEnv[key] = process.env[key]?.replace(dir, newDir)
+              return newEnv
+            }, {} as typeof process.env),
+            NEXT_PRIVATE_DEV_DIR: newDir,
+          },
+          newDir
+        )
       }
       const parentDir = path.join('/', dir, '..')
       const watchedEntryLength = parentDir.split('/').length + 1
diff --git a/packages/next/src/server/lib/start-server.ts b/packages/next/src/server/lib/start-server.ts
@@ -3,14 +3,16 @@ import { warn } from '../../build/output/log'
 import http from 'http'
 import next from '../next'
 import { isIPv6 } from 'net'
-import cluster from 'cluster'
 import v8 from 'v8'
+const isChildProcess = !!process.env.__NEXT_DEV_CHILD_PROCESS
 
 interface StartServerOptions extends NextServerOptions {
   allowRetry?: boolean
   keepAliveTimeout?: number
 }
 
+export const WORKER_SELF_EXIT_CODE = 77
+
 const MAXIMUM_HEAP_SIZE_ALLOWED =
   (v8.getHeapStatistics().heap_size_limit / 1024 / 1024) * 0.9
 
@@ -20,10 +22,14 @@ export function startServer(opts: StartServerOptions) {
   const server = http.createServer((req, res) => {
     return requestHandler(req, res).finally(() => {
       if (
-        cluster.worker &&
+        isChildProcess &&
         process.memoryUsage().heapUsed / 1024 / 1024 > MAXIMUM_HEAP_SIZE_ALLOWED
       ) {
-        cluster.worker.kill()
+        warn(
+          'The server is running out of memory, restarting to free up memory.'
+        )
+        server.close()
+        process.exit(WORKER_SELF_EXIT_CODE)
       }
     })
   })
