mirror of
https://github.com/microsoft/PowerToys.git
synced 2026-10-28 16:44:58 +08:00
main
4
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
b9c2d3c1c5 |
fix(runner): unload modules before CRT teardown (#51433)
<!-- 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 Restore Hybrid CRT together with the Runner shutdown fix for the tray-Quit process hang introduced by #50899. Without explicit early cleanup, Runner's static module-map destruction can hold the shared UCRT exit lock while waiting for a module worker that needs that same lock. Main temporarily disabled Hybrid CRT through #51432. This PR now reverses that rollback **in the same change as the shutdown fix**, completing module shutdown before CRT teardown and before releasing Runner's lifetime guards. The rollback reversal must not be merged independently of the fix. The original three commits were rebased onto main `e7afd3fa439906a8e4ff4388a961511ca5cf81e7` without conflicts or patch changes, followed by the exact revert of #51432. The combined fix was validated at `d62d5e004e70a1f8a4eaca7f591fab8bf7eebd8f`; follow-ups remove both `runner.md` changes and the manual shutdown-check script from this PR without changing product code. Stable and the published 0.102 release are not changed. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [ ] Closes: N/A; regression introduced by #50899, restoring Hybrid CRT after the mitigation in #51432 <!-- - [ ] Closes: #yyy (add separate lines for additional resolved issues) --> - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass — existing native tests and prior manual shutdown checks passed; no new automated test is retained in this PR, and broader restart/elevation/session-ending runtime coverage remains incomplete - [x] **Localization:** All end-user-facing strings can be localized — no new end-user strings - [x] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places — N/A, no new shipping binaries - [ ] [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: N/A; repository developer documentation only <!-- 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 - `src\runner\main.cpp`: a local cleanup guard cancels deferred Settings startup, stops the centralized hook, revokes module hotkey callbacks, and clears the module map before the settings-CLI and single-instance guards are released. It covers ordinary exit and exception unwinding as well as restart; the old restart-only clear is removed. - `src\runner\centralized_hotkeys.cpp`: revoke every matching callback and avoid trying to unregister window-owned hotkeys after their window has been destroyed. - Advanced Paste and PowerDisplay process-manager headers: declare the executor last so its worker joins before the pipe/state accessed by its tasks is destroyed. - `Cpp.Build.props` and `doc\devdocs\development\guidelines.md`: exactly revert `cf45de4118c22dbe80a2aa55c6ea0935ea7e2b3b` (#51432), restoring the shared-UCRT default and its runtime guidance. Existing project-specific CRT choices remain intact. The New+ and logger fixes from #50899 are not reverted. The commit order keeps the shutdown fix before Hybrid CRT is re-enabled. Rebasing already reapplied the fix; it was not duplicated. The final diff against the PR base contains six files. The Runner documentation, new Runner verification profile, and manual shutdown-check script are excluded. Existing Windows session-ending skips for Settings and Quick Access are unchanged. No settings schema, IPC contract, elevation policy, or pipeline changes. The initial diagnosis used matching PDBs on lingering installed 0.102.2883.0 processes and an isolated static/Hybrid CRT linkage experiment. These support the root cause but do not replace runtime validation. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> |
||
|
|
cf45de4118 |
fix(build): disable Hybrid CRT by default (#51432)
<!-- 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 Disable Hybrid CRT by default as a scoped rollback of the build-policy change in #50899, mitigating Runner exit hangs involving the shared UCRT exit-table lock. `Cpp.Build.props` now defaults `EnableHybridCRT` to `false` when no value is supplied. Native projects return to private static UCRT linkage, while explicit opt-ins and independent project-level Hybrid CRT linker settings remain supported. The New+ locale/environment-variable changes and the logger environment-variable fix from #50899 are intentionally retained. Only `Cpp.Build.props` and the directly related runtime guidance in `doc/devdocs/development/guidelines.md` change. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [ ] Closes: N/A (partial rollback of #50899; no separate issue linked) <!-- - [ ] Closes: #yyy (add separate lines for additional resolved issues) --> - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass. No new application unit tests; native build and effective-linkage validation are listed below. - [x] **Localization:** All end-user-facing strings can be localized (N/A: no user-facing strings) - [x] **Dev docs:** Added/updated - [ ] **New binaries:** Added on the required places (N/A: no new binaries) - [ ] [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: N/A (developer guidance is updated in this PR; no user documentation changes) <!-- 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 ### Why restore private UCRT linkage? Matching-symbol inspection of installed 0.102.2883.0 found Runner performing static module destruction inside `ucrtbase!execute_onexit_table`. In one confirmed wait cycle: - The Runner main thread holds the CRT exit-table lock and joins the PowerDisplay executor while destroying the module. - The executor creates an ATL `CString` while sending the shutdown message. Lazy static initialization calls `atexit`/`register_onexit_function`, which needs the same shared CRT lock. Other captured remnants show related exit-table/loader waits involving Light Switch listener cleanup and Mouse Highlighter DLL detach. Sharing the UCRT makes these previously separate runtime locks shared across the executable and module DLLs. The reported working/failing boundary is 0.101.2362.0 to 0.102.2803.0. Git ancestry confirms that the latter includes #50899 but does not yet include #50897 or #51115. This is a build-policy mitigation, not a rewrite of Runner shutdown sequencing. ### Scope and tradeoffs - Preserve `/MT` and `/MTd`; disable the default `libucrt` exclusion and shared-UCRT import-library selection by making their existing condition false. - Preserve `CalculatorEngineCommon`'s explicit opt-in, the Command Palette Extensions SDK's independent Hybrid CRT linker configuration, and existing project opt-outs. - No CI/workflow/build-script override re-enables Hybrid CRT for Runner or its affected native modules. - Preserve the New+ and logger fixes, settings/IPC formats, and installed-user state. - Accept losing the default binary-size reduction until shutdown lifetimes are made safe for a shared CRT. No current installer-size measurement is claimed. <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed ### Native builds: passed The following commands were run in the isolated worktree. All completed with exit code 0 after restoring the fresh worktree's missing native packages. ```powershell # PASS: native package restore. & 'C:\Program Files\PowerShell\7\pwsh.exe' -NoProfile -File 'D:\PowerToys-hybrid-crt-rollback\tools\build\build.ps1' -Platform x64 -Configuration Release -Path 'D:\PowerToys-hybrid-crt-rollback\src\modules\powerdisplay\PowerDisplayModuleInterface' '/t:Restore' '/p:RestorePackagesConfig=true' '/p:SolutionDir=D:\PowerToys-hybrid-crt-rollback\' '/p:SkipLongPathsCheck=true' # PASS: PowerDisplayModuleInterface and native dependencies; 0 errors, 0 warnings. & 'C:\Program Files\PowerShell\7\pwsh.exe' -NoProfile -File 'D:\PowerToys-hybrid-crt-rollback\tools\build\build.ps1' -Platform x64 -Configuration Release -Path 'D:\PowerToys-hybrid-crt-rollback\src\modules\powerdisplay\PowerDisplayModuleInterface' '/p:SolutionDir=D:\PowerToys-hybrid-crt-rollback\' '/p:SkipLongPathsCheck=true' # PASS: Runner and native dependencies; 0 errors, 1 host LIB-path warning. & 'C:\Program Files\PowerShell\7\pwsh.exe' -NoProfile -File 'D:\PowerToys-hybrid-crt-rollback\tools\build\build.ps1' -Platform x64 -Configuration Release -Path 'D:\PowerToys-hybrid-crt-rollback\src\runner' '/restore' '/p:RestorePackagesConfig=true' '/p:SolutionDir=D:\PowerToys-hybrid-crt-rollback\' '/p:SkipLongPathsCheck=true' ``` The host has Windows long-path support disabled; the repository's documented `SkipLongPathsCheck` override was used for these scoped native builds without changing machine policy. Runner's warning was CS1668 for a nonexistent Visual Studio `LIB` search directory, not a source warning introduced by this change. ### Effective compiler/linker configuration: 25 cases passed A local-only MSBuild diagnostic target imported the original `Cpp.Build.targets`, added a non-built `Link` probe item to obtain evaluated item-definition metadata, and returned `EnableHybridCRT`, the actual `ClCompile.RuntimeLibrary` values, `Link.IgnoreSpecificDefaultLibraries`, and `Link.AdditionalOptions`. For each project/configuration/platform row, the command was: ```powershell # PASS for all 24 matrix rows; $probe is the local ReportCrtLinkage.targets file. & $msbuild $project /nologo /v:q /t:ReportCrtLinkage /getTargetResult:ReportCrtLinkage "/p:Platform=$platform" "/p:Configuration=$configuration" '/p:SolutionDir=D:\PowerToys-hybrid-crt-rollback\' "/p:ForceImportAfterCppTargets=$probe" /p:SkipLongPathsCheck=true # PASS: additional Runner x64 Release positive control, using the same command with: # /p:EnableHybridCRT=true ``` | Projects | Configurations | Platforms | Result | |---|---|---|---| | Runner, PowerDisplayModuleInterface, LightSwitchModuleInterface, MouseHighlighter | Debug, Release | x64, ARM64 | Private UCRT; `/MTd` or `/MT`; no shared-UCRT linker overrides | | CalculatorEngineCommon | Debug, Release | x64, ARM64 | Explicit Hybrid CRT opt-in retained | | Command Palette Extensions SDK | Debug, Release | x64, ARM64 | Independent Hybrid CRT linker configuration retained | | Runner with explicit `EnableHybridCRT=true` | Release | x64 | Opt-in still selects the shared UCRT | The four affected native projects and the positive control were evaluated with VS 2026 MSBuild. The two unchanged UWP opt-in projects were evaluated with installed VS 2022 MSBuild because the VS 2026 Build Tools installation lacks the UWP XAML targets. These are configuration evaluations, not ARM64 build/runtime passes. ### Rebuilt binary imports: passed ```powershell # PASS: both dumpbin commands exited 0. & $dumpbin /nologo /imports 'D:\PowerToys-hybrid-crt-rollback\x64\Release\PowerToys.exe' & $dumpbin /nologo /imports 'D:\PowerToys-hybrid-crt-rollback\x64\Release\PowerToys.PowerDisplayModuleInterface.dll' ``` Both rebuilt binaries have no `api-ms-win-crt-*`, `ucrtbase.dll`, or `ucrtbased.dll` imports. This checks the resulting linkage, not only the property value. `git diff --check` also passed. Not claimed: application unit/UI test execution, a full installer or ARM64 build, or a post-change installed-product shutdown/restart A/B. The installed PowerToys binaries and existing hung-process evidence were not replaced or terminated. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: cf21252c-f253-42f2-a45c-3146c2481127 |
||
|
|
89e2048231 |
[Build] Link native binaries against the in-box UCRT (Hybrid CRT) (#50899)
<!-- 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 Native DLLs and EXEs now use the **Hybrid CRT**: the VC++ runtime and STL stay static (`/MT`), and the UCRT comes from the in-box `ucrtbase.dll` instead of a copy in every binary. It's one `Cpp.Build.props` setting (`EnableHybridCRT`), adds no runtime dependency, and matches Windows App SDK and Terminal. <!-- Please review the items on the PR checklist before submitting--> ## PR Checklist - [ ] Closes: #xxx <!-- - [ ] Closes: #yyy (add separate lines for additional resolved issues) --> - [ ] **Communication:** I've discussed this with core contributors already. If the work hasn't been agreed, this work might be rejected - [ ] **Tests:** Added/updated and all pass - [ ] **Localization:** All end-user-facing strings can be localized - [x] **Dev docs:** Added/updated - [ ] **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 - Modules in a process now share one UCRT, so code mustn't change process-wide CRT state; the rules are in `doc/devdocs/development/guidelines.md`. - New+ (in Explorer) no longer calls `std::locale::global`: date names are capitalized with `LCMapStringEx` (same output), and `%VAR%` and the logger's flag use `GetEnvironmentVariableW`. - PowerRename.exe, its two test projects and MeasureToolCore keep a private UCRT: PowerRenameLib's `std::locale::global` would change WinUI 3's C locale, and MeasureToolCore installs a `SIGABRT` handler. Fixing them is a follow-up. - Removed empty `IgnoreSpecificDefaultLibraries` elements in New+ that dropped the `libucrt` exclusion. - Debug builds need `ucrtbased.dll` (Windows SDK). <!-- Describe how you validated the behavior. Add automated tests wherever possible, but list manual validation steps taken as well --> ## Validation Steps Performed - PR CI [390228](https://dev.azure.com/shine-oss/PowerToys/_build/results?buildId=390228) passed: x64/ARM64 builds, installers, unit tests and NewPlus/PowerRename/ScreenRuler UI tests. [390260](https://dev.azure.com/shine-oss/PowerToys/_build/results?buildId=390260), for the comment-only cleanup commit, is in progress. - vs main [390037](https://dev.azure.com/shine-oss/PowerToys/_build/results?buildId=390037): about −13 MB of native binaries and −6.2 MB per x64 installer; nothing newly imports `vcruntime140`/`msvcp140`. - Local x64 Debug/Release builds of the touched projects; PowerRename unit tests 249/249. - New+ date names are unchanged in 8 locales, and the fuzzer runs clean under ASan with the Hybrid CRT. --------- Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> |
||
|
|
725535b760 |
[DevDocs] More content and restructure (#40165)
## Summary of the Pull Request Accumulated information from internal transition about the modules development, and reworked it to be added in dev docs. Also the dev docs intself was restructured to be more organized. New pages was verified by transition team. ## PR Checklist - [x] **Dev docs:** Added/updated --------- Co-authored-by: Zhaopeng Wang (from Dev Box) <zhaopengwang@microsoft.com> Co-authored-by: Hao Liu <liuhao3418@gmail.com> Co-authored-by: Peiyao Zhao <105847726+zhaopy536@users.noreply.github.com> Co-authored-by: Mengyuan <162882040+chenmy77@users.noreply.github.com> Co-authored-by: zhaopeng wang <33367956+wang563681252@users.noreply.github.com> Co-authored-by: Jaylyn Barbee <51131738+Jaylyn-Barbee@users.noreply.github.com> |