fix: kill orphaned process on Windows before OpenCode restart
killProcessOnPort() was a no-op on win32 (POSIX-only, via lsof/kill), so a restart could leave the old OpenCode process holding the port while a new instance spawned on a different one. That's a plausible contributor to a chronic pattern seen in production logs: repeated "OpenCode process exited, restarting" cycles and hundreds of ECONNRESET/proxy errors over multiple days on Windows. Give killProcessOnPort a real Windows branch: parse `netstat -ano` for PIDs listening on the target port, filter out our own pid, and force-kill each via `taskkill /PID <pid> /F` (no /T -- we don't own that process, so only the listener itself is killed, not any children it may have). waitForPortRelease()'s existing soft-fail-and-warn behavior is left untouched -- it's a deliberate safety net for any platform where the port doesn't free up in time, not just Windows, and the restart already rebinds event-stream readers to the actual resulting port via onOpenCodeRestarted.
This commit is contained in:
@@ -2,11 +2,12 @@ import { EventEmitter } from 'node:events';
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||
|
||||
const spawnMock = vi.fn();
|
||||
const spawnSyncMock = vi.fn();
|
||||
const recordStartupPerformanceMock = vi.fn();
|
||||
|
||||
vi.mock('node:child_process', () => ({
|
||||
spawn: spawnMock,
|
||||
spawnSync: vi.fn(),
|
||||
spawnSync: spawnSyncMock,
|
||||
}));
|
||||
vi.mock('./startup-performance.js', () => ({
|
||||
recordStartupPerformance: recordStartupPerformanceMock,
|
||||
@@ -20,6 +21,7 @@ const originalFetch = globalThis.fetch;
|
||||
|
||||
afterEach(() => {
|
||||
spawnMock.mockReset();
|
||||
spawnSyncMock.mockReset();
|
||||
recordStartupPerformanceMock.mockReset();
|
||||
globalThis.fetch = originalFetch;
|
||||
if (typeof originalOpencodeBinary === 'string') {
|
||||
@@ -613,3 +615,75 @@ describe('OpenCode lifecycle', () => {
|
||||
await server.close();
|
||||
});
|
||||
});
|
||||
|
||||
describe('killProcessOnPort on Windows', () => {
|
||||
const originalPlatform = process.platform;
|
||||
|
||||
afterEach(() => {
|
||||
Object.defineProperty(process, 'platform', { value: originalPlatform });
|
||||
});
|
||||
|
||||
const setPlatform = (platform) => {
|
||||
Object.defineProperty(process, 'platform', { value: platform, configurable: true });
|
||||
};
|
||||
|
||||
const netstatOutput = (port, pid) => [
|
||||
'',
|
||||
'Active Connections',
|
||||
'',
|
||||
' Proto Local Address Foreign Address State PID',
|
||||
` TCP 0.0.0.0:${port} 0.0.0.0:0 LISTENING ${pid}`,
|
||||
'',
|
||||
].join('\r\n');
|
||||
|
||||
it('force-kills the process listening on the target port via taskkill', () => {
|
||||
setPlatform('win32');
|
||||
const orphanPid = 54321;
|
||||
spawnSyncMock.mockImplementation((cmd) => {
|
||||
if (cmd === 'netstat') {
|
||||
return { stdout: netstatOutput(45678, orphanPid) };
|
||||
}
|
||||
return { stdout: '' };
|
||||
});
|
||||
|
||||
const runtime = createRuntime();
|
||||
runtime.killProcessOnPort(45678);
|
||||
|
||||
expect(spawnSyncMock).toHaveBeenCalledWith('netstat', ['-ano'], expect.objectContaining({ windowsHide: true }));
|
||||
expect(spawnSyncMock).toHaveBeenCalledWith(
|
||||
'taskkill',
|
||||
['/PID', String(orphanPid), '/F'],
|
||||
expect.objectContaining({ windowsHide: true })
|
||||
);
|
||||
});
|
||||
|
||||
it('never force-kills its own process id', () => {
|
||||
setPlatform('win32');
|
||||
spawnSyncMock.mockImplementation((cmd) => {
|
||||
if (cmd === 'netstat') {
|
||||
return { stdout: netstatOutput(45678, process.pid) };
|
||||
}
|
||||
return { stdout: '' };
|
||||
});
|
||||
|
||||
const runtime = createRuntime();
|
||||
runtime.killProcessOnPort(45678);
|
||||
|
||||
expect(spawnSyncMock).not.toHaveBeenCalledWith('taskkill', expect.anything(), expect.anything());
|
||||
});
|
||||
|
||||
it('does nothing when no process is listening on the target port', () => {
|
||||
setPlatform('win32');
|
||||
spawnSyncMock.mockImplementation((cmd) => {
|
||||
if (cmd === 'netstat') {
|
||||
return { stdout: netstatOutput(9999, 54321) };
|
||||
}
|
||||
return { stdout: '' };
|
||||
});
|
||||
|
||||
const runtime = createRuntime();
|
||||
runtime.killProcessOnPort(45678);
|
||||
|
||||
expect(spawnSyncMock).not.toHaveBeenCalledWith('taskkill', expect.anything(), expect.anything());
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user