From 0a75c4cf76edfa9446ab32126d4a574140ff84b0 Mon Sep 17 00:00:00 2001 From: Juergen Kunz Date: Sun, 31 Aug 2025 07:45:47 +0000 Subject: [PATCH] fix(processmanager): Improve process lifecycle handling and cleanup in daemon, monitors and wrappers --- changelog.md | 8 +++++ ts/00_commitinfo_data.ts | 2 +- ts/daemon/processmanager.ts | 6 +++- ts/daemon/processmonitor.ts | 5 ++++ ts/daemon/processwrapper.ts | 59 ++++++++++++++++++++----------------- ts/daemon/tspm.daemon.ts | 8 +++-- 6 files changed, 56 insertions(+), 32 deletions(-) diff --git a/changelog.md b/changelog.md index c7059a4..4bae3ea 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,13 @@ # Changelog +## 2025-08-31 - 5.6.2 - fix(processmanager) +Improve process lifecycle handling and cleanup in daemon, monitors and wrappers + +- StartAll: when a monitor exists but is not running, restart it instead of skipping — ensures saved processes are reliably brought online. +- ProcessMonitor.stop: cancel any pending restart timers to prevent stray restarts after explicit stop. +- ProcessWrapper: add killProcessTree helper and use it for graceful (SIGTERM) and force (SIGKILL) shutdowns to reliably signal child processes. +- Daemon stopAll: yield briefly after stopping processes and inspect monitors (not only processInfo) to accurately report stopped vs failed processes. + ## 2025-08-31 - 5.6.1 - fix(daemon) Ensure robust process shutdown and improve logs/subscriber diagnostics diff --git a/ts/00_commitinfo_data.ts b/ts/00_commitinfo_data.ts index 9914e82..9ad8a8b 100644 --- a/ts/00_commitinfo_data.ts +++ b/ts/00_commitinfo_data.ts @@ -3,6 +3,6 @@ */ export const commitinfo = { name: '@git.zone/tspm', - version: '5.6.1', + version: '5.6.2', description: 'a no fuzz process manager' } diff --git a/ts/daemon/processmanager.ts b/ts/daemon/processmanager.ts index 3783b67..1a74166 100644 --- a/ts/daemon/processmanager.ts +++ b/ts/daemon/processmanager.ts @@ -499,8 +499,12 @@ export class ProcessManager extends EventEmitter { */ public async startAll(): Promise { for (const [id, config] of this.processConfigs.entries()) { - if (!this.processes.has(id)) { + const monitor = this.processes.get(id); + if (!monitor) { await this.start(config); + } else if (!monitor.isRunning()) { + // If a monitor exists but is not running, restart the process to ensure a clean start + await this.restart(id); } } } diff --git a/ts/daemon/processmonitor.ts b/ts/daemon/processmonitor.ts index 8c40515..82cc2bb 100644 --- a/ts/daemon/processmonitor.ts +++ b/ts/daemon/processmonitor.ts @@ -400,6 +400,11 @@ export class ProcessMonitor extends EventEmitter { if (this.intervalId) { clearInterval(this.intervalId); } + // Cancel any pending restart timer + if (this.restartTimer) { + clearTimeout(this.restartTimer); + this.restartTimer = null; + } if (this.processWrapper) { // Clear pidusage state for current PID before stopping to avoid leaks try { diff --git a/ts/daemon/processwrapper.ts b/ts/daemon/processwrapper.ts index fd0e83b..b439351 100644 --- a/ts/daemon/processwrapper.ts +++ b/ts/daemon/processwrapper.ts @@ -23,6 +23,26 @@ export class ProcessWrapper extends EventEmitter { private runId: string = ''; private stdoutRemainder: string = ''; private stderrRemainder: string = ''; + + // Helper: send a signal to the process and all its children (best-effort) + private async killProcessTree(signal: NodeJS.Signals): Promise { + if (!this.process || !this.process.pid) return; + const rootPid = this.process.pid; + await new Promise((resolve) => { + plugins.psTree(rootPid, (err: any, children: ReadonlyArray<{ PID: string }>) => { + const pids: number[] = [rootPid, ...children.map((c) => Number(c.PID)).filter((n) => Number.isFinite(n))]; + for (const pid of pids) { + try { + // Always signal individual PIDs to avoid accidentally targeting unrelated groups + process.kill(pid, signal); + } catch { + // ignore ESRCH/EPERM + } + } + resolve(); + }); + }); + } constructor(options: IProcessWrapperOptions) { super(); @@ -193,17 +213,13 @@ export class ProcessWrapper extends EventEmitter { // First try SIGTERM for graceful shutdown if (this.process.pid) { try { - this.logger.debug(`Sending SIGTERM to process ${this.process.pid}`); - try { - // Try to signal the whole process group on POSIX to ensure children get the signal too - if (process.platform !== 'win32') { - process.kill(-Math.abs(this.process.pid), 'SIGTERM'); - } else { - process.kill(this.process.pid, 'SIGTERM'); - } - } catch { - // Fallback to direct process kill if group kill fails - process.kill(this.process.pid, 'SIGTERM'); + this.logger.debug(`Sending SIGTERM to process tree rooted at ${this.process.pid}`); + await this.killProcessTree('SIGTERM'); + + // If the process already exited, return immediately + if (typeof this.process.exitCode === 'number') { + this.logger.debug('Process already exited, no need to wait'); + return; } // Wait for exit or escalate @@ -218,26 +234,15 @@ export class ProcessWrapper extends EventEmitter { const onExit = () => cleanup(); this.process!.once('exit', onExit); - const killTimer = setTimeout(() => { + const killTimer = setTimeout(async () => { if (!this.process || !this.process.pid) return cleanup(); this.logger.warn( - `Process ${this.process.pid} did not exit gracefully, force killing...`, - ); - this.addSystemLog( - 'Process did not exit gracefully, force killing...', + `Process ${this.process.pid} did not exit gracefully, force killing tree...`, ); + this.addSystemLog('Process did not exit gracefully, force killing...'); try { - if (process.platform !== 'win32') { - process.kill(-Math.abs(this.process.pid), 'SIGKILL'); - } else { - process.kill(this.process.pid, 'SIGKILL'); - } - } catch (error: any) { - this.logger.debug( - `Failed to send SIGKILL, process probably already exited: ${error?.message || String(error)}`, - ); - } - + await this.killProcessTree('SIGKILL'); + } catch {} // Give a short grace period after SIGKILL setTimeout(() => cleanup(), 500); }, 5000); diff --git a/ts/daemon/tspm.daemon.ts b/ts/daemon/tspm.daemon.ts index d6b4a4d..8e13bbd 100644 --- a/ts/daemon/tspm.daemon.ts +++ b/ts/daemon/tspm.daemon.ts @@ -450,10 +450,12 @@ export class TspmDaemon { await this.tspmInstance.setDesiredStateForAll('stopped'); await this.tspmInstance.stopAll(); + // Yield briefly to allow any pending exit events to settle + await new Promise((r) => setTimeout(r, 50)); - // Get status of all processes - for (const [id, processInfo] of this.tspmInstance.processInfo) { - if (processInfo.status === 'stopped') { + // Determine which monitors are no longer running + for (const [id, monitor] of this.tspmInstance.processes) { + if (!monitor.isRunning()) { stopped.push(id); } else { failed.push({ id, error: 'Failed to stop' });