fix(powerrename): avoid WIC shutdown during DLL teardown #50593

Merged
LegendaryBlair opened 11:18pm on September 15, 2026 wanted to merge 2 commits into microsoft/powertoys main from
LegendaryBlair/fix-powerrename-wic-lifetime

Pull Request Overview

  • Opened on September 16, 2026
  • Status Merged
  • Commit count 2 with first commit September 16, 2026

Total Delta

0 Total Diff Delta

Open Days

Open 3 weekdays

Test Delta

0 Diff Delta in Test Files
Breakdown by Phase

How long has this pull request spent in each phase of its lifecycle?

Fraction of total time Business days Phase
 
0.0 days Authoring 1 commit before pull request opened for review
 
0.0 days Awaiting first review
 
0.2 days Revising work with 1 commit in response to 2 reviews that left 2 comments
 
3.0 days Merge to deploy (still awaiting deployment)

Total time for pull request 0.2 business days from first commit authored to merge
Still awaiting deployment

Author avatar

fix(powerrename): avoid WIC shutdown during DLL teardown

Summary of the Pull Request

Avoid releasing a process-global WIC imaging factory during DLL teardown. A real CI hang showed PowerRename.UnitTests.dll unloading under the loader lock, with the static g_wicFactory destructor entering Windows codecs, MFShutdown, and a thread-pool wait.

Create WIC factories only for decoder operations instead of retaining static COM ownership. Preserve the existing metadata-result cache and caller-owned COM initialization.

This is an independent PowerRename fix extracted from the investigation in #50576. It includes no Workspaces changes, diagnostic tooling, test exclusions, new dependencies, or installer changes.

PR Checklist


  • [x] Communication: The separate fix was requested after reviewing the captured shutdown evidence and diagnostic experiment.

  • [x] Tests: Added a factory-ownership regression and validated the exact patch against the PowerRename and mixed native test suites.

N/A: localization, new binaries, settings migrations, and user documentation changes.

Detailed Description of the Pull Request / Additional comments



  • src/modules/powerrename/lib/WICMetadataExtractor.cpp removes the global COM factory, initialization flag, and factory mutex. Factory references are now local to decoder creation rather than static object destruction.


  • src/modules/powerrename/lib/WICMetadataExtractor.h removes the obsolete initialization helper and grants the existing test pattern's private access to the new lifetime test class.


  • src/modules/powerrename/unittests/WICMetadataExtractorTests.cpp adds FactoryHasNoPersistentOwner. The original code leaves one hidden reference; the corrected code releases the final reference at the operation boundary.

Simply moving the factory into an extractor instance would not cover every path: Renaming.cpp also retains a static MetadataPatternExtractor. Its metadata-result cache is left intact, but it no longer owns a WIC factory indirectly.

The captured executor stack was identical in two non-suspending snapshots:

FreeLibrary

-> PowerRename.UnitTests DLL detach
-> static g_wicFactory destructor
-> IWICImagingFactory release
-> Windows codecs
-> MFShutdown
-> thread-pool wait

Microsoft's MFShutdown documentation warns against calling it from static destructors during process exit.

Tradeoff for review: a metadata cache miss now creates a factory for decoder work. Metadata results remain cached, so repeated cached lookups do not recreate it. Large uncached image batches and codec-specific costs have not been benchmarked; this PR does not claim a performance improvement.

Validation Steps Performed


  • Built the real PowerRename library and native tests in x64 Release.

  • Confirmed the new ownership regression fails against the old global factory: Expected: 0, Actual: 1 remaining references.

  • With the correction, all 249 PowerRename tests and 69 Workspaces tests passed together locally (318/318).

  • Diagnostic CI 388929, with the triggering 19 IPC tests unchanged, completed Native Tests in 182.5 seconds: 7,992 passed, 18 existing skips, 0 failures, and vstest.console.exe exit code 0.

  • Windows 10 PowerRename UI tests passed 25/25 in that run; Windows 11 and ARM64 UI jobs succeeded. The overall diagnostic run had a separate Workspaces process-enumeration UI failure, so it is not described as fully green.

  • Later IPC-fixture cleanup checks independently found and corrected missing callback-drain barriers. With the original WIC implementation restored locally, the corrected IPC fixture could still reproduce the same PowerRename shutdown stack. Both corrections together completed 321/321 mixed tests.

  • This main-based branch contains the exact tested PowerRename source. Independent CI 388938 fully succeeded, including x64 and ARM64 builds and the Windows 10, Windows 11, and ARM64 UI jobs.

Comments Threads Pending Resolution

ljharb reviewed on September 16, 2026
ljharb left a comment

### 🟒 Approval recommended

The focused lifetime fix is consistent across implementation and declarations and includes a targeted regression test.

Pull request overview

Removes process-global WIC ownership to prevent PowerRename DLL teardown hangs.

**Changes:**
- Creates WIC factories per decoder operation.
- Removes obsolete static initialization.
- Adds a factory-lifetime regression test.

File summaries

| File | Description |
| ---- | ----------- |
| `WICMetadataExtractor.cpp` | Replaces the global factory with operation-local creation. |
| `WICMetadataExtractor.h` | Removes initialization API and grants test access. |
| `WICMetadataExtractorTests.cpp` | Verifies no persistent factory reference remains. |

Review details

- **Files reviewed:** 3/3 changed files
- **Comments generated:** 0
- **Review effort level:** Balanced

---

πŸ’‘ Add a `code-review` agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Resolved Comment Threads

No resolved comments have been left on this PR.