4 Commits
Author SHA1 Message Date
Boliang ZhangandCopilot 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>
2026-10-22 16:08:51 +08:00
Boliang ZhangandCopilot 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
2026-10-21 11:28:38 +08:00
Clint RutkasandCopilot App 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>
2026-10-06 07:52:31 -07:00
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>
2025-07-01 14:27:34 +02:00