KEMBAR78
Improve COM static store usage by JohnMcPMS · Pull Request #5680 · microsoft/winget-cli · GitHub
Skip to content

Conversation

JohnMcPMS
Copy link
Member

@JohnMcPMS JohnMcPMS commented Aug 22, 2025

Issue

The use of the COM static store was causing crashes on long lived processes because the implementation of DllCanUnloadNow did not count those objects, nor did it clean them up. This led to our module being unloaded, then when it was reloaded, the recreation of the static store object would invoke the deletion of the old one, which was pointing to the old, unloaded module location.

Design Considerations

WindowsPackageManager.dll serves many purposes, making the lifetime of statics complex.

  • It serves as "in-proc" for the CLI
  • It is the core implementation for the in-proc COM
  • It is the core implementation for the OOP COM

In order to support in-proc COM, we must put static lifetime COM objects in the static store. But in order to support unloading, we must also clean them up. Additionally, we don't want to claim to be in use if the only active objects are our statics (which are typically just event handlers and their owners).

We already use the WRL object count to track OOP COM server lifetime, and similarly we use it to implement DllCanUnloadNow. This is the count externally owned objects; those that the client has requested directly or indirectly.

Change

The major change is to remove all of our static store objects when WRL says we have no more externally owned. This is achieved by tracking the names of the objects that we insert and attempting to remove them when appropriate.

The original change to use the static store was templatized and reused to hold the termination signal handler.

Validation

The new test uses the CLI to validate that the implementation for DllCanUnloadNow (WindowsPackageManagerInProcModuleTerminate) detects the unload state and properly destroys the relevant objects. This is done by checking that there are internal objects allocated before the call, but none after.

Microsoft Reviewers: Open in CodeFlow

@github-actions

This comment was marked as outdated.

Copy link
Contributor

@yao-msft yao-msft left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@JohnMcPMS JohnMcPMS merged commit dabd473 into microsoft:master Aug 25, 2025
9 checks passed
@JohnMcPMS JohnMcPMS deleted the com-lifetime branch August 25, 2025 16:28
JohnMcPMS added a commit to JohnMcPMS/winget-cli that referenced this pull request Aug 25, 2025
The use of the COM static store was causing crashes on long lived
processes because the implementation of `DllCanUnloadNow` did not count
those objects, nor did it clean them up. This led to our module being
unloaded, then when it was reloaded, the recreation of the static store
object would invoke the deletion of the old one, which was pointing to
the old, unloaded module location.

WindowsPackageManager.dll serves many purposes, making the lifetime of
statics complex.
- It serves as "in-proc" for the CLI
- It is the core implementation for the in-proc COM
- It is the core implementation for the OOP COM

In order to support in-proc COM, we must put static lifetime COM objects
in the static store. But in order to support unloading, we must also
clean them up. Additionally, we don't want to claim to be in use if the
only active objects are our statics (which are typically just event
handlers and their owners).

We already use the WRL object count to track OOP COM server lifetime,
and similarly we use it to implement `DllCanUnloadNow`. This is the
count externally owned objects; those that the client has requested
directly or indirectly.

The major change is to remove all of our static store objects when WRL
says we have no more externally owned. This is achieved by tracking the
names of the objects that we insert and attempting to remove them when
appropriate.

The original change to use the static store was templatized and reused
to hold the termination signal handler.

The new test uses the CLI to validate that the implementation for
`DllCanUnloadNow` (`WindowsPackageManagerInProcModuleTerminate`) detects
the unload state and properly destroys the relevant objects. This is
done by checking that there are internal objects allocated before the
call, but none after.
JohnMcPMS added a commit that referenced this pull request Aug 25, 2025
CP from #5680 

Ported changes backward from the termination handling refactor.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants