0 Fork 0

mirror of https://github.com/microsoft/PowerToys.git synced 2026-10-28 16:44:58 +08:00

Files

T

Boliang ZhangandCopilot b9c2d3c1c5 fix(runner): unload modules before CRT teardown (#51433 )

## 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.

## PR Checklist

- [ ] Closes: N/A; regression introduced by #50899, restoring Hybrid CRT
after the mitigation in #51432

- [ ] **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

## 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

7.4 KiB

Raw Permalink Blame History

PowerToys Development Guidelines

Using Open Source Packages and Libraries

License Considerations

MIT license is generally acceptable for inclusion in the project
For any license other than MIT, double check with the PM team
All external packages or projects must be mentioned in the notice.md file
Even if a license permits free use, it's better to verify with the team

Safety and Quality Considerations

Ensure the code being included is safe to use
Avoid repositories or packages that are not widely used
Check for packages with significant downloads/usage and good ratings
Important because our pipeline signs external DLLs with Microsoft certificate
Unsafe code signed with Microsoft certificate can cause serious issues

Code Signing

Signing JSON File

Modifications to the signing JSON file are typically done manually
When adding new DLLs (internal PowerToys modules or external libraries)
When the release pipeline fails with a list of unsigned DLLs/executables:
- For PowerToys DLLs, manually add them to the list
- For external DLLs, verify they're safe to sign before including

File Signing Requirements

All DLLs and executables must be signed
New files need to be added to the signing configuration
CI checks if all files are signed
Even Microsoft-sourced dependencies are signed if they aren't already

Performance Measurement

Currently no built-in timers to measure PowerToys startup time
Startup measurement could be added in the runner:
- At the start of the main method
- After all module interface DLLs are loaded
Alternative: use profilers or Visual Studio profiler
Startup currently takes some time due to:
- Approximately 20 module interface DLLs that need to be loaded
- Modules that are started during loading
No dashboards or dedicated tools for performance measurement
Uses System.Diagnostics.Stopwatch in code
Performance data is logged to default PowerToys logs
Can search logs for stopwatch-related messages to diagnose performance issues
Some telemetry events contain performance information

Dependency Management

WinRT SDK and CS/WinRT

Updates to WinRT SDK and CS/WinRT are done periodically
WinRT SDK often requires higher versions of CS/WinRT or vice versa
Check for new versions in NuGet.org or Visual Studio's NuGet Package Explorer
Prefer stable versions over preview versions
Best practice: Update early in the release cycle to catch potential regressions

WebView2

Used for components like monotone file preview
WebView2 version is related to the WebView runtime in Windows
Previous issues with Windows Update installing new WebView runtime versions
WebView team now includes PowerToys testing in their release cycle
When updating WebView2:
- Update the version
- Open a PR
- Perform sanity checks on components that use WebView2

General Dependency Update Process

When updating via Visual Studio, it will automatically update dependencies
After updates, perform:
- Clean build
- Sanity check that all modules still work
- Open PR with changes

C/C++ Runtime (Hybrid CRT)

Cpp.Build.props links the VC++ runtime and STL statically (/MT) and the Universal CRT from the in-box ucrtbase.dll, so no VC++ redistributable is needed.
Every module in a process shares that UCRT, including WinUI 3 and .NET. Don't change process-wide CRT state:
- No setlocale, std::locale::global or named std::locale objects; MSVC's STL calls setlocale while building them. Use _wcreate_locale with the _l functions, or NLS APIs such as LCMapStringEx.
- No signal or _set_invalid_parameter_handler from a DLL.
- Read environment variables with GetEnvironmentVariableW; the CRT's copy is a snapshot.
If a project sets Link IgnoreSpecificDefaultLibraries or AdditionalOptions, keep the inherited %(...) value.
A project that can't follow these rules sets <EnableHybridCRT>false</EnableHybridCRT> with a comment that says why. Today: PowerRename.exe and its tests (PowerRenameLib calls std::locale::global) and MeasureToolCore (installs a SIGABRT handler).
Debug builds need ucrtbased.dll, which the Windows SDK installs. Use a Release build on machines without the SDK.

Testing Requirements

Multiple Computers

Mouse Without Borders: Requires multiple physical computers for proper testing
- Testing with VMs is not recommended as it may cause confusion between host and guest mouse input
- At least 2 computers are needed, sometimes testing with 3 is done
- Testing is usually assigned to team members known to have multiple computers

Multiple Monitors

Some utilities require multiple monitors for testing
At least 2 monitors are recommended
One monitor should be able to use different DPI settings

Fuzzing Testing

Security team requires fuzzing testing for modules that handle file I/O or user input
Helps identify vulnerabilities and bugs by feeding random, invalid, or unexpected data
PowerToys integrates with Microsoft's OneFuzz service for automated testing
Both .NET (C#) and C++ modules have different fuzzing implementation approaches
New modules handling file I/O or user input should implement fuzzing tests
For detailed setup instructions, see Fuzzing Testing in PowerToys

Testing Process

For reporting bugs during the release candidate testing:
1. Discuss in team chat
2. Determine if it's a regression (check if bug exists in previous version)
3. Check if an issue is already open
4. Open a new issue if needed
5. Decide on criticality for the release (if regression)

Release Testing

Team follows a release checklist
Includes testing for WinGet configuration
Sign-off process:
- Teams sign off on modules independently
- Regressions found in first release candidates lead to PRs
- Second release candidate verified fixes
- Command Palette needs separate sign-off
- Final verification ensures modules don't crash with Command Palette integration

PR Management and Release Process

PR Review Process

PM team typically tags PRs with "need review"
Small fixes from community that don't change much are usually accepted
PM team adds tags like "need review" to highlight PRs
PMs set priorities (sometimes using "info.90" tags)
PMs decide which PRs to prioritize
Team members can help get PRs merged when there's flexibility

PR Approval Requirements

PRs need approval from code owners before merging
New team members can approve PRs but final approval comes from code owners

PR Priority Handling

Old PRs sometimes "slip through the cracks" if not high priority
PMs tag important PRs with "priority one" to indicate they should be included in release
Draft PRs are generally not prioritized

Specific PR Types

CI-related PRs need review and code owner approval
Feature additions (like GPO support) need PM decision on whether the feature is wanted
Bug fixes related to Watson errors sometimes don't have corresponding issue links
Command Palette is considered high priority for the upcoming release

Project Management Notes

Be careful about not merging incomplete features into main
Feature branches should be used for work in progress
PRs touching installer files should be carefully reviewed
Incomplete features should not be merged into main
Use feature branches (feature/name-of-feature) for work-in-progress features
Only merge to main when complete or behind experimentation flags

7.4 KiB Raw Permalink Blame History