From 08f930a6bda6ebfd028ba4437fcb10966d31adcd Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Thu, 3 Sep 2026 03:27:48 -0700 Subject: [PATCH] fix(windows): keep the safe-graphics marker while an install-DACL repair is in flight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate dispatches a repair without arming the probe clock, so waitForInstallDirAclVerdict() returns immediately and the withdrawal deleted the marker inside Chromium's FATAL window (crash 6 lands ~1.3s after crash 3, well inside the 20s gate). The process then died mid-repair, spent no attempt, and relaunched hardware accelerated into the same gate — spawning the same GPU children, FATALing again, forever. Hold the marker while poison.stage is 'pending' so that launch comes back software rendered and the next gate runs to completion. Still not engaged this launch, so --in-process-gpu does not erase the sibling-death evidence. A terminal verdict has no next step to rescue, so it still withdraws. Both new tests are RED without the retention. --- ...pu-lifecycle-install-dir-acl-guard.test.ts | 47 ++++++++++++++++++- src/main/startup/gpu-lifecycle.ts | 16 ++++++- .../windows-install-dir-acl-recovery.ts | 13 +++++ 3 files changed, 73 insertions(+), 3 deletions(-) diff --git a/src/main/startup/gpu-lifecycle-install-dir-acl-guard.test.ts b/src/main/startup/gpu-lifecycle-install-dir-acl-guard.test.ts index d9d891f0a21..95fea9c326a 100644 --- a/src/main/startup/gpu-lifecycle-install-dir-acl-guard.test.ts +++ b/src/main/startup/gpu-lifecycle-install-dir-acl-guard.test.ts @@ -42,6 +42,7 @@ import { readGpuFallbackMarker } from './gpu-fallback-marker' import { handleGpuChildCrash } from './gpu-lifecycle' import { mainProcessState as state } from './main-process-state' import { + isInstallDirAclRepairPending, noteWindowsInstallDirAclProbePending, resetWindowsInstallDirAclRecoveryForTest, startWindowsInstallDirAclRepairIfPoisoned @@ -79,6 +80,26 @@ function reportProbePoisoned(): void { ) } +/** A repair that reports a terminal failure, so `poison.stage` leaves 'pending'. */ +async function reportProbePoisonedWithFailedRepair(): Promise { + startWindowsInstallDirAclRepairIfPoisoned( + { status: 'ok', matchesPoisonSignature: true, wellKnownNameCheckReliable: true }, + { + ...recoveryOptions(), + runProcessFn: (async () => ({ + code: 1, + signal: null, + stdout: '', + stderr: 'access denied', + timedOut: false + })) as unknown as (spec: ProcessSpec) => Promise + } + ) + for (let i = 0; i < 200 && isInstallDirAclRepairPending(); i += 1) { + await new Promise((resolve) => setTimeout(resolve, 5)) + } +} + function reportProbeClean(): void { startWindowsInstallDirAclRepairIfPoisoned( { status: 'ok', matchesPoisonSignature: false }, @@ -171,7 +192,8 @@ describe('handleGpuChildCrash vs the install-dir ACL verdict', () => { expect(readGpuFallbackMarker(userData.path)?.userConfirmed).toBe(false) reportProbePoisoned() await decisive - expect(readGpuFallbackMarker(userData.path)).toBeNull() + // The verdict dispatched a repair, so the marker stays for the launch that repair rescues. + expect(readGpuFallbackMarker(userData.path)?.userConfirmed).toBe(false) }) it('engages immediately once the probe has already reported the install clean', async () => { @@ -182,6 +204,29 @@ describe('handleGpuChildCrash vs the install-dir ACL verdict', () => { expect(showMessageBox).toHaveBeenCalledTimes(1) }) + // Both from the re-run adversarial round. The gate dispatches a repair without arming the + // probe clock, so `waitForInstallDirAclVerdict` returns immediately and the withdrawal used + // to delete the marker inside Chromium's ~1.3s FATAL window — leaving the machine to + // relaunch hardware accelerated into the same 20s gate, forever. + it('keeps the safe-graphics marker on disk while a repair is still in flight', async () => { + reportProbePoisoned() + await crashUpToThreshold() + await handleGpuChildCrash('crashed', null, 600) + + expect(showMessageBox).not.toHaveBeenCalled() + expect(readGpuFallbackMarker(userData.path)?.userConfirmed).toBe(false) + }) + + it('still withdraws the marker once the verdict is terminal rather than a pending repair', async () => { + await reportProbePoisonedWithFailedRepair() + await crashUpToThreshold() + await handleGpuChildCrash('crashed', null, 600) + + expect(showMessageBox).not.toHaveBeenCalled() + // No repair is in flight to rescue a later launch, so the marker is not held. + expect(readGpuFallbackMarker(userData.path)).toBeNull() + }) + // recordGpuCrash reports the threshold crossing once and latches. Withholding consumes // that one report, so without a re-arm the same process could never engage again — a // machine whose tree is repaired and whose driver is genuinely broken would be stuck diff --git a/src/main/startup/gpu-lifecycle.ts b/src/main/startup/gpu-lifecycle.ts index 53051e24079..5629b063f12 100644 --- a/src/main/startup/gpu-lifecycle.ts +++ b/src/main/startup/gpu-lifecycle.ts @@ -17,6 +17,7 @@ import { engageGpuFallbackAfterCrashBurst } from '../crash-reporting/gpu-fallbac import { recordCrashBreadcrumb } from '../crash-reporting/crash-breadcrumb-store' import { recordDurableCrashBreadcrumb } from '../crash-reporting/durable-crash-breadcrumb' import { + isInstallDirAclRepairPending, isInstallDirAclSuspect, waitForInstallDirAclVerdict } from './windows-install-dir-acl-recovery' @@ -144,14 +145,25 @@ async function installDirAclClearsGpuFallback( if (!isInstallDirAclSuspect()) { return true } - if (persisted) { + // Why the marker survives a pending repair: withdrawing it here left a machine that + // Chromium FATALs mid-repair (crash 6 lands ~1.3s after crash 3, well inside the gate) + // relaunching hardware accelerated into the same 20s gate, spawning the same GPU children, + // FATALing again — with no attempt spent, so the loop never advances. Keeping it costs a + // healthy machine nothing: a successful repair clears an unconfirmed marker itself, and a + // clean probe reading means we never reach here. It is still not *engaged* this launch, so + // --in-process-gpu does not erase the sibling-death evidence on the launch that is running. + const repairPending = isInstallDirAclRepairPending() + if (persisted && !repairPending) { clearGpuFallbackMarker(userDataPath) } // Why re-arm: recordGpuCrash reports the threshold crossing once and latches. Withholding // consumed that one report, so without this a later burst — including one after the repair // succeeds and the tree is no longer the suspect — could never engage safe graphics again. state.gpuCrashFallbackTracker.disengage() - recordDurableCrashBreadcrumb('gpu_fallback_withheld_install_dir_acl', { crashesInWindow }) + recordDurableCrashBreadcrumb('gpu_fallback_withheld_install_dir_acl', { + crashesInWindow, + markerHeldForPendingRepair: repairPending + }) return false } diff --git a/src/main/startup/windows-install-dir-acl-recovery.ts b/src/main/startup/windows-install-dir-acl-recovery.ts index 07e4508910a..12ea04802e4 100644 --- a/src/main/startup/windows-install-dir-acl-recovery.ts +++ b/src/main/startup/windows-install-dir-acl-recovery.ts @@ -102,6 +102,19 @@ export function isInstallDirAclSuspect(now: number = Date.now()): boolean { return probePendingSince !== null && now - probePendingSince < PROBE_VERDICT_GRACE_MS } +/** + * True while a repair for this tree is dispatched and has not reported yet. + * + * Why it is not the same question as `isInstallDirAclSuspect`: a suspect tree we are + * actively repairing is one a *future* launch can still be rescued on, so the safe-graphics + * marker earns its keep there — a launch Chromium FATALs mid-repair comes back software + * rendered, stops spawning the GPU children that trigger the FATAL, and lets the next gate + * run to completion. A terminal verdict has no such next step. + */ +export function isInstallDirAclRepairPending(): boolean { + return poison?.stage === 'pending' +} + /** True while the pre-window gate is rewriting the very files a new renderer would load. */ export function isBlockingInstallDirAclRepairInFlight(): boolean { return blockingRepairInFlight