fix: properly cleanup Windows process tree on Obsidian exit
### Fixed - Replaced unreliable taskkill /T with PowerShell Get-CimInstance for child process detection in WindowsProcess.ts - Fixed orphaned node.exe processes when Obsidian closes by killing child processes before parent - Added proper cleanup when shell: true creates cmd.exe -> node.exe process tree ### Added - Static currentProcess field to track active process for cleanup during window close - Static cleanupHandlerRegistered flag to prevent duplicate event handlers - beforeunload event handler for synchronous cleanup when Obsidian window closes - killProcessSync method for immediate process termination without async delays - registerCleanupHandler method to set up window close event listener ### Changed - Updated start method to store process reference and register cleanup handler - Modified stop method to use PowerShell child lookup before killing parent process - Enhanced error handling with try/catch blocks for PowerShell and taskkill operations
This commit is contained in:
committed by
Mateusz Tymek
parent
2c17c9e7f6
commit
2a824b6d19
@@ -2,33 +2,129 @@ import { ChildProcess, spawn, SpawnOptions } from "child_process";
|
|||||||
import { OpenCodeProcess } from "./OpenCodeProcess";
|
import { OpenCodeProcess } from "./OpenCodeProcess";
|
||||||
|
|
||||||
export class WindowsProcess implements OpenCodeProcess {
|
export class WindowsProcess implements OpenCodeProcess {
|
||||||
|
// Static state to track the current process for cleanup
|
||||||
|
private static currentProcess: ChildProcess | null = null;
|
||||||
|
private static cleanupHandlerRegistered = false;
|
||||||
|
|
||||||
start(
|
start(
|
||||||
command: string,
|
command: string,
|
||||||
args: string[],
|
args: string[],
|
||||||
options: SpawnOptions
|
options: SpawnOptions
|
||||||
): ChildProcess {
|
): ChildProcess {
|
||||||
return spawn(command, args, {
|
const process = spawn(command, args, {
|
||||||
...options,
|
...options,
|
||||||
shell: true,
|
shell: true,
|
||||||
windowsHide: true,
|
windowsHide: true,
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Store process for cleanup
|
||||||
|
WindowsProcess.currentProcess = process;
|
||||||
|
WindowsProcess.registerCleanupHandler();
|
||||||
|
|
||||||
|
return process;
|
||||||
}
|
}
|
||||||
|
|
||||||
async stop(process: ChildProcess): Promise<void> {
|
async stop(process: ChildProcess): Promise<void> {
|
||||||
const pid = process.pid;
|
const pid = process.pid;
|
||||||
if (!pid) {
|
if (!pid) {
|
||||||
|
WindowsProcess.currentProcess = null;
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
console.log("[OpenCode] Stopping server process tree, PID:", pid);
|
console.log("[OpenCode] Stopping server process tree, PID:", pid);
|
||||||
|
|
||||||
// Use taskkill with /T flag to kill process tree
|
// Method 1: Find and kill child processes (actual node.exe) using PowerShell
|
||||||
await this.execAsync(`taskkill /T /F /PID ${pid}`);
|
// This is necessary because shell: true spawns cmd.exe -> node.exe, and
|
||||||
|
// killing cmd.exe leaves node.exe orphaned
|
||||||
|
try {
|
||||||
|
const { execSync } = require("child_process");
|
||||||
|
const output = execSync(
|
||||||
|
`powershell -Command "Get-CimInstance Win32_Process -Filter \\"ParentProcessId=${pid}\\" | Select-Object ProcessId"`,
|
||||||
|
{ encoding: "utf8", stdio: ["pipe", "pipe", "ignore"] }
|
||||||
|
);
|
||||||
|
|
||||||
|
const lines = output.split("\n").slice(3); // Skip headers
|
||||||
|
for (const line of lines) {
|
||||||
|
const childPid = line.trim();
|
||||||
|
if (childPid && !isNaN(parseInt(childPid))) {
|
||||||
|
try {
|
||||||
|
execSync(`taskkill /F /PID ${childPid}`, { stdio: "ignore" });
|
||||||
|
} catch {
|
||||||
|
// Child may already be gone
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
// PowerShell lookup failed, continue to other methods
|
||||||
|
}
|
||||||
|
|
||||||
|
// Method 2: Kill the parent process (cmd.exe)
|
||||||
|
try {
|
||||||
|
await this.execAsync(`taskkill /F /PID ${pid}`);
|
||||||
|
} catch {
|
||||||
|
// Parent may already be gone
|
||||||
|
}
|
||||||
|
|
||||||
|
// Clear stored process
|
||||||
|
WindowsProcess.currentProcess = null;
|
||||||
|
|
||||||
// Wait for process to exit
|
// Wait for process to exit
|
||||||
await this.waitForExit(process, 5000);
|
await this.waitForExit(process, 5000);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
private static registerCleanupHandler(): void {
|
||||||
|
if (WindowsProcess.cleanupHandlerRegistered) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Register beforeunload handler for window close cleanup
|
||||||
|
if (typeof window !== "undefined") {
|
||||||
|
window.addEventListener("beforeunload", () => {
|
||||||
|
if (WindowsProcess.currentProcess?.pid) {
|
||||||
|
WindowsProcess.killProcessSync(WindowsProcess.currentProcess.pid);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
WindowsProcess.cleanupHandlerRegistered = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
private static killProcessSync(pid: number): void {
|
||||||
|
try {
|
||||||
|
const { execSync } = require("child_process");
|
||||||
|
|
||||||
|
// Method 1: Kill child processes using PowerShell
|
||||||
|
try {
|
||||||
|
const output = execSync(
|
||||||
|
`powershell -Command "Get-CimInstance Win32_Process -Filter \\"ParentProcessId=${pid}\\" | Select-Object ProcessId"`,
|
||||||
|
{ encoding: "utf8", stdio: ["pipe", "pipe", "ignore"] }
|
||||||
|
);
|
||||||
|
|
||||||
|
const lines = output.split("\n").slice(3);
|
||||||
|
for (const line of lines) {
|
||||||
|
const childPid = line.trim();
|
||||||
|
if (childPid && !isNaN(parseInt(childPid))) {
|
||||||
|
try {
|
||||||
|
execSync(`taskkill /F /PID ${childPid}`, { stdio: "ignore" });
|
||||||
|
} catch {
|
||||||
|
// Child may already be gone
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
// PowerShell lookup failed
|
||||||
|
}
|
||||||
|
|
||||||
|
// Method 2: Kill parent process
|
||||||
|
try {
|
||||||
|
execSync(`taskkill /F /PID ${pid}`, { stdio: "ignore" });
|
||||||
|
} catch {
|
||||||
|
// Parent may already be gone
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
// Process may already be gone
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
async verifyCommand(command: string): Promise<string | null> {
|
async verifyCommand(command: string): Promise<string | null> {
|
||||||
// Use 'where' command to check if executable exists in PATH
|
// Use 'where' command to check if executable exists in PATH
|
||||||
try {
|
try {
|
||||||
|
|||||||
Reference in New Issue
Block a user