diff --git a/src/main/crash-reporting/chromium-utility-service-recoverability.test.ts b/src/main/crash-reporting/chromium-utility-service-recoverability.test.ts new file mode 100644 index 00000000000..84dabbaca70 --- /dev/null +++ b/src/main/crash-reporting/chromium-utility-service-recoverability.test.ts @@ -0,0 +1,39 @@ +import { describe, expect, it } from 'vitest' +import { isRecoverableChromiumUtilityService } from './chromium-utility-service-recoverability' + +describe('isRecoverableChromiumUtilityService', () => { + it('treats the on-demand services from the 1.4.188 crash batch as recoverable', () => { + expect(isRecoverableChromiumUtilityService('proxy_resolver.mojom.ProxyResolverFactory')).toBe( + true + ) + expect(isRecoverableChromiumUtilityService('printing.mojom.PrintCompositor')).toBe(true) + }) + + it('keeps the previously enumerated services recoverable', () => { + expect(isRecoverableChromiumUtilityService('audio.mojom.AudioService')).toBe(true) + expect(isRecoverableChromiumUtilityService('network.mojom.NetworkService')).toBe(true) + expect(isRecoverableChromiumUtilityService('video_capture.mojom.VideoCaptureService')).toBe( + true + ) + }) + + it('covers Chromium services the allowlist never enumerated', () => { + expect(isRecoverableChromiumUtilityService('data_decoder.mojom.DataDecoderService')).toBe(true) + expect(isRecoverableChromiumUtilityService('media.mojom.MediaFoundationService')).toBe(true) + expect(isRecoverableChromiumUtilityService('tracing.mojom.TracedProcess')).toBe(true) + }) + + it('reports the utility that hosts Orca code and the one holding user data', () => { + expect(isRecoverableChromiumUtilityService('node.mojom.NodeService')).toBe(false) + expect(isRecoverableChromiumUtilityService('storage.mojom.StorageService')).toBe(false) + }) + + it('reports anything that is not a Chromium Mojo service name', () => { + expect(isRecoverableChromiumUtilityService(undefined)).toBe(false) + expect(isRecoverableChromiumUtilityService('')).toBe(false) + expect(isRecoverableChromiumUtilityService('com.orca.unexpected')).toBe(false) + expect(isRecoverableChromiumUtilityService('Orca Helper')).toBe(false) + expect(isRecoverableChromiumUtilityService('mojom.Thing')).toBe(false) + expect(isRecoverableChromiumUtilityService('printing.mojom.')).toBe(false) + }) +}) diff --git a/src/main/crash-reporting/chromium-utility-service-recoverability.ts b/src/main/crash-reporting/chromium-utility-service-recoverability.ts new file mode 100644 index 00000000000..78204b39c42 --- /dev/null +++ b/src/main/crash-reporting/chromium-utility-service-recoverability.ts @@ -0,0 +1,35 @@ +// Chromium spawns utility services on demand and tears them down as routine +// churn, so an allowlist of them can only ever be behind: 1.4.188 filed the PAC +// evaluator and the print compositor as user-facing crashes because neither was +// enumerated. We cannot enumerate Chromium's service list, but we can enumerate +// the services whose death is ours to answer for, so the default inverts and the +// exceptions below carry the enumeration. +const NON_RECOVERABLE_UTILITY_SERVICE_NAMES = new Set([ + // Electron's utilityProcess.fork() host: this is Orca's own code dying. + 'node.mojom.NodeService', + // Backs durable profile storage, so an exit can mean real user data loss. + 'storage.mojom.StorageService' +]) + +// Mojo service names are `.mojom.`; anything else is not a +// Chromium-owned service and has to keep reporting. +const CHROMIUM_MOJO_SERVICE_NAME = /^[a-z0-9_]+(?:\.[a-z0-9_]+)*\.mojom\.[A-Za-z0-9_]+$/ + +/** + * Whether a `child-process-gone` utility exit is routine Chromium churn. + * + * Suppression is breadcrumb-only. The exit lands in the durable trail as + * `process_gone_suppressed` carrying {source, processType, serviceName, reason, + * exitCode} and nothing more: no crash report, no minidump signature, no + * `electron.minidump_signature` span in the diagnostic bundle, and the dump is + * never claimed, so pruning can reclaim it. A suppressed CHECK failure stays an + * anonymous 0x80000003 -- no name, file, line or stack survives anywhere. + * So deny a service below whenever we would need that post-mortem detail to act + * on its death, not merely when the service sounds important. + */ +export function isRecoverableChromiumUtilityService(serviceName: string | undefined): boolean { + if (serviceName === undefined || NON_RECOVERABLE_UTILITY_SERVICE_NAMES.has(serviceName)) { + return false + } + return CHROMIUM_MOJO_SERVICE_NAME.test(serviceName) +} diff --git a/src/main/crash-reporting/crashpad-capture.ts b/src/main/crash-reporting/crashpad-capture.ts index 1f628fe9c24..7aed896b0c3 100644 --- a/src/main/crash-reporting/crashpad-capture.ts +++ b/src/main/crash-reporting/crashpad-capture.ts @@ -32,6 +32,15 @@ const MAX_STORED_DUMP_BYTES = 128 * 1024 * 1024 const MAX_STORED_DUMPS = 64 const DUMP_PRUNE_DELAY_MS = 2_000 +type SuppressedDumpBarrier = { + readonly processType: string + readonly armedAtMs: number +} + +// Bounds the list across a crash loop; a barrier older than the recency window +// can no longer match any capture anyway. +const MAX_SUPPRESSED_DUMP_BARRIERS = 32 + type DumpCandidate = { readonly filePath: string readonly mtimeMs: number @@ -46,6 +55,9 @@ let captureStarted = false let captureStartedAtMs: number | null = null const claimedDumpPaths = new Map() const reservedDumpPaths = new Set() +// Suppressed crashes still leave a dump behind and never claim one, so each is +// fenced off here for the next same-type capture to consume instead of adopt. +let suppressedDumpBarriers: SuppressedDumpBarrier[] = [] let dumpPruneTimer: NodeJS.Timeout | null = null export type CrashpadCaptureOptions = { @@ -110,6 +122,7 @@ export function _setCrashpadCaptureStateForTest( captureStartedAtMs = state?.started ? (state.startedAtMs ?? Number.NEGATIVE_INFINITY) : null claimedDumpPaths.clear() reservedDumpPaths.clear() + suppressedDumpBarriers = [] if (dumpPruneTimer) { clearTimeout(dumpPruneTimer) dumpPruneTimer = null @@ -269,6 +282,45 @@ export async function waitForCrashMinidump( return pollDumpCandidates(crashedAtMs, options, async (candidate) => candidate) } +/** + * Records that a crash we deliberately did not report may have left a dump. + * + * Why: the dump is on disk with nothing pointing at it, and the next reportable + * crash of the same process type polls for "a recent dump of my type" — it would + * otherwise attach the suppressed service's CHECK file, line and stack to an + * unrelated report, sending a triager after the wrong crash. + */ +export function noteSuppressedCrashDump(processType: string, crashedAtMs: number): void { + suppressedDumpBarriers.push({ processType, armedAtMs: crashedAtMs }) + if (suppressedDumpBarriers.length > MAX_SUPPRESSED_DUMP_BARRIERS) { + suppressedDumpBarriers.shift() + } +} + +/** One barrier absorbs one dump: consume it so a genuine later crash of the + * same type still pairs with the dump Crashpad writes for it. */ +function consumeSuppressedDumpBarrier( + dump: DumpCandidate, + processType: string | undefined, + crashedAtMs: number +): boolean { + if (processType === undefined) { + return false + } + const index = suppressedDumpBarriers.findIndex( + (barrier) => + barrier.processType === processType && + barrier.armedAtMs <= crashedAtMs && + barrier.armedAtMs >= crashedAtMs - DUMP_RECENCY_WINDOW_MS && + dump.mtimeMs >= barrier.armedAtMs + ) + if (index === -1) { + return false + } + suppressedDumpBarriers.splice(index, 1) + return true +} + export type CapturedMinidump = { readonly filePath: string readonly sizeBytes: number @@ -303,6 +355,11 @@ export async function captureMinidumpSignature( rejectedDumpPaths.add(dump.filePath) return null } + if (consumeSuppressedDumpBarrier(dump, options.expectedProcessType, crashedAtMs)) { + // Deliberately not claimed: a suppressed crash's dump stays prunable. + rejectedDumpPaths.add(dump.filePath) + return null + } claimedDumpPaths.set(dump.filePath, dump.mtimeMs) return { filePath: dump.filePath, sizeBytes: dump.size, signature } } finally { diff --git a/src/main/crash-reporting/crashpad-process-type.ts b/src/main/crash-reporting/crashpad-process-type.ts new file mode 100644 index 00000000000..9dc474dcd4d --- /dev/null +++ b/src/main/crash-reporting/crashpad-process-type.ts @@ -0,0 +1,14 @@ +// Electron's process-gone `processType` vocabulary is not Crashpad's dump +// annotation vocabulary; map once so both the report and the suppression path +// fence dumps by the same name. +const CHILD_CRASHPAD_PROCESS_TYPES: Readonly> = { + gpu: 'gpu-process', + utility: 'utility', + zygote: 'zygote' +} + +export function crashpadProcessTypeFor(source: string, processType: string): string | null { + return source === 'renderer' + ? 'renderer' + : (CHILD_CRASHPAD_PROCESS_TYPES[processType.trim().toLowerCase()] ?? null) +} diff --git a/src/main/crash-reporting/process-gone-classification.test.ts b/src/main/crash-reporting/process-gone-classification.test.ts index 78522b76617..a4e81a85c46 100644 --- a/src/main/crash-reporting/process-gone-classification.test.ts +++ b/src/main/crash-reporting/process-gone-classification.test.ts @@ -226,6 +226,63 @@ describe('shouldRecordProcessGoneCrash', () => { ).toBe(true) }) + // 1.4.188 batch: both were filed as user-facing crashes. The PAC evaluator report + // even carried a bug note about opening a 14MB JSON file. + it('suppresses on-demand Chromium utility teardown outside the old allowlist', () => { + expect( + shouldRecordProcessGoneCrash({ + source: 'child', + processType: 'Utility', + serviceName: 'proxy_resolver.mojom.ProxyResolverFactory', + reason: 'killed', + exitCode: 1, + expectedTeardown: 'none' + }) + ).toBe(false) + expect( + shouldRecordProcessGoneCrash({ + source: 'child', + processType: 'Utility', + serviceName: 'printing.mojom.PrintCompositor', + reason: 'crashed', + exitCode: -1, + expectedTeardown: 'none' + }) + ).toBe(false) + }) + + it('still records utility exits Chromium does not own', () => { + expect( + shouldRecordProcessGoneCrash({ + source: 'child', + processType: 'Utility', + serviceName: 'node.mojom.NodeService', + reason: 'crashed', + exitCode: -1, + expectedTeardown: 'none' + }) + ).toBe(true) + expect( + shouldRecordProcessGoneCrash({ + source: 'child', + processType: 'Utility', + serviceName: 'storage.mojom.StorageService', + reason: 'killed', + exitCode: 1, + expectedTeardown: 'none' + }) + ).toBe(true) + expect( + shouldRecordProcessGoneCrash({ + source: 'child', + processType: 'Utility', + reason: 'crashed', + exitCode: -1, + expectedTeardown: 'none' + }) + ).toBe(true) + }) + it('still records severe Chromium child process failures', () => { expect( shouldRecordProcessGoneCrash({ diff --git a/src/main/crash-reporting/process-gone-classification.ts b/src/main/crash-reporting/process-gone-classification.ts index d3f363d1332..835b2ccf6ef 100644 --- a/src/main/crash-reporting/process-gone-classification.ts +++ b/src/main/crash-reporting/process-gone-classification.ts @@ -1,15 +1,10 @@ +import { isRecoverableChromiumUtilityService } from './chromium-utility-service-recoverability' + export type ProcessGoneSource = 'renderer' | 'child' export type ExpectedTeardownScope = 'none' | 'renderer-reload' | 'app-shutdown' const WINDOWS_CONTROL_TERMINATION_EXIT_CODES = new Set([0xc000013a, 0x40010004]) const RECOVERABLE_CHILD_PROCESS_TYPES = new Set(['gpu']) -const RECOVERABLE_UTILITY_SERVICE_NAMES = new Set([ - 'audio.mojom.AudioService', - 'network.mojom.NetworkService', - // Why: Windows media/screen capture can churn this Chromium utility without - // taking down Orca; prompting users for those child exits is noise. - 'video_capture.mojom.VideoCaptureService' -]) const RECOVERABLE_CHILD_PROCESS_REASONS = new Set(['abnormal-exit', 'crashed', 'killed']) const NON_RECOVERABLE_RENDERER_REASONS = new Set(['integrity-failure']) @@ -41,11 +36,7 @@ function isRecoverableChromiumChildProcess({ if (normalizedProcessType && RECOVERABLE_CHILD_PROCESS_TYPES.has(normalizedProcessType)) { return true } - return ( - normalizedProcessType === 'utility' && - serviceName !== undefined && - RECOVERABLE_UTILITY_SERVICE_NAMES.has(serviceName) - ) + return normalizedProcessType === 'utility' && isRecoverableChromiumUtilityService(serviceName) } export function shouldRecordProcessGoneCrash({ @@ -63,7 +54,7 @@ export function shouldRecordProcessGoneCrash({ exitCode: number | null expectedTeardown: ExpectedTeardownScope }): boolean { - // Why: GPU, Network Service, and Audio Service exits are recoverable Chromium + // Why: GPU exits and on-demand Chromium utility-service exits are recoverable // child-process churn; treating them as app crashes creates noisy user prompts. if (isRecoverableChromiumChildProcess({ source, processType, serviceName, reason })) { return false diff --git a/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts b/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts index fffd36f1016..ebb8d90131f 100644 --- a/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts +++ b/src/main/crash-reporting/process-gone-killed-one-ordering.test.ts @@ -17,6 +17,7 @@ import { import { ProcessGoneDedupe } from './process-gone-dedupe' import { recordProcessGoneCrash, type ProcessGoneCrashEvent } from './process-gone-recorder' import { resetProcessGoneSiblingCorrelationForTest } from './process-gone-sibling-correlation' +import { resetSuppressedProcessGoneRingBudgetForTest } from './suppressed-process-gone-ring-budget' const noMinidump = async () => null const attachDetails = async () => null @@ -63,6 +64,7 @@ beforeEach(() => { now = 1_000 resetExpectedTeardownStateForTest(() => now) clearCrashBreadcrumbsForTest() + resetSuppressedProcessGoneRingBudgetForTest() resetProcessGoneSiblingCorrelationForTest() }) @@ -70,6 +72,7 @@ afterEach(() => { vi.restoreAllMocks() resetExpectedTeardownStateForTest() clearCrashBreadcrumbsForTest() + resetSuppressedProcessGoneRingBudgetForTest() resetProcessGoneSiblingCorrelationForTest() }) diff --git a/src/main/crash-reporting/process-gone-recorder.test.ts b/src/main/crash-reporting/process-gone-recorder.test.ts index 95dbd992401..4428b7a4ab6 100644 --- a/src/main/crash-reporting/process-gone-recorder.test.ts +++ b/src/main/crash-reporting/process-gone-recorder.test.ts @@ -20,6 +20,7 @@ import { ProcessGoneDedupe } from './process-gone-dedupe' import { recordProcessGoneCrash, type ProcessGoneCrashEvent } from './process-gone-recorder' import { resetProcessGoneSiblingCorrelationForTest } from './process-gone-sibling-correlation' import { _resetTracerForTests, setActiveSink, type TracerSink } from '../observability/tracer' +import { resetSuppressedProcessGoneRingBudgetForTest } from './suppressed-process-gone-ring-budget' type CapturingSink = TracerSink & { records: unknown[]; flushMock: ReturnType } @@ -57,6 +58,7 @@ beforeEach(() => { sink = capturingSink() setActiveSink(sink) clearCrashBreadcrumbsForTest() + resetSuppressedProcessGoneRingBudgetForTest() resetProcessGoneSiblingCorrelationForTest() }) @@ -65,6 +67,7 @@ afterEach(() => { vi.restoreAllMocks() _resetTracerForTests() clearCrashBreadcrumbsForTest() + resetSuppressedProcessGoneRingBudgetForTest() resetProcessGoneSiblingCorrelationForTest() }) @@ -595,7 +598,7 @@ describe('minidump signature attachment', () => { event({ source: 'child', processType: 'Utility', - // A utility outside the recoverable-service allowlist still reports. + // A utility Chromium churn does not own still reports. details: { type: 'Utility', serviceName: 'storage.mojom.StorageService' } }), new ProcessGoneDedupe(), diff --git a/src/main/crash-reporting/process-gone-recorder.ts b/src/main/crash-reporting/process-gone-recorder.ts index 47ac561c4a7..1a8c775380f 100644 --- a/src/main/crash-reporting/process-gone-recorder.ts +++ b/src/main/crash-reporting/process-gone-recorder.ts @@ -3,17 +3,13 @@ import { app } from 'electron' import { isCrashReportReason, sanitizeCrashReportDetails, - sanitizeCrashReportString, - type CrashReportBreadcrumbData + sanitizeCrashReportString } from '../../shared/crash-reporting' import { decodePosixWaitStatus, describePosixWaitStatus } from '../../shared/posix-wait-status' import { rendererCrashBreadcrumbOrigin } from '../../shared/crash-breadcrumb-origin' import type { CrashReportStore } from './crash-report-store' import { getCrashBreadcrumbSnapshot } from './crash-breadcrumb-store' -import { - recordCoalescedDurableCrashBreadcrumb, - recordDurableCrashBreadcrumb -} from './durable-crash-breadcrumb' +import { recordDurableCrashBreadcrumb } from './durable-crash-breadcrumb' import { correlateChildProcessDeath, trackRendererSiblingAttribution @@ -24,7 +20,10 @@ import { type ProcessGoneSource } from './process-gone-classification' import { buildProcessGoneCrashDetails } from './process-gone-diagnostics' -import { buildSuppressedProcessGoneBreadcrumbData } from './suppressed-process-gone-breadcrumb' +import { + buildSuppressedProcessGoneBreadcrumbData, + recordSuppressedProcessGone +} from './suppressed-process-gone-breadcrumb' import { getProcessGoneDedupeKey, processGoneDedupe, @@ -40,6 +39,7 @@ import { scheduleCrashpadDumpPrune, type CapturedMinidump } from './crashpad-capture' +import { crashpadProcessTypeFor } from './crashpad-process-type' import { minidumpSignatureDetails } from './minidump-crash-signature' import { flushActiveSink, startSpan } from '../observability/tracer' @@ -61,25 +61,13 @@ export type MinidumpCapture = ( expectedProcessType: string ) => Promise -const CHILD_CRASHPAD_PROCESS_TYPES: Readonly> = { - gpu: 'gpu-process', - utility: 'utility', - zygote: 'zygote' -} - function expectedCrashpadProcessType(event: ProcessGoneCrashEvent): string | null { - return event.source === 'renderer' - ? 'renderer' - : (CHILD_CRASHPAD_PROCESS_TYPES[event.processType.trim().toLowerCase()] ?? null) + return crashpadProcessTypeFor(event.source, event.processType) } const captureProcessMinidump: MinidumpCapture = (crashedAtMs, expectedProcessType) => captureMinidumpSignature(crashedAtMs, { expectedProcessType }) -// Why: the coalesce map prunes every key against the calling window, so a shorter -// one here would weaken the other 30s coalescers. Stay uniform with them. -const SUPPRESSED_PROCESS_GONE_COALESCE_MS = 30_000 - function processGoneBreadcrumbData(event: ProcessGoneCrashEvent) { return buildSuppressedProcessGoneBreadcrumbData(event) } @@ -90,21 +78,6 @@ function processGoneRendererOrigin(event: ProcessGoneCrashEvent): string | undef : rendererCrashBreadcrumbOrigin(event.webContentsId) } -// Why: key off the emitted breadcrumb, not the crash-report dedupe key, so two -// different recoverable services can never suppress each other's evidence. -function suppressedProcessGoneCoalesceKey(data: CrashReportBreadcrumbData): string { - return JSON.stringify([ - data.source, - data.processType, - data.reason, - data.exitCode, - data.expectedTeardown, - data.serviceName ?? null, - data.name ?? null, - data.type ?? null - ]) -} - // Why: POSIX exit codes arrive as raw wait statuses (61696 = exit 241); name the // meaning on the span so bundles read without manual decoding. Display-only — // the recorded exitCode stays raw. launch-failed codes are not wait statuses. @@ -202,20 +175,7 @@ export function recordProcessGoneCrash( expectedTeardown: event.expectedTeardown }) ) { - // Why: Chromium can crash-loop a recoverable child (network service seen at - // 1459/min) and each suppressed event costs a span plus a forced disk flush, - // which both floods the 30-entry ring and evicts the real pre-crash trail. - const suppressedData = processGoneBreadcrumbData(event) - const origin = processGoneRendererOrigin(event) - recordCoalescedDurableCrashBreadcrumb({ - name: 'process_gone_suppressed', - data: suppressedData, - coalesceKey: origin - ? `${origin}\u0000${suppressedProcessGoneCoalesceKey(suppressedData)}` - : suppressedProcessGoneCoalesceKey(suppressedData), - minIntervalMs: SUPPRESSED_PROCESS_GONE_COALESCE_MS, - ...(origin ? { origin } : {}) - }) + recordSuppressedProcessGone(event, goneAt, processGoneRendererOrigin(event)) return } if (!store) { diff --git a/src/main/crash-reporting/suppressed-process-gone-breadcrumb.ts b/src/main/crash-reporting/suppressed-process-gone-breadcrumb.ts index 97b6c1c72f0..ebe0bb73279 100644 --- a/src/main/crash-reporting/suppressed-process-gone-breadcrumb.ts +++ b/src/main/crash-reporting/suppressed-process-gone-breadcrumb.ts @@ -1,5 +1,28 @@ import type { CrashReportBreadcrumbData } from '../../shared/crash-reporting' +import { crashpadProcessTypeFor } from './crashpad-process-type' +import { noteSuppressedCrashDump } from './crashpad-capture' +import { recordCoalescedDurableCrashBreadcrumb } from './durable-crash-breadcrumb' import type { ExpectedTeardownScope } from './process-gone-classification' +import { + budgetedSuppressedCoalesceKey, + isSuppressedOverflowCoalesceKey +} from './suppressed-process-gone-ring-budget' + +type SuppressedProcessGone = { + source: 'renderer' | 'child' + processType: string + reason: string + exitCode: number | null + expectedTeardown: ExpectedTeardownScope + details: Record +} + +// Why: the coalesce map prunes every key against the calling window, so a shorter +// one here would weaken the other 30s coalescers. Stay uniform with them. +const SUPPRESSED_PROCESS_GONE_COALESCE_MS = 30_000 + +// Reasons where Crashpad has actually written a dump for the dead process. +const DUMP_PRODUCING_REASONS = new Set(['crashed', 'abnormal-exit', 'integrity-failure']) function safeString(value: unknown): string | undefined { return typeof value === 'string' && value.length > 0 ? value : undefined @@ -41,3 +64,55 @@ export function buildSuppressedProcessGoneBreadcrumbData({ } return breadcrumb } + +// Why: key off the emitted breadcrumb, not the crash-report dedupe key, so two +// different recoverable services can never suppress each other's evidence. +function suppressedProcessGoneCoalesceKey(data: CrashReportBreadcrumbData): string { + return JSON.stringify([ + data.source, + data.processType, + data.reason, + data.exitCode, + data.expectedTeardown, + data.serviceName ?? null, + data.name ?? null, + data.type ?? null + ]) +} + +/** + * Records a process exit we deliberately do not report. + * + * Why: Chromium can crash-loop a recoverable child (network service seen at + * 1459/min) and each suppressed event costs a span plus a forced disk flush, + * which both floods the 30-entry ring and evicts the real pre-crash trail. + */ +export function recordSuppressedProcessGone( + event: SuppressedProcessGone, + goneAtMs: number, + origin?: string +): void { + const data = buildSuppressedProcessGoneBreadcrumbData(event) + // Nothing reports this exit, so nothing claims the dump Crashpad wrote for + // it; fence it off before an unrelated report adopts its CHECK signature. + const dumpProcessType = crashpadProcessTypeFor(event.source, event.processType) + if (dumpProcessType && DUMP_PRODUCING_REASONS.has(event.reason)) { + noteSuppressedCrashDump(dumpProcessType, goneAtMs) + } + const coalesceKey = budgetedSuppressedCoalesceKey( + origin + ? `${origin}\u0000${suppressedProcessGoneCoalesceKey(data)}` + : suppressedProcessGoneCoalesceKey(data), + SUPPRESSED_PROCESS_GONE_COALESCE_MS + ) + // Flag the shared slot so a triager reads its count as "many services + // churned", not "this one service died N times". + const overflow = isSuppressedOverflowCoalesceKey(coalesceKey) + recordCoalescedDurableCrashBreadcrumb({ + name: 'process_gone_suppressed', + data: overflow ? { ...data, coalescedAcrossServices: true } : data, + coalesceKey, + minIntervalMs: SUPPRESSED_PROCESS_GONE_COALESCE_MS, + ...(origin && !overflow ? { origin } : {}) + }) +} diff --git a/src/main/crash-reporting/suppressed-process-gone-ring-budget.test.ts b/src/main/crash-reporting/suppressed-process-gone-ring-budget.test.ts new file mode 100644 index 00000000000..e4bc1ce63f9 --- /dev/null +++ b/src/main/crash-reporting/suppressed-process-gone-ring-budget.test.ts @@ -0,0 +1,87 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('electron', () => ({ + app: { getVersion: () => '1.2.3-test', getAppMetrics: () => [] } +})) + +import { + clearCrashBreadcrumbsForTest, + getCrashBreadcrumbSnapshot, + recordCrashBreadcrumb +} from './crash-breadcrumb-store' +import { ProcessGoneDedupe } from './process-gone-dedupe' +import { recordProcessGoneCrash, type ProcessGoneCrashEvent } from './process-gone-recorder' +import { resetProcessGoneSiblingCorrelationForTest } from './process-gone-sibling-correlation' +import { _resetTracerForTests, setActiveSink } from '../observability/tracer' +import { resetSuppressedProcessGoneRingBudgetForTest } from './suppressed-process-gone-ring-budget' + +// Chromium services that all suppress by default, so each one is its own coalesce +// key: an OOM sweep or shutdown race can burst the whole set inside one window. +const CHROMIUM_SERVICES = [ + 'proxy_resolver.mojom.ProxyResolverFactory', + 'printing.mojom.PrintCompositor', + 'data_decoder.mojom.DataDecoderService', + 'tracing.mojom.TracedProcess', + 'media.mojom.MediaFoundationService', + 'device.mojom.DeviceService', + 'shape_detection.mojom.ShapeDetectionService', + 'unzip.mojom.Unzipper', + 'patch.mojom.FilePatcher', + 'quarantine.mojom.Quarantine', + 'screen_ai.mojom.ScreenAIService', + 'speech_recognition.mojom.SpeechRecognitionService', + 'paint_preview.mojom.PaintPreviewCompositorCollection', + 'video_effects.mojom.VideoEffectsService', + 'file_util.mojom.SafeArchiveAnalyzer' +] + +function utilityCrash(serviceName: string, exitCode: number): ProcessGoneCrashEvent { + return { + source: 'child', + processType: 'Utility', + reason: 'crashed', + exitCode, + expectedTeardown: 'none', + details: { type: 'Utility', serviceName } + } +} + +beforeEach(() => { + setActiveSink({ push: vi.fn(), flush: vi.fn(), close: vi.fn() }) + clearCrashBreadcrumbsForTest() + resetSuppressedProcessGoneRingBudgetForTest() + resetProcessGoneSiblingCorrelationForTest() +}) + +afterEach(() => { + vi.restoreAllMocks() + _resetTracerForTests() + clearCrashBreadcrumbsForTest() + resetSuppressedProcessGoneRingBudgetForTest() + resetProcessGoneSiblingCorrelationForTest() +}) + +describe('suppressed process-gone breadcrumb ring budget', () => { + it('leaves the pre-crash trail intact when a whole service population churns', () => { + const dedupe = new ProcessGoneDedupe() + for (let index = 0; index < 20; index += 1) { + recordCrashBreadcrumb('user_action', { step: index }) + } + + for (const serviceName of CHROMIUM_SERVICES) { + for (const exitCode of [139, 11]) { + recordProcessGoneCrash( + { record: vi.fn() } as never, + utilityCrash(serviceName, exitCode), + dedupe + ) + } + } + + const names = getCrashBreadcrumbSnapshot().map((breadcrumb) => breadcrumb.name) + expect({ + userAction: names.filter((name) => name === 'user_action').length, + suppressed: names.filter((name) => name === 'process_gone_suppressed').length + }).toEqual({ userAction: 20, suppressed: 6 }) + }) +}) diff --git a/src/main/crash-reporting/suppressed-process-gone-ring-budget.ts b/src/main/crash-reporting/suppressed-process-gone-ring-budget.ts new file mode 100644 index 00000000000..47ba6ec54c4 --- /dev/null +++ b/src/main/crash-reporting/suppressed-process-gone-ring-budget.ts @@ -0,0 +1,49 @@ +// Coalescing bounds a *repeating* suppressed exit to one breadcrumb, but it is +// per-key: once every Chromium utility service suppresses by default, one OOM +// sweep or shutdown race churns a whole population of distinct services and each +// one claims its own ring slot. Thirty such exits fill the 30-entry ring and +// evict the entire pre-crash trail -- the exact blind spot coalescing exists to +// prevent. So cap the slots suppressed churn may hold and fold the rest into a +// single overflow slot, which keeps counting them via `suppressedSinceLast`. + +// A fifth of the 30-entry ring; the last of the six is the overflow slot. +const SUPPRESSED_RING_SLOT_BUDGET = 6 +// Leading space: coalesce keys are JSON arrays, so this can never collide. +const OVERFLOW_COALESCE_KEY = ' process-gone-suppressed-overflow' + +// Why monotonic: wall-clock corrections must not stretch or collapse the window. +const monotonicNow = (): number => performance.now() + +let windowStartedAtMs = Number.NEGATIVE_INFINITY +let slotKeys = new Set() + +/** + * Maps a suppressed process-gone coalesce key onto the key it may actually use. + * Returns the key unchanged while the budget has room, and the shared overflow + * key once it does not, so a service-population burst costs a bounded number of + * ring slots instead of one per service. + */ +export function budgetedSuppressedCoalesceKey(coalesceKey: string, windowMs: number): string { + const now = monotonicNow() + if (now - windowStartedAtMs >= windowMs) { + windowStartedAtMs = now + slotKeys = new Set() + } + if (slotKeys.has(coalesceKey)) { + return coalesceKey + } + if (slotKeys.size < SUPPRESSED_RING_SLOT_BUDGET - 1) { + slotKeys.add(coalesceKey) + return coalesceKey + } + return OVERFLOW_COALESCE_KEY +} + +export function isSuppressedOverflowCoalesceKey(coalesceKey: string): boolean { + return coalesceKey === OVERFLOW_COALESCE_KEY +} + +export function resetSuppressedProcessGoneRingBudgetForTest(): void { + windowStartedAtMs = Number.NEGATIVE_INFINITY + slotKeys = new Set() +} diff --git a/src/main/crash-reporting/suppressed-utility-crash-diagnosability.test.ts b/src/main/crash-reporting/suppressed-utility-crash-diagnosability.test.ts new file mode 100644 index 00000000000..15c37332cd3 --- /dev/null +++ b/src/main/crash-reporting/suppressed-utility-crash-diagnosability.test.ts @@ -0,0 +1,98 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +vi.mock('electron', () => ({ + app: { getVersion: () => '1.2.3-test', getAppMetrics: () => [] } +})) + +import { clearCrashBreadcrumbsForTest, getCrashBreadcrumbSnapshot } from './crash-breadcrumb-store' +import { ProcessGoneDedupe } from './process-gone-dedupe' +import { recordProcessGoneCrash, type ProcessGoneCrashEvent } from './process-gone-recorder' +import { resetProcessGoneSiblingCorrelationForTest } from './process-gone-sibling-correlation' +import { _resetTracerForTests, setActiveSink, type TracerSink } from '../observability/tracer' +import { resetSuppressedProcessGoneRingBudgetForTest } from './suppressed-process-gone-ring-budget' + +const records: unknown[] = [] +const sink: TracerSink = { + push: (record) => records.push(record), + flush: vi.fn(), + close: vi.fn() +} + +/** A repeatable CHECK failure inside an on-demand Chromium utility service. */ +function printCompositorCheckFailure(): ProcessGoneCrashEvent { + return { + source: 'child', + processType: 'utility', + reason: 'crashed', + exitCode: 0x80000003, + expectedTeardown: 'none', + details: { processType: 'utility', serviceName: 'printing.mojom.PrintCompositor' } + } +} + +function suppress(capture: unknown): { record: ReturnType } { + const store = { record: vi.fn(async () => 'report-1'), attachDetails: vi.fn(async () => null) } + recordProcessGoneCrash( + store as never, + printCompositorCheckFailure(), + new ProcessGoneDedupe(), + capture as never + ) + return store +} + +beforeEach(() => { + records.length = 0 + setActiveSink(sink) + clearCrashBreadcrumbsForTest() + resetSuppressedProcessGoneRingBudgetForTest() + resetProcessGoneSiblingCorrelationForTest() +}) + +afterEach(() => { + vi.restoreAllMocks() + _resetTracerForTests() + clearCrashBreadcrumbsForTest() + resetSuppressedProcessGoneRingBudgetForTest() + resetProcessGoneSiblingCorrelationForTest() +}) + +// Pins the contract stated on isRecoverableChromiumUtilityService: suppression is +// breadcrumb-only, so widening the denylist is the ONLY way to keep a service's +// post-mortem detail. A suppressed exit must never pay for a minidump poll either +// -- Chromium can crash-loop these at 1459/min. +describe('suppressed Chromium utility crash diagnosability', () => { + it('never reaches the minidump signature path', async () => { + const capture = vi.fn(async () => null) + + const store = suppress(capture) + await Promise.resolve() + + expect(store.record).not.toHaveBeenCalled() + expect(capture).not.toHaveBeenCalled() + expect(records).not.toContainEqual( + expect.objectContaining({ name: 'electron.minidump_signature' }) + ) + }) + + it('leaves the CHECK anonymous in the durable breadcrumb trail', () => { + suppress(async () => null) + + const [breadcrumb] = getCrashBreadcrumbSnapshot() + expect(breadcrumb).toEqual( + expect.objectContaining({ + name: 'process_gone_suppressed', + data: expect.objectContaining({ + source: 'child', + processType: 'utility', + serviceName: 'printing.mojom.PrintCompositor', + reason: 'crashed', + exitCode: 0x80000003 + }) + }) + ) + expect(Object.keys(breadcrumb.data ?? {})).toEqual( + expect.not.arrayContaining(['minidumpStatus', 'minidumpPath', 'crashSignature']) + ) + }) +}) diff --git a/src/main/crash-reporting/suppressed-utility-dump-attribution.test.ts b/src/main/crash-reporting/suppressed-utility-dump-attribution.test.ts new file mode 100644 index 00000000000..b99259d0d0d --- /dev/null +++ b/src/main/crash-reporting/suppressed-utility-dump-attribution.test.ts @@ -0,0 +1,142 @@ +import { mkdtemp, mkdir, rm, utimes, writeFile } from 'node:fs/promises' +import os from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' + +const parseMinidumpCrashSignatureMock = vi.hoisted(() => vi.fn()) + +vi.mock('electron', () => ({ + app: { + getVersion: () => '1.2.3-test', + getAppMetrics: () => [], + getPath: () => '/unused-in-tests' + }, + crashReporter: { start: vi.fn() } +})) +vi.mock('./minidump-crash-signature', async (importOriginal) => ({ + ...(await importOriginal>()), + parseMinidumpCrashSignature: parseMinidumpCrashSignatureMock +})) + +import { _setCrashpadCaptureStateForTest, captureMinidumpSignature } from './crashpad-capture' +import { clearCrashBreadcrumbsForTest } from './crash-breadcrumb-store' +import { ProcessGoneDedupe } from './process-gone-dedupe' +import { recordProcessGoneCrash, type ProcessGoneCrashEvent } from './process-gone-recorder' +import { resetProcessGoneSiblingCorrelationForTest } from './process-gone-sibling-correlation' +import { _resetTracerForTests, setActiveSink } from '../observability/tracer' +import { resetSuppressedProcessGoneRingBudgetForTest } from './suppressed-process-gone-ring-budget' + +let dumpDir: string + +/** Minimal but valid minidump header; the signature parser is mocked. */ +function emptyDump(): Buffer { + const buf = Buffer.alloc(32) + buf.writeUInt32LE(0x504d444d, 0) + buf.writeUInt32LE(0xa793, 4) + buf.writeUInt32LE(0, 8) + buf.writeUInt32LE(32, 12) + return buf +} + +async function writeDump(relativePath: string, mtimeMs: number): Promise { + const filePath = path.join(dumpDir, relativePath) + await mkdir(path.dirname(filePath), { recursive: true }) + await writeFile(filePath, emptyDump()) + await utimes(filePath, mtimeMs / 1000, mtimeMs / 1000) + return filePath +} + +/** Suppressed by the Chromium-utility default; leaves an unreported dump behind. */ +function printCompositorCheckFailure(): ProcessGoneCrashEvent { + return { + source: 'child', + processType: 'utility', + reason: 'crashed', + exitCode: 0x80000003, + expectedTeardown: 'none', + details: { type: 'Utility', serviceName: 'printing.mojom.PrintCompositor' } + } +} + +/** Reportable: `launch-failed` is not recoverable churn, and it never ran, so it + * has no dump of its own to pair with. */ +function videoCaptureLaunchFailure(): ProcessGoneCrashEvent { + return { + source: 'child', + processType: 'utility', + reason: 'launch-failed', + exitCode: null, + expectedTeardown: 'none', + details: { type: 'Utility', serviceName: 'video_capture.mojom.VideoCaptureService' } + } +} + +function recorderStore() { + return { + record: vi.fn(async () => ({ id: 'report-1' })), + attachDetails: vi.fn(async () => null) + } +} + +// Bounded poll: the recorder's real capture would spin for 8s when no dump matches. +const capture = (crashedAtMs: number, expectedProcessType: string) => + captureMinidumpSignature(crashedAtMs, { expectedProcessType, timeoutMs: 0 }) + +beforeEach(async () => { + parseMinidumpCrashSignatureMock.mockReset() + parseMinidumpCrashSignatureMock.mockReturnValue({ + processType: 'utility', + checkMessage: '[0125/000000.000:FATAL:print_compositor.cc(42)] Check failed: alive.', + checkFile: 'print_compositor.cc', + checkLine: 42, + annotations: {} + }) + dumpDir = await mkdtemp(path.join(os.tmpdir(), 'orca-suppressed-dump-')) + _setCrashpadCaptureStateForTest({ dumpDirectory: dumpDir, started: true }) + setActiveSink({ push: vi.fn(), flush: vi.fn(), close: vi.fn() }) + clearCrashBreadcrumbsForTest() + resetSuppressedProcessGoneRingBudgetForTest() + resetProcessGoneSiblingCorrelationForTest() +}) + +afterEach(async () => { + _setCrashpadCaptureStateForTest(null) + vi.restoreAllMocks() + _resetTracerForTests() + clearCrashBreadcrumbsForTest() + resetSuppressedProcessGoneRingBudgetForTest() + resetProcessGoneSiblingCorrelationForTest() + await rm(dumpDir, { recursive: true, force: true }) +}) + +describe('minidump attribution after a suppressed utility crash', () => { + it('never hands a suppressed service dump to an unrelated utility report', async () => { + const store = recorderStore() + const dedupe = new ProcessGoneDedupe() + + recordProcessGoneCrash(store as never, printCompositorCheckFailure(), dedupe, capture) + await writeDump(path.join('reports', 'print-compositor.dmp'), Date.now() + 100) + recordProcessGoneCrash(store as never, videoCaptureLaunchFailure(), dedupe, capture) + + await vi.waitFor(() => expect(store.attachDetails).toHaveBeenCalled()) + expect(store.attachDetails).toHaveBeenCalledWith('report-1', { minidumpStatus: 'absent' }) + }) + + it('still pairs a reportable utility crash with its own dump', async () => { + const store = recorderStore() + + await writeDump(path.join('reports', 'video-capture.dmp'), Date.now() + 100) + recordProcessGoneCrash( + store as never, + videoCaptureLaunchFailure(), + new ProcessGoneDedupe(), + capture + ) + + await vi.waitFor(() => expect(store.attachDetails).toHaveBeenCalled()) + expect(store.attachDetails).toHaveBeenCalledWith( + 'report-1', + expect.objectContaining({ minidumpStatus: 'captured' }) + ) + }) +})