Pull Request Overview
- Opened on September 16, 2026
- Status Merged
- Commit count 2 with first commit September 16, 2026
Total Delta
Open Days
Test Delta
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
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.cppremoves 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.hremoves the obsolete initialization helper and grants the existing test pattern's private access to the new lifetime test class. -
src/modules/powerrename/unittests/WICMetadataExtractorTests.cppaddsFactoryHasNoPersistentOwner. 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: 1remaining 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.exeexit 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
Resolved Comment Threads
No resolved comments have been left on this PR.
### π’ 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.