diff --git a/src/main/crash-reporting/crashpad-capture.test.ts b/src/main/crash-reporting/crashpad-capture.test.ts index 265823185b9..093dc092cf6 100644 --- a/src/main/crash-reporting/crashpad-capture.test.ts +++ b/src/main/crash-reporting/crashpad-capture.test.ts @@ -261,4 +261,32 @@ describe('Crashpad dump pruning', () => { expect((await readdir(path.join(dumpDir, 'reports'))).sort()).toEqual(['middle.dmp', 'new.dmp']) }) + + it('caps the dump count even when every dump fits the byte budget', async () => { + await writeDump(path.join('reports', 'old.dmp'), CRASHED_AT, Buffer.alloc(8)) + await writeDump(path.join('reports', 'middle.dmp'), CRASHED_AT + 100, Buffer.alloc(8)) + await writeDump(path.join('reports', 'new.dmp'), CRASHED_AT + 200, Buffer.alloc(8)) + + await _pruneCrashpadDumpsForTest(1024, 2) + + expect((await readdir(path.join(dumpDir, 'reports'))).sort()).toEqual(['middle.dmp', 'new.dmp']) + }) + + it('keeps a dump already claimed by a persisted crash report', async () => { + await writeDump(path.join('reports', 'claimed.dmp'), CRASHED_AT + 200, Buffer.alloc(8)) + const captured = await captureMinidumpSignature(CRASHED_AT, { + timeoutMs: 0, + now: () => CRASHED_AT + }) + expect(captured?.filePath).toBe(path.join(dumpDir, 'reports', 'claimed.dmp')) + // Newer than the claimed dump, so the claim is what protects it, not index 0. + await writeDump(path.join('reports', 'newest.dmp'), CRASHED_AT + 400, Buffer.alloc(8)) + + await _pruneCrashpadDumpsForTest(8) + + expect((await readdir(path.join(dumpDir, 'reports'))).sort()).toEqual([ + 'claimed.dmp', + 'newest.dmp' + ]) + }) }) diff --git a/src/main/crash-reporting/crashpad-capture.ts b/src/main/crash-reporting/crashpad-capture.ts index 56126999f68..b0935cd81d3 100644 --- a/src/main/crash-reporting/crashpad-capture.ts +++ b/src/main/crash-reporting/crashpad-capture.ts @@ -27,6 +27,9 @@ const MAX_DUMP_BYTES = 64 * 1024 * 1024 // Match Crashpad's default budget, but enforce it after crashes instead of // waiting for its first 10-minute and later daily pruning passes. const MAX_STORED_DUMP_BYTES = 128 * 1024 * 1024 +// A burst of small dumps stays under the byte budget while still growing the +// directory walk, so cap the file count too. +const MAX_STORED_DUMPS = 64 const DUMP_PRUNE_DELAY_MS = 2_000 type DumpCandidate = { @@ -74,6 +77,14 @@ export function startCrashpadCapture(options: CrashpadCaptureOptions = {}): bool return false } crashpadDumpDirectory = options.dumpDirectory ?? resolveDumpDirectory() + // Why: a dying main process never delivers process-gone, so a crash loop + // never reaches the post-crash prune, and Crashpad's own pass runs in the + // handler child after a delayed first sweep. Pruning here is the only thing + // that bounds disk across repeatedly crashed launches, so it must not be + // deferred behind the coalescing timer a crash loop outruns. + void pruneCrashpadDumps().catch((error) => { + console.error('[crash-reporting] Crashpad startup dump pruning failed:', error) + }) return true } @@ -131,7 +142,10 @@ async function collectDumpCandidates(directory: string): Promise { +async function pruneCrashpadDumps( + maxBytes = MAX_STORED_DUMP_BYTES, + maxDumps = MAX_STORED_DUMPS +): Promise { const directory = crashpadDumpDirectory if (!directory) { return @@ -140,11 +154,18 @@ async function pruneCrashpadDumps(maxBytes = MAX_STORED_DUMP_BYTES): Promise right.mtimeMs - left.mtimeMs ) let retainedBytes = 0 + let retainedCount = 0 for (let index = 0; index < candidates.length; index += 1) { const candidate = candidates[index] - const mustKeep = index === 0 || reservedDumpPaths.has(candidate.filePath) - if (mustKeep || retainedBytes + candidate.size <= maxBytes) { + // claimed dumps are referenced by a persisted report; pruning one leaves a + // dangling minidumpPath behind. + const mustKeep = + index === 0 || + reservedDumpPaths.has(candidate.filePath) || + claimedDumpPaths.has(candidate.filePath) + if (mustKeep || (retainedBytes + candidate.size <= maxBytes && retainedCount < maxDumps)) { retainedBytes += candidate.size + retainedCount += 1 continue } try { @@ -172,9 +193,12 @@ export function scheduleCrashpadDumpPrune(): void { dumpPruneTimer.unref() } -/** Test seam for byte-budget behavior without a real Crashpad database. */ -export async function _pruneCrashpadDumpsForTest(maxBytes: number): Promise { - await pruneCrashpadDumps(maxBytes) +/** Test seam for byte/count-budget behavior without a real Crashpad database. */ +export async function _pruneCrashpadDumpsForTest( + maxBytes: number, + maxDumps = MAX_STORED_DUMPS +): Promise { + await pruneCrashpadDumps(maxBytes, maxDumps) } type DumpPollingOptions = { @@ -267,7 +291,9 @@ export async function captureMinidumpSignature( } reservedDumpPaths.add(dump.filePath) try { - const signature = parseMinidumpCrashSignature(await readFile(dump.filePath)) + const signature = parseMinidumpCrashSignature(await readFile(dump.filePath), { + expectedProcessType: options.expectedProcessType + }) if ( !signature || (options.expectedProcessType !== undefined && diff --git a/src/main/crash-reporting/minidump-crash-signature.test.ts b/src/main/crash-reporting/minidump-crash-signature.test.ts index e3fc0eb54df..2d1fbb90972 100644 --- a/src/main/crash-reporting/minidump-crash-signature.test.ts +++ b/src/main/crash-reporting/minidump-crash-signature.test.ts @@ -229,6 +229,45 @@ describe('parseMinidumpCrashSignature', () => { expect(signature?.processType).toBe('renderer') }) + it('stops at the process type when the dump belongs to another process', () => { + const { dump } = buildDump({ annotations: { ptype: 'gpu-process' } }) + const dumpWithMemory = Buffer.concat([ + dump, + Buffer.from(`\0${ELECTRON_43_CHECK_LINE}\0`, 'utf8') + ]) + + const signature = parseMinidumpCrashSignature(dumpWithMemory, { + expectedProcessType: 'renderer' + }) + + expect(signature?.processType).toBe('gpu-process') + // The whole-buffer scan is skipped; the caller discards this dump anyway. + expect(signature?.checkMessage).toBeUndefined() + }) + + it('still parses fully when the process type matches', () => { + const { dump } = buildDump({ annotations: { ptype: 'renderer' } }) + const dumpWithMemory = Buffer.concat([ + dump, + Buffer.from(`\0${ELECTRON_43_CHECK_LINE}\0`, 'utf8') + ]) + + const signature = parseMinidumpCrashSignature(dumpWithMemory, { + expectedProcessType: 'renderer' + }) + + expect(signature?.checkMessage).toBe(ELECTRON_43_CHECK_LINE) + }) + + it('ignores a log prefix further back than the prefix limit', () => { + const { dump } = buildDump({ annotations: { ptype: 'renderer' } }) + // `[` separated from the marker by more than MAX_LOG_PREFIX_BYTES (96). + const farPrefix = `[${'x'.repeat(200)}:FATAL:render_frame_impl.cc(4821)] Check failed: far.` + const dumpWithMemory = Buffer.concat([dump, Buffer.from(`\0${farPrefix}\0`, 'utf8')]) + + expect(parseMinidumpCrashSignature(dumpWithMemory)?.checkMessage).toBeUndefined() + }) + it('does not promote an unrelated Chromium ERROR line containing CHECK', () => { const { dump } = buildDump({}) const unrelated = diff --git a/src/main/crash-reporting/minidump-crash-signature.ts b/src/main/crash-reporting/minidump-crash-signature.ts index 4104f21f6d9..3e246d4b77a 100644 --- a/src/main/crash-reporting/minidump-crash-signature.ts +++ b/src/main/crash-reporting/minidump-crash-signature.ts @@ -107,6 +107,22 @@ function isPrintableLogByte(value: number): boolean { return value === 0x09 || (value >= 0x20 && value <= 0x7e) } +/** + * `lastIndexOf(byte, from)` restricted to `within` bytes before `from`. An + * unbounded search scans the whole dump backward on a miss only for the result + * to be thrown away by the same prefix limit; zero-filled regions are normal in + * a minidump, so that miss is the common case, not the adversarial one. + */ +function lastIndexOfWithin(dump: Buffer, byte: number, from: number, within: number): number { + const floor = Math.max(0, from - within) + for (let at = from; at >= floor; at -= 1) { + if (dump[at] === byte) { + return at + } + } + return -1 +} + /** Electron 43 omits LOG_FATAL but keeps Chromium's formatted log line in memory. */ function findEmbeddedCheckMessage(dump: Buffer): LocatedCheckMessage | undefined { for (const marker of CHROMIUM_LOG_MARKERS) { @@ -117,8 +133,8 @@ function findEmbeddedCheckMessage(dump: Buffer): LocatedCheckMessage | undefined break } from = markerAt + marker.length - const start = dump.lastIndexOf(0x5b, markerAt) - if (start === -1 || markerAt - start > MAX_LOG_PREFIX_BYTES) { + const start = lastIndexOfWithin(dump, 0x5b, markerAt, MAX_LOG_PREFIX_BYTES) + if (start === -1) { continue } let end = markerAt + marker.length @@ -170,11 +186,24 @@ function findFaultingModule( return undefined } +export type MinidumpParseOptions = { + /** + * Process type the caller will accept. A dump from any other process is + * discarded by the caller anyway, so parsing stops at `processType` and the + * returned signature is deliberately partial — read only `processType` when + * it does not match. + */ + readonly expectedProcessType?: string +} + /** * Parses a Crashpad minidump into the fields that make a CHECK failure * nameable. Returns null when the buffer is not a minidump. */ -export function parseMinidumpCrashSignature(dump: Buffer): MinidumpCrashSignature | null { +export function parseMinidumpCrashSignature( + dump: Buffer, + options: MinidumpParseOptions = {} +): MinidumpCrashSignature | null { if (!isMinidump(dump)) { return null } @@ -185,6 +214,16 @@ export function parseMinidumpCrashSignature(dump: Buffer): MinidumpCrashSignatur -readonly [K in keyof MinidumpCrashSignature]: MinidumpCrashSignature[K] } = { annotations } + const processType = annotations['ptype'] + if (processType) { + signature.processType = processType + } + // Annotations are bounded; the scans below are not. A renderer crash would + // otherwise scan every fresh GPU/utility dump end to end before rejecting it. + if (options.expectedProcessType !== undefined && processType !== options.expectedProcessType) { + return signature + } + const annotatedCheckMessage = annotations['LOG_FATAL'] ?? annotations['abort-message'] const embeddedCheck = annotatedCheckMessage ? undefined : findEmbeddedCheckMessage(dump) const checkMessage = annotatedCheckMessage ?? embeddedCheck?.message @@ -198,10 +237,6 @@ export function parseMinidumpCrashSignature(dump: Buffer): MinidumpCrashSignatur signature.checkLine = location.line } } - if (annotations['ptype']) { - signature.processType = annotations['ptype'] - } - const exception = findStream(view, STREAM_TYPE_EXCEPTION) if (exception) { const code = view.u32(exception.rva + EXCEPTION_CODE_OFFSET)