From c93abca5637204bcdd99193c9ccc1f6770e347f5 Mon Sep 17 00:00:00 2001 From: selarkin Date: Wed, 23 Sep 2026 19:54:15 -0700 Subject: [PATCH 1/2] [rush-daemon] Read Linux process group state from procfs instead of ps Fixes #6072 Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...process-group-checks_2026-09-24-03-00.json | 10 ++ .../rush-daemon/src/LinuxProcessGroupExit.ts | 73 +++++++++++- .../src/test/LinuxProcessGroupExit.test.ts | 109 ++++++++++++++++-- .../test/NativeMutationCleanupFailure.test.ts | 6 +- 4 files changed, 183 insertions(+), 15 deletions(-) create mode 100644 common/changes/@rushstack/rush-daemon/fix-rushd-procfs-process-group-checks_2026-09-24-03-00.json diff --git a/common/changes/@rushstack/rush-daemon/fix-rushd-procfs-process-group-checks_2026-09-24-03-00.json b/common/changes/@rushstack/rush-daemon/fix-rushd-procfs-process-group-checks_2026-09-24-03-00.json new file mode 100644 index 0000000000..11fbf2beaf --- /dev/null +++ b/common/changes/@rushstack/rush-daemon/fix-rushd-procfs-process-group-checks_2026-09-24-03-00.json @@ -0,0 +1,10 @@ +{ + "changes": [ + { + "packageName": "@rushstack/rush-daemon", + "comment": "On Linux, verify that a rushx or global command's process group has exited by reading /proc instead of running `ps --sid`, so cleanup no longer fails (and retires the workspace session) on images without procps `ps` or with busybox `ps`.", + "type": "patch" + } + ], + "packageName": "@rushstack/rush-daemon" +} diff --git a/libraries/rush-daemon/src/LinuxProcessGroupExit.ts b/libraries/rush-daemon/src/LinuxProcessGroupExit.ts index 5aa60f1c32..b306c3e20c 100644 --- a/libraries/rush-daemon/src/LinuxProcessGroupExit.ts +++ b/libraries/rush-daemon/src/LinuxProcessGroupExit.ts @@ -2,16 +2,39 @@ // See LICENSE in the project root for license information. import { execFile } from 'node:child_process'; +import * as fs from 'node:fs'; import { setTimeout as delayAsync } from 'node:timers/promises'; +const PROC_ROOT: string = '/proc'; +const PID_ENTRY_REGEXP: RegExp = /^\d+$/; +// Indices into the fields that follow "(comm) " in /proc//stat. +const PROC_STAT_STATE_INDEX: number = 0; +const PROC_STAT_SESSION_INDEX: number = 3; const DEFAULT_EXIT_TIMEOUT_MS: number = 5_000; const EXIT_POLL_INTERVAL_MS: number = 10; const MAX_TIMEOUT_MS: number = 0x7fffffff; -/** Waits for a captured detached Linux group/session to disappear or contain only zombies. */ +/** Reads Linux procfs. Injectable so tests can simulate process tables or a missing procfs. */ +export interface ILinuxProcfsReader { + /** Lists the entries of the procfs root; rejects when procfs is unavailable. */ + readonly listEntriesAsync: () => Promise; + /** Reads `/proc//stat`; rejects when the process has exited. */ + readonly readStatAsync: (pid: string) => Promise; +} + +export const NODE_PROCFS_READER: ILinuxProcfsReader = { + listEntriesAsync: () => fs.promises.readdir(PROC_ROOT), + readStatAsync: (pid: string) => fs.promises.readFile(`${PROC_ROOT}/${pid}/stat`, 'utf8') +}; + +/** + * Waits for a captured detached Linux group/session to disappear or contain only zombies. + * Member states come from procfs; `ps --sid` is used only when procfs is unavailable. + */ export async function waitForLinuxProcessGroupExitAsync( groupId: number, - timeoutMs: number = DEFAULT_EXIT_TIMEOUT_MS + timeoutMs: number = DEFAULT_EXIT_TIMEOUT_MS, + procfs: ILinuxProcfsReader = NODE_PROCFS_READER ): Promise { if (!Number.isSafeInteger(groupId) || groupId <= 0 || groupId === process.pid) { throw new RangeError('Expected an owned child process group ID.'); @@ -25,7 +48,7 @@ export async function waitForLinuxProcessGroupExitAsync( if (remaining <= 0) { throw new Error(`Owned Linux process group ${groupId} did not exit within ${timeoutMs}ms.`); } - const states: string[] = await readSessionStatesAsync(groupId, remaining); + const states: string[] = await readSessionStatesAsync(groupId, remaining, procfs); if (states.length > 0 && states.every((state) => state.startsWith('Z'))) return; await delayAsync(Math.min(EXIT_POLL_INTERVAL_MS, remaining)); } @@ -44,7 +67,49 @@ function processGroupExists(groupId: number): boolean { } } -function readSessionStatesAsync(groupId: number, timeoutMs: number): Promise { +async function readSessionStatesAsync( + groupId: number, + timeoutMs: number, + procfs: ILinuxProcfsReader +): Promise { + const states: string[] | undefined = await tryReadSessionStatesFromProcAsync(groupId, procfs); + return states ?? (await readSessionStatesFromPsAsync(groupId, timeoutMs)); +} + +/** + * Reads member states from procfs, which every Linux system has; `ps` is missing from slim/distroless images + * and busybox `ps` does not support `--sid`. Returns `undefined` only when procfs itself is unavailable. + */ +async function tryReadSessionStatesFromProcAsync( + groupId: number, + procfs: ILinuxProcfsReader +): Promise { + let entries: string[]; + try { + entries = await procfs.listEntriesAsync(); + } catch { + return undefined; + } + const states: string[] = []; + await Promise.all( + entries.map(async (entry: string) => { + if (!PID_ENTRY_REGEXP.test(entry)) return; + let stat: string; + try { + stat = await procfs.readStatAsync(entry); + } catch { + // The process exited between readdir and read. + return; + } + // Format: "pid (comm) state ppid pgrp session ..."; comm may contain spaces and parentheses. + const fields: string[] = stat.slice(stat.lastIndexOf(')') + 2).split(' '); + if (Number(fields[PROC_STAT_SESSION_INDEX]) === groupId) states.push(fields[PROC_STAT_STATE_INDEX]); + }) + ); + return states; +} + +function readSessionStatesFromPsAsync(groupId: number, timeoutMs: number): Promise { return new Promise((resolve, reject) => { // detached=true creates a new process group and session with the child's PID. execFile( diff --git a/libraries/rush-daemon/src/test/LinuxProcessGroupExit.test.ts b/libraries/rush-daemon/src/test/LinuxProcessGroupExit.test.ts index 5d4af13f54..bf009fde30 100644 --- a/libraries/rush-daemon/src/test/LinuxProcessGroupExit.test.ts +++ b/libraries/rush-daemon/src/test/LinuxProcessGroupExit.test.ts @@ -8,13 +8,17 @@ jest.mock('node:child_process', () => ({ import * as childProcess from 'node:child_process'; -import { waitForLinuxProcessGroupExitAsync } from '../LinuxProcessGroupExit'; +import { type ILinuxProcfsReader, waitForLinuxProcessGroupExitAsync } from '../LinuxProcessGroupExit'; const GROUP_ID: number = 123_456_789; const execFileMock = jest.mocked(childProcess.execFile); const gone = (): never => { throw Object.assign(new Error('No such process group'), { code: 'ESRCH' }); }; +const NO_PROCFS: ILinuxProcfsReader = { + listEntriesAsync: () => Promise.reject(Object.assign(new Error('No procfs'), { code: 'ENOENT' })), + readStatAsync: () => Promise.reject(new Error('Unexpected procfs read')) +}; function reportPs( args: Parameters, @@ -28,7 +32,10 @@ function reportPs( return new childProcess.ChildProcess(); } -describe('Linux subprocess group completion', () => { +describe('Linux subprocess group completion (ps fallback without procfs)', () => { + const waitAsync = (groupId: number, timeoutMs?: number): Promise => + waitForLinuxProcessGroupExitAsync(groupId, timeoutMs, NO_PROCFS); + beforeEach(() => { execFileMock.mockReset(); jest.spyOn(process, 'kill').mockReturnValue(true); @@ -40,14 +47,14 @@ describe('Linux subprocess group completion', () => { it('avoids process inspection when the captured group has disappeared', async () => { jest.mocked(process.kill).mockImplementation(gone); - await waitForLinuxProcessGroupExitAsync(GROUP_ID); + await waitAsync(GROUP_ID); expect(execFileMock).not.toHaveBeenCalled(); }); it('waits for live members rather than treating signal delivery as completion', async () => { let queries: number = 0; execFileMock.mockImplementation((...args) => reportPs(args, ++queries === 1 ? 'R\nZ\n' : 'Z\nZ\n')); - await waitForLinuxProcessGroupExitAsync(GROUP_ID); + await waitAsync(GROUP_ID); expect(queries).toBe(2); expect( jest.mocked(process.kill).mock.calls.every(([pid, signal]) => pid === -GROUP_ID && signal === 0) @@ -59,19 +66,19 @@ describe('Linux subprocess group completion', () => { execFileMock.mockImplementation((...args) => reportPs(args, '', Object.assign(new Error('No matching processes'), { code: 1 })) ); - await waitForLinuxProcessGroupExitAsync(GROUP_ID); + await waitAsync(GROUP_ID); expect(process.kill).toHaveBeenCalledTimes(2); }); it('surfaces inspection failures instead of completing cleanup', async () => { const failure = Object.assign(new Error('Cannot execute ps'), { code: 'ENOENT' }); execFileMock.mockImplementation((...args) => reportPs(args, '', failure)); - await expect(waitForLinuxProcessGroupExitAsync(GROUP_ID)).rejects.toBe(failure); + await expect(waitAsync(GROUP_ID)).rejects.toBe(failure); }); it('rejects inspection diagnostics rather than trusting incomplete process state', async () => { execFileMock.mockImplementation((...args) => reportPs(args, 'Z\n', undefined, 'inspection warning')); - await expect(waitForLinuxProcessGroupExitAsync(GROUP_ID)).rejects.toThrow('inspection warning'); + await expect(waitAsync(GROUP_ID)).rejects.toThrow('inspection warning'); }); it('does not mistake permission denial for a released group', async () => { @@ -79,16 +86,98 @@ describe('Linux subprocess group completion', () => { jest.mocked(process.kill).mockImplementation(() => { throw failure; }); - await expect(waitForLinuxProcessGroupExitAsync(GROUP_ID)).rejects.toBe(failure); + await expect(waitAsync(GROUP_ID)).rejects.toBe(failure); }); it('bounds the wait for members that remain live', async () => { execFileMock.mockImplementation((...args) => reportPs(args, 'S\n')); - await expect(waitForLinuxProcessGroupExitAsync(GROUP_ID, 25)).rejects.toThrow('did not exit within 25ms'); + await expect(waitAsync(GROUP_ID, 25)).rejects.toThrow('did not exit within 25ms'); }); it.each([0, -1, 1.5, process.pid])('rejects invalid or unowned group ID %s', async (pid) => { - await expect(waitForLinuxProcessGroupExitAsync(pid)).rejects.toThrow('owned child process group'); + await expect(waitAsync(pid)).rejects.toThrow('owned child process group'); expect(process.kill).not.toHaveBeenCalled(); }); }); + +describe('Linux subprocess group completion (procfs)', () => { + const procStat = (pid: number, state: string, session: number): string => + `${pid} (sh -c (x) y) ${state} 1 ${session} ${session} 0 -1 4194560`; + let table: Map; + const fakeProcfs: ILinuxProcfsReader = { + listEntriesAsync: async () => ['self', 'sys', ...table.keys()], + readStatAsync: async (pid: string) => { + const stat: string | undefined = table.get(pid); + if (stat === undefined) throw Object.assign(new Error('gone'), { code: 'ENOENT' }); + return stat; + } + }; + const waitAsync = (timeoutMs?: number): Promise => + waitForLinuxProcessGroupExitAsync(GROUP_ID, timeoutMs, fakeProcfs); + + beforeEach(() => { + execFileMock.mockReset(); + jest.spyOn(process, 'kill').mockReturnValue(true); + table = new Map(); + }); + afterEach(() => { + jest.restoreAllMocks(); + }); + + it('completes from procfs without executing ps when only zombies remain', async () => { + table = new Map([ + ['10', procStat(10, 'Z', GROUP_ID)], + ['11', procStat(11, 'S', 42)] + ]); + await waitAsync(); + expect(execFileMock).not.toHaveBeenCalled(); + }); + + it('ignores members of other sessions whose command names look like session fields', async () => { + table = new Map([ + ['10', procStat(10, 'Z', GROUP_ID)], + ['11', `11 (x) S 1 ${GROUP_ID} ${GROUP_ID}) S 1 42 42 0 -1 4194560`] + ]); + await waitAsync(); + expect(execFileMock).not.toHaveBeenCalled(); + }); + + it('ignores members that exit between listing and reading', async () => { + const reads: string[] = []; + const racingProcfs: ILinuxProcfsReader = { + listEntriesAsync: async () => ['10', '11'], + readStatAsync: async (pid: string) => { + reads.push(pid); + if (pid === '11') throw Object.assign(new Error('gone'), { code: 'ENOENT' }); + return procStat(10, 'Z', GROUP_ID); + } + }; + await waitForLinuxProcessGroupExitAsync(GROUP_ID, undefined, racingProcfs); + expect(reads.sort()).toEqual(['10', '11']); + expect(execFileMock).not.toHaveBeenCalled(); + }); + + it('waits for live session members found in procfs', async () => { + table = new Map([['10', procStat(10, 'R', GROUP_ID)]]); + setTimeout(() => { + table = new Map([['10', procStat(10, 'Z', GROUP_ID)]]); + }, 30); + await waitAsync(); + expect(execFileMock).not.toHaveBeenCalled(); + }); + + it('bounds the wait for live procfs members', async () => { + table = new Map([['10', procStat(10, 'S', GROUP_ID)]]); + await expect(waitAsync(25)).rejects.toThrow('did not exit within 25ms'); + expect(execFileMock).not.toHaveBeenCalled(); + }); + + it('does not depend on ps being installed while procfs is readable', async () => { + execFileMock.mockImplementation((...args) => + reportPs(args, '', Object.assign(new Error('spawn ps ENOENT'), { code: 'ENOENT' })) + ); + table = new Map([['10', procStat(10, 'Z', GROUP_ID)]]); + await waitAsync(); + expect(execFileMock).not.toHaveBeenCalled(); + }); +}); diff --git a/libraries/rush-daemon/src/test/NativeMutationCleanupFailure.test.ts b/libraries/rush-daemon/src/test/NativeMutationCleanupFailure.test.ts index 873a07b5f9..c64a429c1b 100644 --- a/libraries/rush-daemon/src/test/NativeMutationCleanupFailure.test.ts +++ b/libraries/rush-daemon/src/test/NativeMutationCleanupFailure.test.ts @@ -68,7 +68,11 @@ jest.setTimeout(30_000); .spyOn(linuxProcessGroupExit, 'waitForLinuxProcessGroupExitAsync') .mockImplementation(async (pid) => { workerPid = pid; - await originalWait(pid, 25); + // Hide procfs so the injected `ps` inspection outcome is what the join observes. + await originalWait(pid, 25, { + listEntriesAsync: () => Promise.reject(new Error('procfs hidden by test')), + readStatAsync: () => Promise.reject(new Error('procfs hidden by test')) + }); }); jest .spyOn(process, 'kill') From 5bac397f7a42bced5d40b9e7d562b05fd9ac7909 Mon Sep 17 00:00:00 2001 From: selarkin Date: Thu, 24 Sep 2026 11:04:23 -0700 Subject: [PATCH 2/2] [rush-daemon] Bound procfs scans and never trust a scan with unreadable entries Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../rush-daemon/src/LinuxProcessGroupExit.ts | 67 +++++++++++++---- .../src/test/LinuxProcessGroupExit.test.ts | 73 +++++++++++++++++++ 2 files changed, 124 insertions(+), 16 deletions(-) diff --git a/libraries/rush-daemon/src/LinuxProcessGroupExit.ts b/libraries/rush-daemon/src/LinuxProcessGroupExit.ts index b306c3e20c..67b2181360 100644 --- a/libraries/rush-daemon/src/LinuxProcessGroupExit.ts +++ b/libraries/rush-daemon/src/LinuxProcessGroupExit.ts @@ -10,6 +10,9 @@ const PID_ENTRY_REGEXP: RegExp = /^\d+$/; // Indices into the fields that follow "(comm) " in /proc//stat. const PROC_STAT_STATE_INDEX: number = 0; const PROC_STAT_SESSION_INDEX: number = 3; +const PROC_STAT_READ_CONCURRENCY: number = 32; +// Sentinel (never a real one-letter state) for a procfs entry that exists but cannot be read. +const UNREADABLE_STATE: string = ''; const DEFAULT_EXIT_TIMEOUT_MS: number = 5_000; const EXIT_POLL_INTERVAL_MS: number = 10; const MAX_TIMEOUT_MS: number = 0x7fffffff; @@ -78,7 +81,8 @@ async function readSessionStatesAsync( /** * Reads member states from procfs, which every Linux system has; `ps` is missing from slim/distroless images - * and busybox `ps` does not support `--sid`. Returns `undefined` only when procfs itself is unavailable. + * and busybox `ps` does not support `--sid`. Returns `undefined` when procfs is unavailable or an entry cannot + * be read, so the caller falls back to `ps`. */ async function tryReadSessionStatesFromProcAsync( groupId: number, @@ -90,25 +94,56 @@ async function tryReadSessionStatesFromProcAsync( } catch { return undefined; } + const pids: string[] = entries.filter((entry: string) => PID_ENTRY_REGEXP.test(entry)); const states: string[] = []; - await Promise.all( - entries.map(async (entry: string) => { - if (!PID_ENTRY_REGEXP.test(entry)) return; - let stat: string; - try { - stat = await procfs.readStatAsync(entry); - } catch { - // The process exited between readdir and read. - return; - } - // Format: "pid (comm) state ppid pgrp session ..."; comm may contain spaces and parentheses. - const fields: string[] = stat.slice(stat.lastIndexOf(')') + 2).split(' '); - if (Number(fields[PROC_STAT_SESSION_INDEX]) === groupId) states.push(fields[PROC_STAT_STATE_INDEX]); - }) - ); + // Bound concurrent reads so a large process table cannot flood the shared libuv thread pool. + for (let start: number = 0; start < pids.length; start += PROC_STAT_READ_CONCURRENCY) { + const batch: (string | undefined)[] = await Promise.all( + pids + .slice(start, start + PROC_STAT_READ_CONCURRENCY) + .map((pid: string) => readSessionMemberStateAsync(pid, groupId, procfs)) + ); + for (const state of batch) { + if (state === undefined) continue; + // An unreadable entry could hide a live member, so procfs cannot prove the group exited. + if (state === UNREADABLE_STATE) return undefined; + // One live member already means "not exited"; only a zombies-only result needs a full scan. + if (!state.startsWith('Z')) return [state]; + states.push(state); + } + } return states; } +/** + * Returns the member's state, `undefined` for a vanished PID or another session, or `UNREADABLE_STATE` + * when the entry cannot be read (for example `hidepid` or `EIO`) and so might hide a live member. + */ +async function readSessionMemberStateAsync( + pid: string, + groupId: number, + procfs: ILinuxProcfsReader +): Promise { + let stat: string; + try { + stat = await procfs.readStatAsync(pid); + } catch (error) { + return isProcessGoneError(error) ? undefined : UNREADABLE_STATE; + } + // Format: "pid (comm) state ppid pgrp session ..."; comm may contain spaces and parentheses. + const fields: string[] = stat.slice(stat.lastIndexOf(')') + 2).split(' '); + return Number(fields[PROC_STAT_SESSION_INDEX]) === groupId ? fields[PROC_STAT_STATE_INDEX] : undefined; +} + +function isProcessGoneError(error: unknown): boolean { + return ( + typeof error === 'object' && + error !== null && + 'code' in error && + (error.code === 'ENOENT' || error.code === 'ESRCH') + ); +} + function readSessionStatesFromPsAsync(groupId: number, timeoutMs: number): Promise { return new Promise((resolve, reject) => { // detached=true creates a new process group and session with the child's PID. diff --git a/libraries/rush-daemon/src/test/LinuxProcessGroupExit.test.ts b/libraries/rush-daemon/src/test/LinuxProcessGroupExit.test.ts index bf009fde30..237854b90d 100644 --- a/libraries/rush-daemon/src/test/LinuxProcessGroupExit.test.ts +++ b/libraries/rush-daemon/src/test/LinuxProcessGroupExit.test.ts @@ -172,6 +172,79 @@ describe('Linux subprocess group completion (procfs)', () => { expect(execFileMock).not.toHaveBeenCalled(); }); + it.each(['EACCES', 'EIO'])( + 'does not trust a zombie when another entry is unreadable (%s); falls back to ps', + async (code) => { + table = new Map([['10', procStat(10, 'Z', GROUP_ID)]]); + const unreadableProcfs: ILinuxProcfsReader = { + listEntriesAsync: async () => ['10', '11'], + readStatAsync: async (pid: string) => { + if (pid === '11') throw Object.assign(new Error('unreadable'), { code }); + return fakeProcfs.readStatAsync(pid); + } + }; + let queries: number = 0; + execFileMock.mockImplementation((...args) => reportPs(args, ++queries === 1 ? 'S\nZ\n' : 'Z\nZ\n')); + await waitForLinuxProcessGroupExitAsync(GROUP_ID, undefined, unreadableProcfs); + expect(queries).toBe(2); + } + ); + + it('surfaces the ps failure when an unreadable entry forces the fallback and ps is missing', async () => { + const unreadableProcfs: ILinuxProcfsReader = { + listEntriesAsync: async () => ['10', '11'], + readStatAsync: async (pid: string) => { + if (pid === '11') throw Object.assign(new Error('unreadable'), { code: 'EACCES' }); + return procStat(10, 'Z', GROUP_ID); + } + }; + const failure = Object.assign(new Error('spawn ps ENOENT'), { code: 'ENOENT' }); + execFileMock.mockImplementation((...args) => reportPs(args, '', failure)); + await expect(waitForLinuxProcessGroupExitAsync(GROUP_ID, undefined, unreadableProcfs)).rejects.toBe( + failure + ); + }); + + it('treats ESRCH from a stat read as a vanished process', async () => { + const racingProcfs: ILinuxProcfsReader = { + listEntriesAsync: async () => ['10', '11'], + readStatAsync: async (pid: string) => { + if (pid === '11') throw Object.assign(new Error('gone'), { code: 'ESRCH' }); + return procStat(10, 'Z', GROUP_ID); + } + }; + await waitForLinuxProcessGroupExitAsync(GROUP_ID, undefined, racingProcfs); + expect(execFileMock).not.toHaveBeenCalled(); + }); + + it('bounds concurrent stat reads and stops scanning after a live member', async () => { + const pids: string[] = Array.from({ length: 200 }, (unused, index) => String(index + 1)); + let inFlight: number = 0; + let maxInFlight: number = 0; + let reads: number = 0; + let live: boolean = true; + const largeProcfs: ILinuxProcfsReader = { + listEntriesAsync: async () => pids, + readStatAsync: async (pid: string) => { + reads++; + maxInFlight = Math.max(maxInFlight, ++inFlight); + await new Promise((resolve) => setImmediate(resolve)); + inFlight--; + return pid === '1' ? procStat(1, live ? 'R' : 'Z', GROUP_ID) : procStat(Number(pid), 'S', 42); + } + }; + setTimeout(() => { + live = false; + }, 30); + await waitForLinuxProcessGroupExitAsync(GROUP_ID, undefined, largeProcfs); + expect(maxInFlight).toBeLessThanOrEqual(32); + const scans: number = jest.mocked(process.kill).mock.calls.length; + // Each scan while the member is live stops after the first batch; only the final scan reads everything. + expect(scans).toBeGreaterThan(1); + expect(reads).toBeLessThanOrEqual((scans - 1) * 32 + pids.length); + expect(execFileMock).not.toHaveBeenCalled(); + }); + it('does not depend on ps being installed while procfs is readable', async () => { execFileMock.mockImplementation((...args) => reportPs(args, '', Object.assign(new Error('spawn ps ENOENT'), { code: 'ENOENT' }))