diff --git a/docs/reference/windows-edr-posture.md b/docs/reference/windows-edr-posture.md index d44aaf33e38..06eb2d5ff9b 100644 --- a/docs/reference/windows-edr-posture.md +++ b/docs/reference/windows-edr-posture.md @@ -72,12 +72,20 @@ behavioural engine can be expected to score it low. ### Every process gets a handle, on a timer -`src/main/windows/windows-process-table.ts` takes a Toolhelp32 snapshot under one -of two flag sets. Identity (`None | CreationTime`) answers pid/ppid/name from the -snapshot alone and opens nothing; the detailed set adds `CommandLine`, which -costs one `OpenProcess(PROCESS_QUERY_LIMITED_INFORMATION)` per process. `Memory` -is retired — it took a second handle carrying `PROCESS_VM_READ` and never read -through it. +`src/main/windows/windows-process-table.ts` takes a Toolhelp32 snapshot under +**one** flag set, `CommandLine | CreationTime`, shared by every caller. pid, ppid +and name come out of the snapshot itself and open nothing. `CommandLine` is what +opens a handle: the addon calls `GetProcessCommandLine` per process, which opens +`PROCESS_QUERY_INFORMATION | PROCESS_VM_READ` and walks the PEB with three +`ReadProcessMemory` calls (`src/process_commandline.cc:32,41-47` in the vendored +`@vscode/windows-process-tree` 0.8.0 source that `config/patches/` patches). + +`Memory` is retired as of this change, and that is a real reduction: it made +`GetProcessMemoryUsage` open a **second** `PROCESS_QUERY_INFORMATION | +PROCESS_VM_READ` handle per process for a `GetProcessMemoryInfo` call whose +result no caller read (`src/process.cc:47-63`). Dropping it halves the handles +opened per snapshot. It does not remove the remote memory read, because the +command line still performs one. It exists because seven independent readers used to fork `powershell.exe` for a `Get-CimInstance Win32_Process` scan. That cost, measured: a PowerShell @@ -90,23 +98,41 @@ panes multiplied it (#15036). The native snapshot answers the same question in See [`windows-process-enumeration.md`](./windows-process-enumeration.md). -Asking for fewer fields is cheaper, and since the split the module does: one -cache per flag set, so teardown identity and the session owner probe open no -handle at all (6.3 ms p50) while only the callers that read a command line pay -for one (12.3 ms p50, at 492 processes). Each cache still single-flights within -itself, and one gate serializes the native reads because the vendored wrapper -coalesces the flags of two overlapping calls. +Asking for fewer fields is cheaper, and the module now asks for the smallest set +that still answers every caller. There is **no** per-flag-set cache split: one +TTL-cached snapshot serves everyone, deliberately, because a split would restore +the per-pane fan-out the cache exists to remove — a 32-wide teardown has to +collapse into one scan. So the cheap identity-only read is not something any +caller can select; every read pays for `CommandLine`. An earlier revision of this +file described a two-cache design with 6.3 ms / 12.3 ms p50 figures at 492 +processes. That design is not in the tree and those numbers describe no code +path here; the figures that do apply are the module's own, in +[`windows-process-enumeration.md`](./windows-process-enumeration.md). **How an EDR reads it:** a cross-process handle plus a remote memory read against every process on the box, repeating on a cadence, is the read half of the telemetry that credential dumping and process injection produce. MDE surfaced it -as "suspicious memory activity". The memory read is gone: the command line now -comes from the kernel, through `NtQueryInformationProcess`'s -`ProcessCommandLineInformation` class, which needs only -`PROCESS_QUERY_LIMITED_INFORMATION`. `ReadProcessMemory` is absent from the -compiled addon, asserted against the binary's import table because the published -prebuild loads fine and emits byte-identical strings. What is left to declare to -administrators is the per-process handle itself. +as "suspicious memory activity". + +**That signal is still present.** An earlier revision of this file claimed the +command line "now comes from the kernel" through `NtQueryInformationProcess`'s +`ProcessCommandLineInformation` class, needing only +`PROCESS_QUERY_LIMITED_INFORMATION`, and that `ReadProcessMemory` was absent from +the compiled addon. None of that is true of the code we ship. +`process_commandline.cc` calls `NtQueryInformationProcess` with +`ProcessBasicInformation` only — to locate the PEB — and then issues three +`ReadProcessMemory` calls against a `PROCESS_VM_READ` handle to read the PEB, the +`RTL_USER_PROCESS_PARAMETERS`, and the command-line buffer. Nothing asserts an +import table, and no such assertion would pass. + +What this change did remove is the `Memory` flag's second handle and its +`GetProcessMemoryInfo` call, so the per-process handle count per snapshot halves. +What remains to declare to administrators is unchanged in kind: one +`PROCESS_QUERY_INFORMATION | PROCESS_VM_READ` handle and a PEB read against every +process on the box, at the shared snapshot's cadence. Moving to +`ProcessCommandLineInformation` (Windows 8.1+, `PROCESS_QUERY_LIMITED_INFORMATION` +only) would genuinely retire the remote read, but it is an addon patch nobody has +written; treat it as unclaimed work, not as shipped. ### Encoded, policy-bypassing PowerShell diff --git a/docs/reference/windows-process-enumeration.md b/docs/reference/windows-process-enumeration.md index ef1faf5237c..87ac2a97fb1 100644 --- a/docs/reference/windows-process-enumeration.md +++ b/docs/reference/windows-process-enumeration.md @@ -40,6 +40,15 @@ Measured on Windows 11 with 1050 processes (p50 / p95): | + memory + command line | 30.6 ms | 33.7 ms | | `Get-CimInstance` via PowerShell | 706 ms | 723 ms | +Those are the module's published figures. The flag set this module actually +requests is `CommandLine | CreationTime` — **not** `Memory`, which cost a second +`OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ)` plus +`GetProcessMemoryInfo` per process (`src/process.cc:47-63`) for a value nothing +read. Dropping it halves the handles a snapshot opens. The remaining set sits +between the two rows above and has not been measured separately; on a real +Windows host, `Get-Counter '\Process(Orca)\Handle Count'` sampled across a +snapshot cadence is the check. + Those CIM numbers are from a 1050-process host. The scan scales with process count: on a 1486-process Windows SSH host it measured **1.36 s** and produced **4.8 MiB** of JSON, against the fallback's 3 s and 8 MiB limits. Both limits @@ -220,11 +229,13 @@ ownership, and CPU accounting in the memory collector — still reads it through its own query. Those callers are not migrated. Committed private bytes have no equivalent either, and the one memory value the -snapshot does carry is unusable for the sizes Orca now sees: `process.cc` stores +snapshot _can_ carry is unusable for the sizes Orca now sees: `process.cc` stores `pmc.WorkingSetSize` into a `DWORD`, so anything above 4 GB wraps. That is the second reason `windows-process-resource-collector.ts` still runs its own `Get-CimInstance` sweep — it needs `PageFileUsage` (commit) and the CPU-time -counters in the same pass. Migrating it to the native table would cost both. +counters in the same pass. Migrating it to the native table would cost both, and +it is why this module no longer sets the `Memory` flag at all: the field had no +reader, and asking for it opened a handle per process on every snapshot. Start time is a proxy for identity, not identity. The durable answer for the process trees Orca itself spawns is an inherited handle: a job object names the diff --git a/src/main/providers/agent-foreground-process.ts b/src/main/providers/agent-foreground-process.ts index d244e0100dc..2000d35261c 100644 --- a/src/main/providers/agent-foreground-process.ts +++ b/src/main/providers/agent-foreground-process.ts @@ -1,7 +1,9 @@ import { recognizeAgentProcessFromCommandLine } from '../../shared/agent-process-recognition' import { resolveOuterWrapperForegroundProcess } from '../../shared/foreground-wrapper-agent' import { + collectDescendantsFromIndex, getFreshProcessTableSnapshot, + getProcessTableIndex, getProcessTableSnapshot, type ProcessTableRow } from '../../shared/process-table-snapshot' @@ -43,29 +45,6 @@ type ShellForegroundConfirmationOptions = { | Promise | null> } -function collectDescendants( - rows: Row[], - rootPid: number -): (Row & { depth: number })[] { - const childrenByParent = new Map() - for (const row of rows) { - const children = childrenByParent.get(row.ppid) ?? [] - children.push(row) - childrenByParent.set(row.ppid, children) - } - - const descendants: (Row & { depth: number })[] = [] - const stack = (childrenByParent.get(rootPid) ?? []).map((row) => ({ row, depth: 1 })) - while (stack.length > 0) { - const { row, depth } = stack.pop()! - descendants.push({ ...row, depth }) - for (const child of childrenByParent.get(row.pid) ?? []) { - stack.push({ row: child, depth: depth + 1 }) - } - } - return descendants -} - function commandExecutable(command: string): string { const trimmed = command.trim().replace(/^[-]/, '') if (trimmed.startsWith('"') || trimmed.startsWith("'")) { @@ -97,12 +76,12 @@ export async function confirmShellForegroundProcess( } } try { - const rows = await getFreshProcessTableSnapshot() - if (!rows.some((row) => row.pid === shellPid)) { + const index = getProcessTableIndex(await getFreshProcessTableSnapshot()) + const root = index.byPid.get(shellPid) + if (!root) { return false } - const root = rows.find((row) => row.pid === shellPid)! - const tree = [{ ...root, depth: 0 }, ...collectDescendants(rows, shellPid)] + const tree = [{ ...root, depth: 0 }, ...collectDescendantsFromIndex(index, shellPid)] const spawnedShellBasename = executableBasename(spawnedShellProcess) const foregroundShell = tree .filter( @@ -172,7 +151,7 @@ export async function resolveAgentForegroundProcessWithAvailability( const rows = options.fresh ? await getFreshProcessTableSnapshot() : await getProcessTableSnapshot() - if (options.fresh && !rows.some((row) => row.pid === shellPid)) { + if (options.fresh && !getProcessTableIndex(rows).byPid.has(shellPid)) { return { available: false, processName: fallbackProcess } } return { @@ -186,11 +165,13 @@ export async function resolveAgentForegroundProcessWithAvailability( } export function resolveAgentForegroundProcessFromPs( - rows: ProcessTableRow[], + rows: readonly ProcessTableRow[], shellPid: number ): string | null { - const shellRow = rows.find((row) => row.pid === shellPid) - const candidates = collectDescendants(rows, shellPid) + // Memoized per snapshot identity, so the caller's own index build is reused. + const index = getProcessTableIndex(rows) + const shellRow = index.byPid.get(shellPid) + const candidates = collectDescendantsFromIndex(index, shellPid) // Why: `+` in `ps stat` marks the process holding the terminal foreground. // The root shell can hold it after Ctrl-Z, so use the whole PTY tree as the // foreground gate; otherwise a stopped agent child still masquerades as live. diff --git a/src/main/providers/windows-foreground-process-inspection-cost.test.ts b/src/main/providers/windows-foreground-process-inspection-cost.test.ts new file mode 100644 index 00000000000..f08d1b690bb --- /dev/null +++ b/src/main/providers/windows-foreground-process-inspection-cost.test.ts @@ -0,0 +1,156 @@ +// Regression guard on the per-inspection cost of Windows agent foreground +// inspection — the Windows analogue of the POSIX index memo (#6288). +// +// The shared TTL cache already collapses N panes into one Toolhelp32 snapshot +// (windows-agent-foreground-process-scan-volume.test.ts). What it never +// collapsed is the work each pane does ON that snapshot: a full +// `native.map(toProcessRow)` projection, a `childrenByPpid` Map rebuilt from +// scratch, and two linear scans. This file counts that work at a realistic +// table size and pane count, and pins the flag set the snapshot asks for. +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +import { __setWindowsProcessTreeLoaderForTests } from '../windows/windows-process-table' +import { + queryWindowsPaneProcessInventory, + resetWindowsProcessRowsSnapshotForTests +} from './windows-foreground-process-rows' + +// 1050 processes is the host measured in windows-process-enumeration.md; 11 +// panes is the fan-out the shared snapshot exists to serve. +const TABLE_SIZE = 1050 +const PANE_COUNT = 11 + +const SELF_ROW = { pid: process.pid, ppid: 0, name: 'vitest.exe', commandLine: 'vitest' } + +const shellPid = (pane: number): number => 10_000 + pane * 10 +const agentPid = (pane: number): number => shellPid(pane) + 1 +/** A row every pane can look up, so distinct results == distinct projections. */ +const PROBE_PID = 900_000 + TABLE_SIZE - 1 + +/** One shell + one agent child per pane, padded out to a real table size. */ +function buildNativeTable(): { pid: number; ppid: number; name: string; commandLine: string }[] { + const rows = [SELF_ROW] + for (let pane = 0; pane < PANE_COUNT; pane += 1) { + rows.push({ pid: shellPid(pane), ppid: 4, name: 'cmd.exe', commandLine: 'cmd.exe' }) + rows.push({ + pid: agentPid(pane), + ppid: shellPid(pane), + name: 'node.exe', + commandLine: 'node C:/Users/dev/AppData/codex/bin/codex.js' + }) + } + for (let filler = rows.length; filler < TABLE_SIZE; filler += 1) { + rows.push({ pid: 900_000 + filler, ppid: 4, name: 'svchost.exe', commandLine: 'svchost.exe' }) + } + return rows +} + +const NATIVE_TABLE = buildNativeTable() + +/** + * Count `Map.prototype.set` calls — the primitive both the old per-call + * `childrenByPpid` rebuild and the shared index build are made of. Patched for + * one awaited region and restored in `finally`, so nothing else observes it. + */ +async function countMapInsertions(run: () => Promise): Promise { + const original = Map.prototype.set + let insertions = 0 + Map.prototype.set = function patched(this: Map, key: unknown, value: unknown) { + insertions += 1 + return original.call(this, key, value) + } as typeof Map.prototype.set + try { + await run() + } finally { + Map.prototype.set = original + } + return insertions +} + +describe('windows foreground inspection cost per pane', () => { + const getAllProcesses = vi.fn() + let platform: PropertyDescriptor | undefined + let flagsSeen: number[] = [] + + beforeEach(() => { + flagsSeen = [] + getAllProcesses.mockReset() + getAllProcesses.mockImplementation((cb: (rows: unknown) => void, flags: number) => { + flagsSeen.push(flags) + cb(NATIVE_TABLE) + }) + platform = Object.getOwnPropertyDescriptor(process, 'platform') + Object.defineProperty(process, 'platform', { configurable: true, value: 'win32' }) + __setWindowsProcessTreeLoaderForTests(() => ({ + ProcessDataFlag: { None: 0, Memory: 1, CommandLine: 2, CreationTime: 4 }, + getAllProcesses + })) + resetWindowsProcessRowsSnapshotForTests() + vi.useFakeTimers({ toFake: ['Date'] }) + vi.setSystemTime(0) + }) + + afterEach(() => { + vi.useRealTimers() + __setWindowsProcessTreeLoaderForTests() + if (platform) { + Object.defineProperty(process, 'platform', platform) + } + }) + + async function sweepPanes(): Promise<(number | undefined)[]> { + const resolved: (number | undefined)[] = [] + for (let pane = 0; pane < PANE_COUNT; pane += 1) { + const inventory = await queryWindowsPaneProcessInventory(shellPid(pane), { + anchorPid: agentPid(pane) + }) + expect(inventory?.candidates).toHaveLength(1) + resolved.push(inventory?.candidates[0]?.pid) + } + return resolved + } + + it('never sets the Memory flag on the snapshot', async () => { + await queryWindowsPaneProcessInventory(shellPid(0)) + expect(flagsSeen).toHaveLength(1) + // Memory is bit 0, and it costs the addon a second OpenProcess per process + // carrying PROCESS_VM_READ (process.cc `GetProcessMemoryUsage`). + expect(flagsSeen[0]! & 1).toBe(0) + // CommandLine (2) | CreationTime (4). + expect(flagsSeen[0]).toBe(6) + }) + + it('projects the shared snapshot once for the whole pane fan-out', async () => { + const probeRows: unknown[] = [] + for (let pane = 0; pane < PANE_COUNT; pane += 1) { + const inventory = await queryWindowsPaneProcessInventory(shellPid(pane), { + anchorPid: PROBE_PID + }) + probeRows.push(inventory?.anchorRow) + } + expect(probeRows.filter(Boolean)).toHaveLength(PANE_COUNT) + // One projection produced every pane's row object. Pre-fix each pane ran + // its own `native.map(toProcessRow)` over all 1050 rows, so this set held + // PANE_COUNT distinct objects and the sweep allocated PANE_COUNT * 1050. + expect(new Set(probeRows).size).toBe(1) + }) + + it('indexes the shared snapshot once for the whole pane fan-out', async () => { + // Prime the TTL cache and the index so the snapshot read is not in the count. + await queryWindowsPaneProcessInventory(shellPid(0), { anchorPid: agentPid(0) }) + + const insertions = await countMapInsertions(async () => { + await sweepPanes() + }) + + // Pre-fix every pane rebuilt a whole-table `childrenByPpid`, so this was + // >= PANE_COUNT * (rows with a distinct ppid). One shared index makes the + // whole sweep cost no table-sized Map build at all. + expect(insertions).toBeLessThan(TABLE_SIZE) + }) + + it('resolves the same foreground child for every pane as an unshared scan would', async () => { + const resolved = await sweepPanes() + expect(resolved).toEqual(Array.from({ length: PANE_COUNT }, (_, pane) => agentPid(pane))) + }) +}) diff --git a/src/main/providers/windows-foreground-process-rows.ts b/src/main/providers/windows-foreground-process-rows.ts index f5a74bd0fe2..5be4734dddc 100644 --- a/src/main/providers/windows-foreground-process-rows.ts +++ b/src/main/providers/windows-foreground-process-rows.ts @@ -1,3 +1,7 @@ +import { + collectDescendantsFromIndex, + getProcessTableIndex +} from '../../shared/process-table-snapshot' import { readWindowsProcessTable, readWindowsProcessTableFresh, @@ -25,15 +29,40 @@ function toProcessRow(row: NativeWindowsProcessRow): WindowsProcessRow { } } +/** + * One projection per snapshot identity, mirroring `getProcessTableIndex`. + * + * The TTL cache already gives every pane the same native rows array; without + * this each of them still rebuilt ~1050 row objects, which also handed + * `getProcessTableIndex` a new array each time and defeated its memo by + * construction. Keyed weakly, so a projection dies with its snapshot. Rows are + * shared, never mutated: descendants are copied with their depth, and + * `anchorRow` is read-only to every caller. + */ +const projectedRows = new WeakMap() + +function projectProcessRows(native: readonly NativeWindowsProcessRow[]): WindowsProcessRow[] { + const cached = projectedRows.get(native) + if (cached) { + return cached + } + const rows = native.map(toProcessRow) + projectedRows.set(native, rows) + return rows +} + /** * Rows from a scan that starts after this call. * * PID-identity checks in teardown must not reuse a cached row — it can predate * the very recycle it is meant to detect. Rejects when the table is unreadable, * so "unavailable" stays distinguishable from "nothing is running". + * + * `readonly` because the projection is shared with every other reader of the + * same snapshot. */ -export async function queryWindowsProcessRowsFresh(): Promise { - return (await readWindowsProcessTableFresh()).map(toProcessRow) +export async function queryWindowsProcessRowsFresh(): Promise { + return projectProcessRows(await readWindowsProcessTableFresh()) } export async function queryWindowsProcessDescendants( @@ -63,21 +92,22 @@ export async function queryWindowsPaneProcessInventory( options.fresh === true ? await readWindowsProcessTableFresh() : await readWindowsProcessTable() - rows = native.map(toProcessRow) + rows = projectProcessRows(native) } catch { return null } + // One index per snapshot, shared by every pane inspecting inside the TTL + // window: `byPid` answers both lookups that used to be linear scans, and + // `childrenByPpid` replaces a per-call Map rebuild over the whole table. + const index = getProcessTableIndex(rows) // Why: a snapshot that omitted the PTY root may be stale or permission- // filtered; only an observed root can authoritatively have no descendants. - if (!rows.some((row) => row.pid === rootPid)) { + if (!index.byPid.has(rootPid)) { return null } return { - candidates: collectDescendants(rows, rootPid).sort((a, b) => b.depth - a.depth), - anchorRow: - options.anchorPid !== undefined - ? (rows.find((row) => row.pid === options.anchorPid) ?? null) - : null + candidates: collectDescendantsFromIndex(index, rootPid).sort((a, b) => b.depth - a.depth), + anchorRow: options.anchorPid !== undefined ? (index.byPid.get(options.anchorPid) ?? null) : null } } @@ -85,26 +115,3 @@ export async function queryWindowsPaneProcessInventory( export function resetWindowsProcessRowsSnapshotForTests(): void { resetWindowsProcessTableForTests() } - -function collectDescendants( - rows: Row[], - rootPid: number -): (Row & { depth: number })[] { - const childrenByParent = new Map() - for (const row of rows) { - const children = childrenByParent.get(row.ppid) ?? [] - children.push(row) - childrenByParent.set(row.ppid, children) - } - - const descendants: (Row & { depth: number })[] = [] - const stack = (childrenByParent.get(rootPid) ?? []).map((row) => ({ row, depth: 1 })) - while (stack.length > 0) { - const { row, depth } = stack.pop()! - descendants.push({ ...row, depth }) - for (const child of childrenByParent.get(row.pid) ?? []) { - stack.push({ row: child, depth: depth + 1 }) - } - } - return descendants -} diff --git a/src/main/windows/windows-process-table-cim-scan.ts b/src/main/windows/windows-process-table-cim-scan.ts index 898f0c36f33..213f157f63b 100644 --- a/src/main/windows/windows-process-table-cim-scan.ts +++ b/src/main/windows/windows-process-table-cim-scan.ts @@ -75,8 +75,6 @@ export function parseWindowsCimProcessRows(stdout: string): WindowsProcessRow[] return [] } const name = fieldAsString(row.Name) - // memoryBytes stays undefined: Win32_Process reports WorkingSetSize, but no - // caller reads it off this table and asking widens an already costly scan. return [{ pid, ppid, name, command: fieldAsString(row.CommandLine) || name }] }) } diff --git a/src/main/windows/windows-process-table.test.ts b/src/main/windows/windows-process-table.test.ts index 609de009820..360dae4ec14 100644 --- a/src/main/windows/windows-process-table.test.ts +++ b/src/main/windows/windows-process-table.test.ts @@ -52,21 +52,24 @@ describe('windows process table', () => { it('maps native rows, defaulting an unreadable command line to empty', async () => { const rows = await readWindowsProcessTableFresh() expect(rows).toEqual([ - { pid: process.pid, ppid: 0, name: 'vitest.exe', command: '', memoryBytes: undefined }, + { pid: process.pid, ppid: 0, name: 'vitest.exe', command: '' }, { pid: 100, ppid: 4, name: 'orca.exe', command: '"C:/a b/orca.exe" --x', - memoryBytes: 4096, creationTimeMs: 1_700_000_000_000 } ]) }) - it('requests memory and command line together', async () => { + it('requests the command line and creation time, never memory', async () => { await readWindowsProcessTableFresh() - expect(getAllProcesses.mock.calls[0]?.[1]).toBe(7) + // CommandLine (2) | CreationTime (4). The Memory bit (1) stays clear: the + // addon opens a second PROCESS_VM_READ handle per process to serve it and + // nothing reads a working set off this table. + expect(getAllProcesses.mock.calls[0]?.[1]).toBe(6) + expect((getAllProcesses.mock.calls[0]?.[1] as number) & 1).toBe(0) }) it('only advertises PID-safe ownership when the native creation-time field exists', () => { @@ -400,20 +403,19 @@ describe('resolving the native reader', () => { }) const rows = await readWindowsProcessTableFresh() expect(rows).toEqual([ - { pid: process.pid, ppid: 0, name: 'vitest.exe', command: '', memoryBytes: undefined }, + { pid: process.pid, ppid: 0, name: 'vitest.exe', command: '' }, { pid: 100, ppid: 4, name: 'orca.exe', command: '"C:/a b/orca.exe" --x', - memoryBytes: 4096, creationTimeMs: 1_700_000_000_000 } ]) expect(isWindowsProcessTableAvailable()).toBe(true) }) - it('asks the addon for memory and command line, as the package path does', async () => { + it('asks the addon for the command line but not memory, as the package path does', async () => { const addon = addonReturning(NATIVE) __setWindowsProcessTreeRequireForTests((specifier: string) => { if (specifier === ADDON_SPECIFIER) { @@ -422,9 +424,10 @@ describe('resolving the native reader', () => { throw new Error('MODULE_NOT_FOUND') }) await readWindowsProcessTableFresh() - // Memory | CommandLine. A bare snapshot would silently drop the command - // line every agent-recognition caller matches on first. - expect(addon.getProcessList).toHaveBeenCalledWith(expect.any(Function), 3) + // CommandLine only: a bare snapshot would silently drop the command line + // every agent-recognition caller matches on first, and the relay addon + // exposes no CreationTime bit to add. + expect(addon.getProcessList).toHaveBeenCalledWith(expect.any(Function), 2) }) it('reaches the CIM scan when neither the package nor the addon is present', async () => { diff --git a/src/main/windows/windows-process-table.ts b/src/main/windows/windows-process-table.ts index 9308c64a9f0..42d48f4fc79 100644 --- a/src/main/windows/windows-process-table.ts +++ b/src/main/windows/windows-process-table.ts @@ -23,6 +23,10 @@ import { readWindowsProcessRowsWithCim } from './windows-process-table-cim-scan' * pid+ppid+name 15.9 / 17.5 ms * +memory +commandLine 30.6 / 33.7 ms * PowerShell CIM 706 / 723 ms + * + * Those are the module's published figures for both extra fields together; the + * only flag set this module asks for is `CommandLine` (+ `CreationTime`, free), + * which sits between the two rows and has not been separately measured. */ export type WindowsProcessRow = { @@ -31,8 +35,6 @@ export type WindowsProcessRow = { name: string /** Full command line. Empty when the process denied a query handle. */ command: string - /** Working set in bytes, or undefined when not requested/queryable. */ - memoryBytes?: number /** Process creation time in Unix milliseconds, when the native snapshot provides it. */ creationTimeMs?: number } @@ -41,7 +43,6 @@ type NativeProcessInfo = { pid: number ppid: number name: string - memory?: number commandLine?: string creationTimeMs?: number } @@ -49,7 +50,6 @@ type NativeProcessInfo = { type WindowsProcessTreeModule = { ProcessDataFlag: { None: number - Memory: number CommandLine: number CreationTime?: number } @@ -82,7 +82,10 @@ type WindowsProcessTreeAddon = { ) => void } -/** Mirrors the package's enum; the addon takes the raw bit field. */ +/** + * Mirrors the package's enum; the addon takes the raw bit field. `Memory` (1) + * is listed for completeness and is deliberately never set — see `flags` below. + */ const PROCESS_DATA_FLAG = { None: 0, Memory: 1, CommandLine: 2 } as const /** Staged beside the relay bundle by build-relay; see RELAY_ARTIFACTS. */ @@ -190,16 +193,16 @@ function readNativeRows(): Promise { } const readId = ++readSequence const readerEpoch = nativeReaderEpoch - // Why always both flags: each adds an OpenProcess per process (Memory a - // GetProcessMemoryInfo, CommandLine a PEB read), so asking for less would be - // cheaper -- 15.9ms p50 versus 30.6ms at 1050 processes. But every read shares - // one snapshot so a 32-wide teardown collapses into a single scan, and that - // snapshot has to satisfy every caller. Splitting the cache per field set - // would restore exactly the fan-out it exists to prevent. - const flags = - native.ProcessDataFlag.Memory | - native.ProcessDataFlag.CommandLine | - (native.ProcessDataFlag.CreationTime ?? 0) + // Why CommandLine but not Memory: each flag costs one OpenProcess per process + // inside the addon (process.cc), and every caller of this table matches on + // `command`, while nothing reads a working set off it -- the Resource Manager + // runs its own CIM sweep because it needs commit and CPU time in one pass, and + // `process.cc` truncates the working set into a DWORD anyway. Dropping Memory + // halves the per-snapshot handle count; the remaining flags stay in ONE flag + // set because every read shares one snapshot, so a 32-wide teardown collapses + // into a single scan. Splitting the cache per field set would restore exactly + // the fan-out it exists to prevent. + const flags = native.ProcessDataFlag.CommandLine | (native.ProcessDataFlag.CreationTime ?? 0) return new Promise((resolve, reject) => { // Hoisted so a synchronous throw from getAllProcesses can clear it. An // orphaned timer would otherwise fire later and wedge a reader that had @@ -241,7 +244,6 @@ function readNativeRows(): Promise { ppid: row.ppid, name: row.name, command: row.commandLine ?? '', - memoryBytes: row.memory, ...(typeof row.creationTimeMs === 'number' ? { creationTimeMs: row.creationTimeMs } : {}) diff --git a/src/shared/process-table-snapshot.ts b/src/shared/process-table-snapshot.ts index 2d65fc3f324..54764767740 100644 --- a/src/shared/process-table-snapshot.ts +++ b/src/shared/process-table-snapshot.ts @@ -124,28 +124,36 @@ export type ProcessTableIndexStats = { indexLookups: number } -export type ProcessTableIndex = { - rows: readonly ProcessTableRow[] - byPid: ReadonlyMap - childrenByPpid: ReadonlyMap +/** The parent/child fields every process-table row shape shares. */ +export type ProcessIdentityRow = { pid: number; ppid: number } + +export type ProcessTableIndexOf = { + rows: readonly Row[] + byPid: ReadonlyMap + childrenByPpid: ReadonlyMap stats?: ProcessTableIndexStats } +export type ProcessTableIndex = ProcessTableIndexOf + /** * Build the correlation indexes in one linear pass over a capture. Only the * indexes a resolver actually reads are materialized: group indexes would cost * two more maps plus a per-row array allocation on every capture, and foreground * membership is derived from each row's own `pgid` against the root's `tpgid`. + * + * Generic over the row shape so the Windows snapshot (`pid`/`ppid`/`name`/ + * `command`) shares this pass rather than carrying a parallel one. */ -export function buildProcessTableIndex( - rows: readonly ProcessTableRow[], +export function buildProcessTableIndex( + rows: readonly Row[], stats?: ProcessTableIndexStats -): ProcessTableIndex { +): ProcessTableIndexOf { if (stats) { stats.indexBuilds += 1 } - const byPid = new Map() - const childrenByPpid = new Map() + const byPid = new Map() + const childrenByPpid = new Map() for (const row of rows) { if (stats) { stats.rowVisits += 1 @@ -161,6 +169,29 @@ export function buildProcessTableIndex( return { rows, byPid, childrenByPpid, stats } } +/** + * Depth-first descendants of `rootPid`, deepest-last, off a prebuilt index. + * + * Each row is copied with its depth, so callers may not mutate the index's rows + * through the result. Ordering matches a per-call `childrenByPpid` walk exactly: + * children keep capture order and the stack pops last-pushed first. + */ +export function collectDescendantsFromIndex( + index: ProcessTableIndexOf, + rootPid: number +): (Row & { depth: number })[] { + const descendants: (Row & { depth: number })[] = [] + const stack = (index.childrenByPpid.get(rootPid) ?? []).map((row) => ({ row, depth: 1 })) + while (stack.length > 0) { + const { row, depth } = stack.pop()! + descendants.push({ ...row, depth }) + for (const child of index.childrenByPpid.get(row.pid) ?? []) { + stack.push({ row: child, depth: depth + 1 }) + } + } + return descendants +} + /** * Rank a descendant row as a foreground candidate: a `+` (foreground process * group) row always outranks a background one, then the deepest wins. @@ -169,9 +200,9 @@ export function scoreForegroundCandidateRow(row: ProcessTableRow & { depth: numb return (row.stat.includes('+') ? 10_000 : 0) + row.depth } -export function lookupProcessTableIndex( - index: ProcessTableIndex, - lookup: (index: ProcessTableIndex) => T, +export function lookupProcessTableIndex( + index: ProcessTableIndexOf, + lookup: (index: ProcessTableIndexOf) => T, stats = index.stats ): T { if (stats) { @@ -180,7 +211,9 @@ export function lookupProcessTableIndex( return lookup(index) } -const processTableIndexes = new WeakMap() +// Keyed by array identity, which also pins the row shape the entry was built +// for, so the one cast below cannot hand a caller another row type's index. +const processTableIndexes = new WeakMap() /** * Memoize one index per snapshot identity, so the panes that share a TTL-cached @@ -195,8 +228,10 @@ const processTableIndexes = new WeakMap( + rows: readonly Row[] +): ProcessTableIndexOf { + const cached = processTableIndexes.get(rows) as ProcessTableIndexOf | undefined if (cached) { return cached }