Skip to content

Commit 7ba7cc6

Browse files
committed
fix to dedupe process stop
1 parent 3932b0e commit 7ba7cc6

File tree

1 file changed

+39
-25
lines changed

1 file changed

+39
-25
lines changed

packages/core/src/shared/utilities/processUtils.ts

Lines changed: 39 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,41 @@ export class ChildProcess {
409409
return this.#childProcess?.exitCode ?? -1
410410
}
411411

412+
/**
413+
* Attempts to kill a process group on Unix systems, falling back to regular kill if it fails
414+
* @param child The child process to kill
415+
* @param signal The signal to send (defaults to SIGTERM)
416+
* @param checkSpawnProps Whether to check for child.spawnargs and child.spawnfile (used in first call only)
417+
* @returns true if process group kill was attempted, false if regular kill was used
418+
*/
419+
public killProcessGroup(
420+
child: proc.ChildProcess,
421+
signal: NodeJS.Signals = 'SIGTERM',
422+
checkSpawnProps: boolean = false
423+
): boolean {
424+
// First call in stop() uses the stricter condition with spawnargs/spawnfile checks
425+
// Second call in the force kill uses the less restrictive condition
426+
const condition = checkSpawnProps
427+
? process.platform !== 'win32' && child.spawnargs && child.spawnfile
428+
: process.platform !== 'win32'
429+
430+
if (condition && child.pid) {
431+
try {
432+
// On Unix systems, negative PID kills the process group
433+
this.#log.debug(`Attempting to kill process group -${child.pid} with ${signal}`)
434+
process.kill(-child.pid, signal)
435+
return true
436+
} catch (err) {
437+
this.#log.debug(`Failed to kill process group with ${signal}: ${err}, falling back to regular kill`)
438+
child.kill(signal)
439+
return false
440+
}
441+
} else {
442+
child.kill(signal)
443+
return false
444+
}
445+
}
446+
412447
/**
413448
* Stops the process.
414449
*
@@ -427,37 +462,16 @@ export class ChildProcess {
427462
const pid = this.pid()
428463
if (!this.stopped) {
429464
// Try to kill the process group if possible (Unix systems only)
430-
if (process.platform !== 'win32' && child.pid && child.spawnargs && child.spawnfile) {
431-
try {
432-
// On Unix systems, negative PID kills the process group
433-
this.#log.debug(`Attempting to kill process group -${child.pid} with ${signal || 'SIGTERM'}`)
434-
process.kill(-child.pid, signal || 'SIGTERM')
435-
} catch (err) {
436-
this.#log.debug(`Failed to kill process group: ${err}, falling back to regular kill`)
437-
child.kill(signal)
438-
}
439-
} else {
440-
child.kill(signal)
441-
}
465+
// First call uses the stricter condition with spawnargs/spawnfile checks
466+
this.killProcessGroup(child, signal || 'SIGTERM', true)
442467

443468
if (force === true) {
444469
waitUntil(async () => this.stopped, { timeout: ChildProcess.stopTimeout, interval: 200, truthy: true })
445470
.then((stopped) => {
446471
if (!stopped) {
447472
// Try to kill the process group with SIGKILL if possible
448-
if (process.platform !== 'win32' && child.pid) {
449-
try {
450-
this.#log.debug(`Attempting to kill process group -${child.pid} with SIGKILL`)
451-
process.kill(-child.pid, 'SIGKILL')
452-
} catch (err) {
453-
this.#log.debug(
454-
`Failed to kill process group with SIGKILL: ${err}, falling back to regular kill`
455-
)
456-
child.kill('SIGKILL')
457-
}
458-
} else {
459-
child.kill('SIGKILL')
460-
}
473+
// Second call uses the less restrictive condition
474+
this.killProcessGroup(child, 'SIGKILL', false)
461475
}
462476
})
463477
.catch((e) => {

0 commit comments

Comments
 (0)