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 }