Merge pull request #2844 from sergiofspedro/fix/windows-port-release
fix: kill orphaned process on Windows before OpenCode restart
This commit is contained in:
@@ -1162,8 +1162,8 @@ const openCodeLifecycleRuntime = createOpenCodeLifecycleRuntime({
|
|||||||
return [...new Set(directories)];
|
return [...new Set(directories)];
|
||||||
},
|
},
|
||||||
// A managed restart can move OpenCode to a NEW port (the old one may stay
|
// A managed restart can move OpenCode to a NEW port (the old one may stay
|
||||||
// occupied by an orphaned process, e.g. killProcessOnPort is a no-op on
|
// occupied if killProcessOnPort/waitForPortRelease didn't free it in time,
|
||||||
// Windows). Rebind the message-stream upstream readers to the current port
|
// on any platform). Rebind the message-stream upstream readers to the current port
|
||||||
// so the UI keeps receiving events instead of staying pinned to the old
|
// so the UI keeps receiving events instead of staying pinned to the old
|
||||||
// process (#2638). The runtime is created later by the startup pipeline;
|
// process (#2638). The runtime is created later by the startup pipeline;
|
||||||
// by the time any restart runs, it is assigned.
|
// by the time any restart runs, it is assigned.
|
||||||
|
|||||||
@@ -112,8 +112,45 @@ export const createOpenCodeLifecycleRuntime = (deps) => {
|
|||||||
now = Date.now,
|
now = Date.now,
|
||||||
} = deps;
|
} = deps;
|
||||||
|
|
||||||
|
const killProcessOnPortWin32 = (port) => {
|
||||||
|
try {
|
||||||
|
// Get-NetTCPConnection reads the same locale-independent WinNT API
|
||||||
|
// netstat's display layer translates (e.g. "LISTENING" renders as
|
||||||
|
// "ABHÖREN"/"ÉCOUTE"/"ESCUTANDO" on non-English Windows), so this
|
||||||
|
// works regardless of the OS display language.
|
||||||
|
const result = spawnSync(
|
||||||
|
'powershell',
|
||||||
|
[
|
||||||
|
'-NoProfile',
|
||||||
|
'-NonInteractive',
|
||||||
|
'-Command',
|
||||||
|
`Get-NetTCPConnection -State Listen -LocalPort ${Number.parseInt(port, 10)} -ErrorAction SilentlyContinue | Select-Object -ExpandProperty OwningProcess`,
|
||||||
|
],
|
||||||
|
{ encoding: 'utf8', timeout: 5000, windowsHide: true }
|
||||||
|
);
|
||||||
|
const output = result.stdout || '';
|
||||||
|
const myPid = process.pid;
|
||||||
|
const pids = new Set();
|
||||||
|
for (const line of output.split(/\r?\n/)) {
|
||||||
|
const pid = Number.parseInt(line.trim(), 10);
|
||||||
|
if (pid && pid !== myPid) pids.add(pid);
|
||||||
|
}
|
||||||
|
for (const pid of pids) {
|
||||||
|
try {
|
||||||
|
spawnSync('taskkill', ['/PID', String(pid), '/F'], { stdio: 'ignore', timeout: 3000, windowsHide: true });
|
||||||
|
} catch {
|
||||||
|
}
|
||||||
|
}
|
||||||
|
} catch {
|
||||||
|
}
|
||||||
|
};
|
||||||
|
|
||||||
const killProcessOnPort = (port) => {
|
const killProcessOnPort = (port) => {
|
||||||
if (!port || process.platform === 'win32') return;
|
if (!port) return;
|
||||||
|
if (process.platform === 'win32') {
|
||||||
|
killProcessOnPortWin32(port);
|
||||||
|
return;
|
||||||
|
}
|
||||||
try {
|
try {
|
||||||
const result = spawnSync('lsof', ['-ti', `:${port}`], { encoding: 'utf8', timeout: 5000, windowsHide: true });
|
const result = spawnSync('lsof', ['-ti', `:${port}`], { encoding: 'utf8', timeout: 5000, windowsHide: true });
|
||||||
const output = result.stdout || '';
|
const output = result.stdout || '';
|
||||||
@@ -860,10 +897,11 @@ export const createOpenCodeLifecycleRuntime = (deps) => {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// The restart may have landed on a NEW port (the old one can remain
|
// The restart may have landed on a NEW port (the old one can remain
|
||||||
// occupied by an orphaned process, e.g. Windows killProcessOnPort is a
|
// occupied if killProcessOnPort/waitForPortRelease didn't free it in
|
||||||
// no-op). Upstream event readers pinned to the old process would keep
|
// time, on any platform). Upstream event readers pinned to the old
|
||||||
// the UI silent forever, so rebind them to the current port. Best
|
// process would keep the UI silent forever, so rebind them to the
|
||||||
// effort: a failure here must not fail the restart itself.
|
// current port. Best effort: a failure here must not fail the restart
|
||||||
|
// itself.
|
||||||
try {
|
try {
|
||||||
onOpenCodeRestarted?.();
|
onOpenCodeRestarted?.();
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
|
|||||||
@@ -2,11 +2,12 @@ import { EventEmitter } from 'node:events';
|
|||||||
import { afterEach, describe, expect, it, vi } from 'vitest';
|
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||||
|
|
||||||
const spawnMock = vi.fn();
|
const spawnMock = vi.fn();
|
||||||
|
const spawnSyncMock = vi.fn();
|
||||||
const recordStartupPerformanceMock = vi.fn();
|
const recordStartupPerformanceMock = vi.fn();
|
||||||
|
|
||||||
vi.mock('node:child_process', () => ({
|
vi.mock('node:child_process', () => ({
|
||||||
spawn: spawnMock,
|
spawn: spawnMock,
|
||||||
spawnSync: vi.fn(),
|
spawnSync: spawnSyncMock,
|
||||||
}));
|
}));
|
||||||
vi.mock('./startup-performance.js', () => ({
|
vi.mock('./startup-performance.js', () => ({
|
||||||
recordStartupPerformance: recordStartupPerformanceMock,
|
recordStartupPerformance: recordStartupPerformanceMock,
|
||||||
@@ -20,6 +21,7 @@ const originalFetch = globalThis.fetch;
|
|||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
spawnMock.mockReset();
|
spawnMock.mockReset();
|
||||||
|
spawnSyncMock.mockReset();
|
||||||
recordStartupPerformanceMock.mockReset();
|
recordStartupPerformanceMock.mockReset();
|
||||||
globalThis.fetch = originalFetch;
|
globalThis.fetch = originalFetch;
|
||||||
if (typeof originalOpencodeBinary === 'string') {
|
if (typeof originalOpencodeBinary === 'string') {
|
||||||
@@ -841,3 +843,70 @@ describe('OpenCode lifecycle', () => {
|
|||||||
await server.close();
|
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 });
|
||||||
|
};
|
||||||
|
|
||||||
|
it('force-kills the process listening on the target port via taskkill', () => {
|
||||||
|
setPlatform('win32');
|
||||||
|
const orphanPid = 54321;
|
||||||
|
spawnSyncMock.mockImplementation((cmd) => {
|
||||||
|
if (cmd === 'powershell') {
|
||||||
|
return { stdout: `${orphanPid}\r\n` };
|
||||||
|
}
|
||||||
|
return { stdout: '' };
|
||||||
|
});
|
||||||
|
|
||||||
|
const runtime = createRuntime();
|
||||||
|
runtime.killProcessOnPort(45678);
|
||||||
|
|
||||||
|
expect(spawnSyncMock).toHaveBeenCalledWith(
|
||||||
|
'powershell',
|
||||||
|
expect.arrayContaining([expect.stringContaining('-LocalPort 45678')]),
|
||||||
|
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 === 'powershell') {
|
||||||
|
return { stdout: `${process.pid}\r\n` };
|
||||||
|
}
|
||||||
|
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 === 'powershell') {
|
||||||
|
return { stdout: '' };
|
||||||
|
}
|
||||||
|
return { stdout: '' };
|
||||||
|
});
|
||||||
|
|
||||||
|
const runtime = createRuntime();
|
||||||
|
runtime.killProcessOnPort(45678);
|
||||||
|
|
||||||
|
expect(spawnSyncMock).not.toHaveBeenCalledWith('taskkill', expect.anything(), expect.anything());
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user