From 3b83f396fd010c0ec7dc2c331ba7d97779436549 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 21 Aug 2026 15:12:49 -0700 Subject: [PATCH] fix(windows): bound retries after the process table wedges The 3s deadline stops a caller hanging, but the timed-out call leaves its callback in the vendored module's queue -- and that queue drains only when the latched request completes, which in this wedge never happens. Retrying at the caller's poll rate would add a closure per tick forever. A 30s cooldown bounds it to one probe, and a late callback clears the cooldown because it proves the reader recovered. Also pins the deadlock invariant in the patch: the exit thread's lock must close before tsfn.BlockingCall, because that waits on the JS thread and the JS thread can be waiting on the same mutex inside PtyTerminateJob. Correct today by scoping; a comment so a later refactor does not widen it. --- config/patches/node-pty@1.1.0.patch | 17 ++++---- pnpm-lock.yaml | 6 +-- .../windows/windows-process-table.test.ts | 39 +++++++++++++++++++ src/main/windows/windows-process-table.ts | 28 +++++++++++-- 4 files changed, 76 insertions(+), 14 deletions(-) diff --git a/config/patches/node-pty@1.1.0.patch b/config/patches/node-pty@1.1.0.patch index 9e3e0b740f5..a03a0cfa0b2 100644 --- a/config/patches/node-pty@1.1.0.patch +++ b/config/patches/node-pty@1.1.0.patch @@ -430,7 +430,7 @@ index 7b4b9e1f990fbf95b51528bb56dc9717f5b87532..383df0c9c48355547c65e6c9bbba593d } #endif diff --git a/src/win/conpty.cc b/src/win/conpty.cc -index 7b286d3d644c26141df516929703aa6e129df4b2..cad5cbcee5db36f510158e900e21849b298f4de5 100644 +index 7b286d3d644c26141df516929703aa6e129df4b2..9ede3298f311bff35c97c4d62122faed059c3bc9 100644 --- a/src/win/conpty.cc +++ b/src/win/conpty.cc @@ -18,6 +18,7 @@ @@ -472,7 +472,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..cad5cbcee5db36f510158e900e21849b static volatile LONG ptyCounter; static pty_baton* get_pty_baton(int id) { -@@ -102,8 +120,24 @@ void SetupExitCallback(Napi::Env env, Napi::Function cb, pty_baton* baton) { +@@ -102,8 +120,27 @@ void SetupExitCallback(Napi::Env env, Napi::Function cb, pty_baton* baton) { // Get process exit code. GetExitCodeProcess(baton->hShell, (LPDWORD)(&exit_event->exit_code)); // Clean up handles @@ -496,10 +496,13 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..cad5cbcee5db36f510158e900e21849b + assert(removed); + (void)removed; + } ++ // Why the lock ends here: BlockingCall below waits on the JS thread, and the ++ // JS thread can be waiting on ptyJobMutex inside PtyTerminateJob. Holding ++ // the lock across it deadlocks. Do not widen this scope. auto status = tsfn.BlockingCall(exit_event, callback); // In main thread switch (status) { -@@ -416,7 +450,10 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { +@@ -416,7 +453,10 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { nullptr, // lpProcessAttributes nullptr, // lpThreadAttributes false, // bInheritHandles VERY IMPORTANT that this is false @@ -511,7 +514,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..cad5cbcee5db36f510158e900e21849b envArg, // lpEnvironment mutableCwd.get(), // lpCurrentDirectory &siEx.StartupInfo, // lpStartupInfo -@@ -426,6 +463,47 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { +@@ -426,6 +466,47 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { throw errorWithCode(info, "Cannot create process"); } @@ -559,7 +562,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..cad5cbcee5db36f510158e900e21849b HANDLE hLibrary = LoadConptyDll(info, useConptyDll); bool fLoadedDll = hLibrary != nullptr; if (useConptyDll && fLoadedDll) -@@ -440,6 +518,11 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { +@@ -440,6 +521,11 @@ static Napi::Value PtyConnect(const Napi::CallbackInfo& info) { // Update handle handle->hShell = piClient.hProcess; @@ -571,7 +574,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..cad5cbcee5db36f510158e900e21849b // Close the thread handle to avoid resource leak CloseHandle(piClient.hThread); -@@ -567,6 +650,94 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) { +@@ -567,6 +653,94 @@ static Napi::Value PtyKill(const Napi::CallbackInfo& info) { return env.Undefined(); } @@ -666,7 +669,7 @@ index 7b286d3d644c26141df516929703aa6e129df4b2..cad5cbcee5db36f510158e900e21849b /** * Init */ -@@ -577,6 +748,8 @@ Napi::Object init(Napi::Env env, Napi::Object exports) { +@@ -577,6 +751,8 @@ Napi::Object init(Napi::Env env, Napi::Object exports) { exports.Set("resize", Napi::Function::New(env, PtyResize)); exports.Set("clear", Napi::Function::New(env, PtyClear)); exports.Set("kill", Napi::Function::New(env, PtyKill)); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index b75ce03bfdf..00f107fc1c9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -27,7 +27,7 @@ patchedDependencies: hash: 7333b3837f80a7fbd045964db6d76ba4fc118e49134bdbabb00585b6b7b60673 path: config/patches/lint-staged@16.4.0.patch node-pty@1.1.0: - hash: c48841645e1873768dfdfec82cc018f54dec09c42293491aba043650801c09fe + hash: 68eacd868f74f1802d5589ea3b67360d9bdfe2182dee0a94491568e4ea1aabc1 path: config/patches/node-pty@1.1.0.patch importers: @@ -69,7 +69,7 @@ importers: version: 3.3.1 node-pty: specifier: ^1.1.0 - version: 1.1.0(patch_hash=c48841645e1873768dfdfec82cc018f54dec09c42293491aba043650801c09fe) + version: 1.1.0(patch_hash=68eacd868f74f1802d5589ea3b67360d9bdfe2182dee0a94491568e4ea1aabc1) posthog-node: specifier: ^5.33.3 version: 5.33.3 @@ -12046,7 +12046,7 @@ snapshots: node-int64@0.4.0: {} - node-pty@1.1.0(patch_hash=c48841645e1873768dfdfec82cc018f54dec09c42293491aba043650801c09fe): + node-pty@1.1.0(patch_hash=68eacd868f74f1802d5589ea3b67360d9bdfe2182dee0a94491568e4ea1aabc1): dependencies: node-addon-api: 7.1.1 diff --git a/src/main/windows/windows-process-table.test.ts b/src/main/windows/windows-process-table.test.ts index f2902bb4d8f..c730ffda016 100644 --- a/src/main/windows/windows-process-table.test.ts +++ b/src/main/windows/windows-process-table.test.ts @@ -106,3 +106,42 @@ describe('windows process table', () => { expect(isWindowsProcessTableAvailable()).toBe(false) }) }) + +describe('wedge cooldown', () => { + let platform: PropertyDescriptor | undefined + + beforeEach(() => { + platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + }) + + afterEach(() => { + vi.useRealTimers() + __setWindowsProcessTreeLoaderForTests() + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + }) + + it('stops calling the reader after a timeout instead of queueing a callback per tick', async () => { + // The vendored reader latches a global while a request is in flight and + // drains its queue only when that request completes. In the wedge this + // guards against it never does, so every retry would add a closure that is + // never called. One probe per cooldown bounds that. + vi.useFakeTimers() + const getAllProcesses = vi.fn(() => {}) + __setWindowsProcessTreeLoaderForTests(() => ({ + ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2 }, + getAllProcesses + })) + + const first = readWindowsProcessTableFresh() + const firstAssertion = expect(first).rejects.toThrow(/timed out/) + await vi.advanceTimersByTimeAsync(3_000) + await firstAssertion + expect(getAllProcesses).toHaveBeenCalledTimes(1) + + await expect(readWindowsProcessTableFresh()).rejects.toThrow(/cooling down/) + expect(getAllProcesses).toHaveBeenCalledTimes(1) + }) +}) diff --git a/src/main/windows/windows-process-table.ts b/src/main/windows/windows-process-table.ts index 1b437a0316a..d64dd7ce100 100644 --- a/src/main/windows/windows-process-table.ts +++ b/src/main/windows/windows-process-table.ts @@ -96,6 +96,19 @@ function loadWindowsProcessTree(): WindowsProcessTreeModule | null { */ const WINDOWS_PROCESS_QUERY_TIMEOUT_MS = 3_000 +/** + * How long to stop calling the reader after it misses its deadline. + * + * Why a cooldown and not just the deadline: a timed-out call leaves its + * callback in the vendored module's queue, and that queue only drains when the + * latched request finally completes -- which, in the wedge this guards against, + * never happens. Retrying at the caller's poll rate would then add a closure + * per tick forever. One probe per cooldown bounds it. + */ +const WINDOWS_PROCESS_QUERY_COOLDOWN_MS = 30_000 + +let wedgedUntilMs = 0 + function readNativeRows(fields: WindowsProcessFields): Promise { const native = moduleLoader() if (!native) { @@ -108,18 +121,23 @@ function readNativeRows(fields: WindowsProcessFields): Promise { try { - const deadline = setTimeout( - () => reject(new Error('windows process table timed out')), - WINDOWS_PROCESS_QUERY_TIMEOUT_MS - ) + const deadline = setTimeout(() => { + wedgedUntilMs = Date.now() + WINDOWS_PROCESS_QUERY_COOLDOWN_MS + reject(new Error('windows process table timed out')) + }, WINDOWS_PROCESS_QUERY_TIMEOUT_MS) deadline.unref?.() native.getAllProcesses((processes) => { clearTimeout(deadline) + // A late callback proves the reader recovered, so stop refusing. + wedgedUntilMs = 0 if (!processes) { reject(new Error('windows process table returned no snapshot')) return @@ -205,6 +223,7 @@ export function __setWindowsProcessTreeLoaderForTests( ): void { moduleLoader = loader ?? loadWindowsProcessTree cachedModule = undefined + wedgedUntilMs = 0 snapshotReader.reset() } @@ -212,4 +231,5 @@ export function __setWindowsProcessTreeLoaderForTests( export function resetWindowsProcessTableForTests(): void { snapshotReader.reset() cachedModule = undefined + wedgedUntilMs = 0 }