diff --git a/src/process/ProcessOwnership.identity.test.ts b/src/process/ProcessOwnership.identity.test.ts index 9cce96f93d..1efe9d8570 100644 --- a/src/process/ProcessOwnership.identity.test.ts +++ b/src/process/ProcessOwnership.identity.test.ts @@ -20,6 +20,51 @@ const observedIdentity = (entry: ProcessTableEntry, identity: string) => ({ }, }); +it("keeps an unresolved earlier generation unknown when a later unrelated process has a foreign marker", async () => { + const row: ProcessTableEntry = { + pid: 920, + parentPid: 1, + processGroupId: 920, + state: "S", + command: "unrelated", + }; + let readable = false; + const evidenceOwner = new ProcessOwnershipEvidenceOwner(); + const signalGroup = vi.fn(); + const host: ProcessOwnershipHost = { + listProcesses: async () => [row], + environment: async () => ({ REA_PROCESS_RUN_ID: "foreign" }), + processIdentities: async (entries) => + new Map( + entries.map((entry) => [ + entry.pid, + readable + ? observedIdentity(entry, "later-generation") + : { state: "unavailable" as const, reason: "identity unavailable" }, + ]), + ), + signalGroup, + }; + const relation = { leaderPid: 100, processGroupId: 100, evidenceOwner }; + await expect( + verifyNoTokenOwnedProcesses("run-token", host, [], relation), + ).resolves.toMatchObject({ + cleaned: false, + failures: [{ pid: row.pid, reason: "process-identity-unavailable" }], + }); + readable = true; + await expect( + verifyNoTokenOwnedProcesses("run-token", host, [], relation), + ).resolves.toMatchObject({ + cleaned: false, + failures: [{ pid: row.pid, reason: "process-identity-unavailable" }], + }); + expect(evidenceOwner.candidates).toMatchObject([ + { pid: row.pid, identity: undefined, applicability: "identity-unresolved" }, + ]); + expect(signalGroup).not.toHaveBeenCalled(); +}); + it.each([undefined, "run-token"])( "validates a current generation with token %s when its baseline identity was unknown", async (runId) => { diff --git a/src/process/ProcessOwnership.movedProbe.test.ts b/src/process/ProcessOwnership.movedProbe.test.ts new file mode 100644 index 0000000000..da74cdd300 --- /dev/null +++ b/src/process/ProcessOwnership.movedProbe.test.ts @@ -0,0 +1,162 @@ +// Regression for a child moving groups during the production probe cleanup. +import { afterEach, expect, it, vi } from "vitest"; +import { EventEmitter } from "node:events"; +import { PassThrough } from "node:stream"; + +const originalPlatform = vi.hoisted(() => { + const descriptor = Object.getOwnPropertyDescriptor(process, "platform")!; + Object.defineProperty(process, "platform", { + value: "linux", + configurable: true, + }); + return descriptor; +}); +const state = vi.hoisted(() => ({ + readFile: vi.fn(), + spawn: vi.fn(), + ps: vi.fn(), +})); +vi.mock("node:fs/promises", async (original) => ({ + ...(await original()), + readFile: state.readFile, +})); +vi.mock("node:child_process", async (original) => ({ + ...(await original()), + spawn: state.spawn, +})); +vi.mock("./ExecFileOutput.js", () => ({ execFileOutput: state.ps })); + +import { runLinuxPrivateDisplayProbe } from "../hopper/LinuxPrivateDisplayProbe.js"; +import { cleanupOwnedProcessGroup } from "./ProcessOwnership.js"; +import { systemProcessOwnershipHost } from "./ProcessOwnershipObservation.js"; + +afterEach(() => { + vi.restoreAllMocks(); + Object.defineProperty(process, "platform", originalPlatform); +}); + +it("runs the production private-display timeout cleanup through the Linux host", async () => { + expect(systemProcessOwnershipHost.platform).toBe("linux"); + const signals: number[] = []; + let token = ""; + let moved = false; + let rootLive = true; + let childLive = true; + const helper = Object.assign(new EventEmitter(), { + pid: 100, + exitCode: null as number | null, + signalCode: null as NodeJS.Signals | null, + stdout: new PassThrough(), + stderr: new PassThrough(), + kill: vi.fn(), + unref: vi.fn(), + }); + state.spawn.mockImplementation((_command, _arguments, options) => { + token = options.env.REA_PROCESS_RUN_ID; + queueMicrotask(() => helper.emit("spawn")); + return helper; + }); + const rows = () => [ + `${process.pid} ${process.ppid} ${process.pid} 1000 S verifier`, + ...(rootLive + ? [`100 ${process.pid} 100 1000 S /usr/bin/python3 helper.py`] + : []), + ...(childLive + ? [`101 ${rootLive ? 100 : 1} ${moved ? 101 : 100} 1000 S Xvfb`] + : []), + ]; + state.ps.mockImplementation(async (command) => { + expect(command).toBe("ps"); + return { stdout: rows().join("\n"), stderr: "" }; + }); + state.readFile.mockImplementation(async (path) => { + const match = /^\/proc\/(\d+)\/(stat|environ)$/u.exec(String(path)); + if (match === null) throw new Error(`Unexpected read: ${String(path)}`); + const pid = Number(match[1]); + if ((pid === 100 && !rootLive) || (pid === 101 && !childLive)) + throw Object.assign(new Error("ESRCH"), { code: "ESRCH" }); + if (match[2] === "environ") { + // A same-generation child moves after the native relation snapshot, + // during the identity/token/identity bracket used by the real host. + if (pid === 101) moved = true; + return pid === process.pid + ? "FOREIGN=value\0" + : `REA_PROCESS_RUN_ID=${token}\0`; + } + const parent = + pid === 100 + ? process.pid + : pid === 101 + ? rootLive + ? 100 + : 1 + : process.ppid; + const group = + pid === 101 ? (moved ? 101 : 100) : pid === 100 ? 100 : process.pid; + const fields = ["S", String(parent), String(group), "100", "0", "0", "0"]; + while (fields.length < 19) fields.push("0"); + fields.push(String(pid * 10)); + return `${pid} (fixture) ${fields.join(" ")}`; + }); + vi.spyOn(process, "kill").mockImplementation((pid, signal) => { + if ( + signal === 0 && + ((pid === 100 && !rootLive) || (pid === 101 && !childLive)) + ) + throw Object.assign(new Error("ESRCH"), { code: "ESRCH" }); + if (pid >= 0 || signal !== "SIGKILL") + throw new Error(`Unexpected signal ${pid}`); + signals.push(-pid); + if (pid === -100) { + rootLive = false; + if (!moved) childLive = false; + helper.signalCode = "SIGKILL"; + helper.stdout.end(); + helper.stderr.end(); + helper.emit("exit", null, "SIGKILL"); + helper.emit("close", null, "SIGKILL"); + } else if (pid === -101) childLive = false; + return true; + }); + const result = await runLinuxPrivateDisplayProbe("direct", { + helperPath: "scripts/hopper-demo-x11.py", + timeoutMs: 1, + environment: {}, + }); + expect(result).toMatchObject({ + outcome: "timed-out", + cleanup: { status: "pending" }, + }); + expect(signals).toEqual([]); + expect(childLive).toBe(true); + expect(moved).toBe(true); + // The launcher may exit while cleanup is pending. Retry the same lease, + // which must retain the moved child instead of treating the empty root as released. + rootLive = false; + helper.signalCode = "SIGKILL"; + helper.stdout.end(); + helper.stderr.end(); + helper.emit("exit", null, "SIGKILL"); + helper.emit("close", null, "SIGKILL"); + if (result.cleanup.status !== "pending") + throw new Error("expected pending cleanup"); + await result.cleanup.lease.release(); + expect(signals).toEqual([101]); + expect(childLive).toBe(false); + + // The same safety applies to callers which use the optional implicit owner. + rootLive = true; + childLive = true; + moved = false; + signals.length = 0; + const implicitOwner = { + runId: token, + leaderPid: 100, + processGroupId: 100, + }; + expect((await cleanupOwnedProcessGroup(implicitOwner)).cleaned).toBe(false); + rootLive = false; + expect((await cleanupOwnedProcessGroup(implicitOwner)).cleaned).toBe(true); + expect(signals).toEqual([101]); + expect(childLive).toBe(false); +}); diff --git a/src/process/ProcessOwnership.ts b/src/process/ProcessOwnership.ts index 027e5de518..e6f373ff83 100644 --- a/src/process/ProcessOwnership.ts +++ b/src/process/ProcessOwnership.ts @@ -340,10 +340,26 @@ const mergeUnverifiedObservations = ( const OWNED_GROUP_REVALIDATION_LIMIT = 2; /** Token-validate every rooted POSIX process group before signaling any. */ +// Callers that do not request a full token sweep can omit a ledger. Keep one +// scoped to their ownership object so a moved member cannot be forgotten on +// a retry. A new ownership object does not inherit signal authority. +const implicitEvidenceOwners = new WeakMap< + OwnedProcessGroup, + ProcessOwnershipEvidenceOwner +>(); + export const cleanupOwnedProcessGroup = async ( ownership: OwnedProcessGroup, host: ProcessOwnershipHost = systemHost, ): Promise => { + if (ownership.evidenceOwner === undefined) { + let evidenceOwner = implicitEvidenceOwners.get(ownership); + if (evidenceOwner === undefined) { + evidenceOwner = new ProcessOwnershipEvidenceOwner(); + implicitEvidenceOwners.set(ownership, evidenceOwner); + } + ownership = { ...ownership, evidenceOwner }; + } const exitSettlement: ProcessExitSettlementBudget = { deadline: undefined }; const processTable = await readLiveProcessTable(host); if (!processTable.available) @@ -917,17 +933,24 @@ const createOwnedCleanupPlan = async ( Number.isSafeInteger(processGroupId) && processGroupId > 0, ), ); + const retainedPids = new Set( + ownership.evidenceOwner?.candidates.map(({ pid }) => pid) ?? [], + ); const tokenOwned = - ownership.sweepTokenOwnedProcesses === true + ownership.sweepTokenOwnedProcesses === true || retainedPids.size > 0 ? await scanTokenOwnedProcesses( - processes, + ownership.sweepTokenOwnedProcesses === true + ? processes + : processes.filter(({ pid }) => retainedPids.has(pid)), ownership.runId, host, ownership.captureBaseline, { leaderPid: ownership.leaderPid, processGroupId: ownership.processGroupId, - evidenceOwner: ownership.evidenceOwner, + // A full sweep requires the owner; retained PIDs can only exist + // when that same persistent owner is present. + evidenceOwner: ownership.evidenceOwner!, ...(sampledProcessGroupIds.size === 0 ? {} : { sampledProcessGroupIds: [...sampledProcessGroupIds] }), @@ -1024,7 +1047,7 @@ const createOwnedCleanupPlan = async ( if (members.length === 0) continue; const failures = await processOwnershipFailures( members, - ownership.runId, + ownership, host, tokenOwned.identities, exitSettlement, @@ -1142,7 +1165,7 @@ const revalidateOwnedProcessGroup = async ( } const failures = await processOwnershipFailures( members, - ownership.runId, + ownership, host, expectedIdentities, exitSettlement, @@ -1255,11 +1278,12 @@ const sanitizedTokenReadFailure = (diagnostic: string | undefined): string => { const processOwnershipFailures = async ( members: readonly ProcessTableEntry[], - runId: string, + ownership: OwnedProcessGroup, host: ProcessOwnershipHost, expectedIdentities: ReadonlyMap = new Map(), exitSettlement: ProcessExitSettlementBudget = { deadline: undefined }, ): Promise => { + const runId = ownership.runId; if (host.processIdentities !== undefined) { const facts = await readProcessOwnershipFacts(members, host, undefined, { exitSettlement, @@ -1284,8 +1308,25 @@ const processOwnershipFailures = async ( if ( relation !== undefined && relation.processGroupId !== fact.process.processGroupId - ) + ) { + // Preserve the observed generation for a later cleanup attempt. The + // earlier table relation scoped it, while token and native identity + // continue to decide whether it can ever be signaled. + ownership.evidenceOwner?.reconcile([fact], { + runId, + listedRows: 1, + listedPids: new Set([fact.process.pid]), + leaderPid: ownership.leaderPid, + processGroupIds: new Set([fact.process.processGroupId]), + scopeFacts: [fact], + }); + failures.push({ + pid: fact.process.pid, + reason: "process-identity-unavailable", + diagnostic: "process group changed during ownership validation", + }); continue; + } if (fact.state !== "same-generation") { failures.push({ pid: fact.process.pid, @@ -1683,7 +1724,12 @@ const scanWithOwnershipEvidence = async ( ) failures.push({ pid: fact.process.pid, - reason: "run-token-mismatch", + // An earlier observation without an identity cannot bind that + // observation to this currently readable process generation. + reason: + retainedCandidate.applicability === "identity-unresolved" + ? "process-identity-unavailable" + : "run-token-mismatch", }); if (fact.token.state !== "unavailable") continue; const relationBefore = diff --git a/tests/boundary/javascript/javascriptSourceAnalysisWorker.test.ts b/tests/boundary/javascript/javascriptSourceAnalysisWorker.test.ts index 062a9b70fa..271f4f5335 100644 --- a/tests/boundary/javascript/javascriptSourceAnalysisWorker.test.ts +++ b/tests/boundary/javascript/javascriptSourceAnalysisWorker.test.ts @@ -8,6 +8,7 @@ import { JavaScriptAnalysisWorker } from "../../../src/javascript/analysis/JavaS import { createJavaScriptSourceAnalysis } from "../../../src/artifacts/javascript/JavaScriptSourceAnalysis.js"; import { ArtifactResourceScope } from "../../../src/artifacts/ArtifactResourceScope.js"; import { cleanupOwnedProcessGroup } from "../../../src/process/ProcessOwnership.js"; +import { ProcessOwnershipEvidenceOwner } from "../../../src/process/ProcessOwnershipEvidence.js"; import type { OwnedProcessGroup } from "../../../src/process/ProcessOwnership.js"; import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js"; import { ProviderCleanupError } from "../../../src/domain/providerCleanupError.js"; @@ -549,7 +550,9 @@ const worker = new JavaScriptAnalysisWorker({ heapMb: 128, timeoutMs: 10000 }, a ] }); // The launcher is deliberately orphaned when this host exits; cleanup still // requires its run token and executable identity, rather than its old parent. - const { expectedParentPid, ...ownership } = launched.ownership; + // The live ledger belongs to this host and cannot be serialized. Transfer + // only the process facts needed for the independent fixture cleanup. + const { expectedParentPid, evidenceOwner, ...ownership } = launched.ownership; await writeFile(${JSON.stringify(ownershipPath)}, JSON.stringify(ownership)); return { ...launched, cleanup: async () => ({ cleaned: false, reason: "fixture inspection unavailable" }) }; }); @@ -575,9 +578,12 @@ catch (error) { const ownership = JSON.parse( await readFile(ownershipPath, "utf8"), ) as OwnedProcessGroup; - expect(await cleanupOwnedProcessGroup(ownership)).toMatchObject({ - cleaned: true, - }); + expect( + await cleanupOwnedProcessGroup({ + ...ownership, + evidenceOwner: new ProcessOwnershipEvidenceOwner(), + }), + ).toMatchObject({ cleaned: true }); } });