Skip to content

Commit

Permalink
Allow dev server to exit cleanly (SIGINT/SIGTERM)
Browse files Browse the repository at this point in the history
Previously we were sending SIGKILL immediately to the child. This
ensures that the child exits as quickly as possible, but it also means
that the child probably won't have time to finish flushing files to
disk (trace files, cache hit statistics, etc.), and can lead to
incomplete writes.

We should give the child a small amount of time to exit cleanly with
SIGINT/SIGTERM before sending SIGKILL. The timeout is needed in case the
child process is unresponsive/hanging.
  • Loading branch information
bgw committed Jul 9, 2024
1 parent e8e9212 commit d7a49d5
Showing 1 changed file with 21 additions and 5 deletions.
26 changes: 21 additions & 5 deletions packages/next/src/cli/next-dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ import {
} from '../lib/helpers/get-reserved-port'
import os from 'os'
import { once } from 'node:events'
import { clearTimeout } from 'timers'

type NextDevOptions = {
turbo?: boolean
Expand All @@ -57,13 +58,26 @@ let traceUploadUrl: string
let sessionStopHandled = false
let sessionStarted = Date.now()

// How long should we wait for the child to cleanly exit after sending
// SIGINT/SIGTERM to the child process before sending SIGKILL?
const CHILD_EXIT_TIMEOUT_MS = 1000

const handleSessionStop = async (signal: NodeJS.Signals | number | null) => {
if (child?.pid) child.kill(signal ?? 0)
if (signal != null && child?.pid) child.kill(signal)
if (sessionStopHandled) return
sessionStopHandled = true

if (child?.pid && child.exitCode === null && child.signalCode === null) {
if (
signal != null &&
child?.pid &&
child.exitCode === null &&
child.signalCode === null
) {
let exitTimeout = setTimeout(() => {
child?.kill('SIGKILL')
}, CHILD_EXIT_TIMEOUT_MS)
await once(child, 'exit').catch(() => {})
clearTimeout(exitTimeout)
}

try {
Expand Down Expand Up @@ -124,8 +138,8 @@ const handleSessionStop = async (signal: NodeJS.Signals | number | null) => {
process.exit(0)
}

process.on('SIGINT', () => handleSessionStop('SIGKILL'))
process.on('SIGTERM', () => handleSessionStop('SIGKILL'))
process.on('SIGINT', () => handleSessionStop('SIGINT'))
process.on('SIGTERM', () => handleSessionStop('SIGTERM'))

// exit event must be synchronous
process.on('exit', () => child?.kill('SIGKILL'))
Expand Down Expand Up @@ -291,7 +305,9 @@ const nextDev = async (
}
return startServer(startServerOptions)
}
await handleSessionStop(signal)
// Call handler (e.g. upload telemetry). Don't try to send a signal to
// the child, as it has already exited.
await handleSessionStop(/* signal */ null)
})
})
}
Expand Down

0 comments on commit d7a49d5

Please sign in to comment.