[PowerDisplay] Fix false-positive crash detection on cooperative shutdown (#48173)
<!-- Enter a brief description/summary of your PR here. What does it fix/what does it change/how was it tested (even manually, if necessary)? --> ## Summary of the Pull Request Cooperative shutdowns of `PowerDisplay.exe` — Runner's `TerminateApp` NamedPipe message, the `Terminate` named event, tray-quit, Runner-exit detection, and PowerToys upgrades — all call `Environment.Exit(0)` immediately. If DDC/CI discovery is mid-flight, that path skips the `try/finally` that owns `CrashDetectionScope`, leaving `discovery.lock` on disk. Phase 0 at the next `PowerDisplay.exe` startup then treats this orphan as evidence of a real crash and auto-disables the module, surfacing the "PowerDisplay has crashed" InfoBar in Settings UI. This PR adds an `AppDomain.ProcessExit` safety-net inside `CrashDetectionScope`. ProcessExit fires for `Environment.Exit` but **not** for `FailFast` / BSOD / external `TerminateProcess` — exactly the partition we need: cooperative exit → best-effort delete the lock; involuntary kill → leave the lock for Phase 0 to detect (original design intent preserved). <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [x] Closes: #48169 - [x] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [x] **Tests:** Added/updated and all pass - [x] **Localization:** All end-user-facing strings can be localized <!-- no user-facing strings changed --> - [x] **Dev docs:** Added/updated <!-- inline XML doc on CrashDetectionScope explains the ProcessExit partition --> - [ ] **New binaries:** Added on the required places - [ ] [JSON for signing](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ESRPSigning_core.json) for new binaries - [ ] [WXS for installer](https://github.com/microsoft/PowerToys/blob/main/installer/PowerToysSetup/Product.wxs) for new binaries and localization folder - [ ] [YML for CI pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/ci/templates/build-powertoys-steps.yml) for new test projects - [ ] [YML for signed pipeline](https://github.com/microsoft/PowerToys/blob/main/.pipelines/release.yml) - [ ] **Documentation updated:** If checked, please file a pull request on [our docs repo](https://github.com/MicrosoftDocs/windows-uwp/tree/docs/hub/powertoys) and link it here: #xxx <!-- Provide a more detailed description of the PR, other things fixed, or any additional comments/features here --> ## Detailed Description of the Pull Request / Additional comments ### Root cause `CrashDetectionScope.Begin()` writes `discovery.lock` before DDC/CI capability fetch and `Dispose()` deletes it when the `using` block exits. The lock is intentionally designed to survive any code path that cannot run user-mode cleanup (BSOD, kernel OOM, `TerminateProcess`), so that the next `PowerDisplay.exe` start can see it and run Phase 0 (write `crash_detected.flag`, set `enabled.PowerDisplay=false` in global `settings.json`, signal `AutoDisablePowerDisplayEvent`). The bug is that several **cooperative** shutdown paths route to `Environment.Exit(0)` immediately: | Path | Code | |---|---| | Runner's `TerminateApp` NamedPipe | `App.xaml.cs::OnNamedPipeMessage` → `Shutdown()` → `Environment.Exit(0)` | | `Terminate` named event | `App.xaml.cs::OnLaunched` → `RegisterEvent(..., () => Environment.Exit(0), "Terminate")` | | Tray-quit | `TrayIconService` callback → `Environment.Exit(0)` | | Runner-exit detection | `RunnerHelper.WaitForPowerToysRunner` callback → `Environment.Exit(0)` | `Environment.Exit` calls `ExitProcess` under the hood, which terminates all threads abruptly. Background `Task.WhenAll` doing DDC capability fetch is killed mid-flight; the `finally` block that calls `scope.Dispose()` never runs; `discovery.lock` orphans; Phase 0 next time false-positives. Concrete repro from logs: - `15:08:42.510` lock written - `15:08:42.79` probe monitor #1 - `15:08:46.92` probe monitor #2 (started, not finished — typical probe takes ~5s) - `15:08:49.03` `TerminateApp` received → `Environment.Exit(0)` → no `Dispose` log line - `15:10:10.03` next startup: Phase 0 sees orphan lock with `pid:17712, startedAt:2026-05-28T07:08:42Z` → writes `crash_detected.flag` → auto-disables ### Fix `CrashDetectionScope.Begin()` now also subscribes to `AppDomain.CurrentDomain.ProcessExit`. The handler does a best-effort `File.Delete(_lockPath)` (swallowing exceptions, as required for ProcessExit handlers). `Dispose()` unsubscribes before deleting. An `Interlocked.Exchange` guards the race between Dispose and ProcessExit so only one of the two performs the delete. ProcessExit's semantics match the cooperative/involuntary partition exactly: | Shutdown path | ProcessExit fires? | Behavior after this PR | |---|---|---| | `Environment.Exit(code)` (all 4 paths above) | yes | lock deleted by handler | | `Environment.FailFast` | no | lock survives → Phase 0 catches it (correct: explicit FailFast = real failure) | | BSOD / external `TerminateProcess` / kernel OOM | no | lock survives → Phase 0 catches it (correct: original design) | | Discovery completes normally / throws | n/a | `try/finally` calls `Dispose()` as before; handler unsubscribed first | ### Testability A new `IProcessExitHook` interface abstracts the subscription so unit tests can simulate ProcessExit without terminating the test runner. Production code uses the default `AppDomainProcessExitHook` singleton; tests inject a fake whose `RaiseExit()` invokes subscribed handlers synchronously. ### Files touched - `src/modules/powerdisplay/PowerDisplay.Lib/Services/IProcessExitHook.cs` *(new)* — interface + production singleton - `src/modules/powerdisplay/PowerDisplay.Lib/Services/CrashDetectionScope.cs` — subscribe in `Begin`, unsubscribe in `Dispose`, add `OnProcessExit` handler, expanded class doc - `src/modules/powerdisplay/PowerDisplay.Lib.UnitTests/CrashDetectionScopeTests.cs` *(new)* — 10 unit tests <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed ### Automated 10 new unit tests in `CrashDetectionScopeTests`, all passing: ``` Passed Begin_WritesLockFileAtomically Passed Begin_SubscribesToProcessExit Passed Dispose_UnsubscribesFromProcessExit Passed Dispose_DeletesLockFile Passed ProcessExitFired_BeforeDispose_DeletesLock (core scenario) Passed ProcessExitFired_AfterDispose_DoesNothing Passed Dispose_AfterProcessExit_DoesNotThrow Passed ProcessExitFired_LockFileMissing_DoesNotThrow Passed Dispose_IsIdempotent Passed MultipleScopes_DoNotShareState ``` Full `PowerDisplay.Lib.UnitTests` suite: **129 / 132 passing**. The 3 failures (`DetectOrphanAndDisable_RunsFullSequenceWhenOrphanPresent`, `DetectOrphanAndDisable_HandlesUnknownVersionAsOrphan`, `DetectOrphanAndDisable_LeavesLockIntactOnSignalFailure`) are **pre-existing on `main`** — they fail with `REGDB_E_CLASSNOTREG` from `Constants.AutoDisablePowerDisplayEvent()` (WinRT activation factory not COM-registered in the test environment). Verified by stashing this PR's changes and re-running the same 3 tests on baseline `main` — same failures, same cause, unrelated to this change. ### Manual 1. Reproduced the original false-positive on `main`: - Enable PowerDisplay → open Settings UI → quickly toggle PowerDisplay off - Observe `discovery.lock` left in `%LOCALAPPDATA%\Microsoft\PowerToys\PowerDisplay\` - Re-enable PowerDisplay → Phase 0 writes `crash_detected.flag` → InfoBar appears 2. Repeated the same steps with this branch: - Toggling PowerDisplay off cleanly deletes `discovery.lock` (ProcessExit handler ran) - Re-enabling PowerDisplay shows no InfoBar, no `crash_detected.flag` created 3. BSOD path is unchanged (verified by inspecting the conditional logic — `AppDomain.ProcessExit` does not fire for involuntary terminations; the lock survives just as before). --------- Co-authored-by: Yu Leng <yuleng@microsoft.com> Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
M
moooyo committed
cd5027fa1a60a4f8679214149a9fdec1b23f962d
Parent: 7da62cd
Committed by GitHub <noreply@github.com>
on 5/28/2026, 9:02:14 AM