commit 2634edea378da1adc58745e434c9ebfc7fdb884c
parent 7478a787efa1e3f27132f9688dee20b33bd9cb0c
Author: KEINOS <github+fork-qiita-news@keinos.com>
Date: Mon, 25 Aug 2025 16:34:34 +0000
Merge remote-tracking branch 'upstream/master'
Diffstat:
12 files changed, 271 insertions(+), 60 deletions(-)
diff --git a/src/AppInstallerCLICore/Commands/TestCommand.cpp b/src/AppInstallerCLICore/Commands/TestCommand.cpp
@@ -11,6 +11,7 @@
#include "Public/ShutdownMonitoring.h"
#include "Workflows/ConfigurationFlow.h"
#include "Workflows/MSStoreInstallerHandler.h"
+#include <winget/RepositorySource.h>
#include <winrt/Microsoft.Management.Configuration.h>
using namespace AppInstaller::CLI::Workflow;
@@ -29,7 +30,7 @@ namespace AppInstaller::CLI
HRESULT WaitForShutdown(Execution::Context& context)
{
LogAndReport(context, "Waiting for app shutdown event");
- if (!ShutdownMonitoring::TerminationSignalHandler::Instance().WaitForAppShutdownEvent())
+ if (!ShutdownMonitoring::TerminationSignalHandler::Instance()->WaitForAppShutdownEvent())
{
LogAndReport(context, "Failed getting app shutdown event");
return APPINSTALLER_CLI_ERROR_INTERNAL_ERROR;
@@ -41,7 +42,7 @@ namespace AppInstaller::CLI
HRESULT AppShutdownWindowMessage(Execution::Context& context)
{
- auto windowHandle = ShutdownMonitoring::TerminationSignalHandler::Instance().GetWindowHandle();
+ auto windowHandle = ShutdownMonitoring::TerminationSignalHandler::Instance()->GetWindowHandle();
if (windowHandle == NULL)
{
@@ -217,6 +218,65 @@ namespace AppInstaller::CLI
InvokeFindUnitProcessors;
}
};
+
+ struct TestCanUnloadNowCommand final : public Command
+ {
+ TestCanUnloadNowCommand(std::string_view parent) : Command("can-unload-now", {}, parent, Visibility::Hidden) {}
+
+ Resource::LocString ShortDescription() const override
+ {
+ return "Test DllCanUnloadNow"_lis;
+ }
+
+ Resource::LocString LongDescription() const override
+ {
+ return "Verifies that the function that implements the inproc DllCanUnloadNow properly blocks unload due to static storage object."_lis;
+ }
+
+ protected:
+ void ExecuteInternal(Execution::Context& context) const override
+ {
+ Repository::Source source{ Repository::PredefinedSource::Installed };
+
+ ProgressCallback progress;
+ source.Open(progress);
+
+ HMODULE self = GetModuleHandle(L"WindowsPackageManager.dll");
+ if (!self)
+ {
+ LogAndReport(context, "Couldn't get WindowsPackageManager module");
+ return;
+ }
+
+ auto WindowsPackageManagerInProcModuleTerminate = reinterpret_cast<bool (__stdcall *)()>(GetProcAddress(self, "WindowsPackageManagerInProcModuleTerminate"));
+
+ // Report the object counts, attempt to terminate, report the object counts again
+ ReportObjectCounts(context);
+ LogAndReport(context, WindowsPackageManagerInProcModuleTerminate() ? "DllCanUnloadNow" : "DllCannotUnloadNow");
+ ReportObjectCounts(context);
+ }
+
+ private:
+ void ReportObjectCounts(Execution::Context& context) const
+ {
+ std::ostringstream stream;
+ stream << "Internal objects: " << GetInternalObjectCount() << '\n';
+ stream << "External objects: " << GetExternalObjectCount();
+
+ LogAndReport(context, stream.str());
+ }
+
+ uint32_t GetInternalObjectCount() const
+ {
+ return winrt::get_module_lock().operator unsigned int();
+ }
+
+ unsigned long GetExternalObjectCount() const
+ {
+ auto module = Microsoft::WRL::GetModuleBase();
+ return module ? module->GetObjectCount() : 0;
+ }
+ };
}
std::vector<std::unique_ptr<Command>> TestCommand::GetCommands() const
@@ -226,6 +286,7 @@ namespace AppInstaller::CLI
std::make_unique<TestAppShutdownCommand>(FullName()),
std::make_unique<TestConfigurationExportCommand>(FullName()),
std::make_unique<TestConfigurationFindUnitProcessorsCommand>(FullName()),
+ std::make_unique<TestCanUnloadNowCommand>(FullName()),
});
}
diff --git a/src/AppInstallerCLICore/Public/ShutdownMonitoring.h b/src/AppInstallerCLICore/Public/ShutdownMonitoring.h
@@ -5,6 +5,7 @@
#include <AppInstallerProgress.h>
#include <winrt/Windows.ApplicationModel.h>
#include <wil/resource.h>
+#include <memory>
#include <mutex>
namespace AppInstaller::ShutdownMonitoring
@@ -12,8 +13,12 @@ namespace AppInstaller::ShutdownMonitoring
// Type to contain the CTRL signal and window messages handler.
struct TerminationSignalHandler
{
+ TerminationSignalHandler();
+
+ ~TerminationSignalHandler();
+
// Gets the singleton handler.
- static TerminationSignalHandler& Instance();
+ static std::shared_ptr<TerminationSignalHandler> Instance();
// Add a termination listener.
void AddListener(ICancellable* cancellable);
@@ -33,10 +38,6 @@ namespace AppInstaller::ShutdownMonitoring
#endif
private:
- TerminationSignalHandler();
-
- ~TerminationSignalHandler();
-
void StartAppShutdown();
static BOOL WINAPI StaticCtrlHandlerFunction(DWORD ctrlType);
diff --git a/src/AppInstallerCLICore/ShutdownMonitoring.cpp b/src/AppInstallerCLICore/ShutdownMonitoring.cpp
@@ -5,13 +5,19 @@
#include <AppInstallerErrors.h>
#include <AppInstallerLogging.h>
#include <AppInstallerRuntime.h>
+#include <winget/COMStaticStorage.h>
namespace AppInstaller::ShutdownMonitoring
{
- TerminationSignalHandler& TerminationSignalHandler::Instance()
+ std::shared_ptr<TerminationSignalHandler> TerminationSignalHandler::Instance()
{
- static TerminationSignalHandler s_instance;
- return s_instance;
+ struct Singleton : public WinRT::COMStaticStorageBase<TerminationSignalHandler>
+ {
+ Singleton() : COMStaticStorageBase(L"WindowsPackageManager.TerminationSignalHandler") {}
+ };
+
+ static Singleton s_instance;
+ return s_instance.Get();
}
void TerminationSignalHandler::AddListener(ICancellable* cancellable)
@@ -28,19 +34,25 @@ namespace AppInstaller::ShutdownMonitoring
std::lock_guard<std::mutex> lock{ m_listenersLock };
auto itr = std::find(m_listeners.begin(), m_listeners.end(), cancellable);
- THROW_HR_IF(E_NOT_VALID_STATE, itr == m_listeners.end());
- m_listeners.erase(itr);
+ if (itr == m_listeners.end())
+ {
+ AICLI_LOG(CLI, Warning, << "TerminationSignalHandler::RemoveListener did not find requested object");
+ }
+ else
+ {
+ m_listeners.erase(itr);
+ }
}
void TerminationSignalHandler::EnableListener(bool enabled, ICancellable* cancellable)
{
if (enabled)
{
- Instance().AddListener(cancellable);
+ Instance()->AddListener(cancellable);
}
else
{
- Instance().RemoveListener(cancellable);
+ Instance()->RemoveListener(cancellable);
}
}
@@ -109,7 +121,7 @@ namespace AppInstaller::ShutdownMonitoring
BOOL WINAPI TerminationSignalHandler::StaticCtrlHandlerFunction(DWORD ctrlType)
{
- return Instance().CtrlHandlerFunction(ctrlType);
+ return Instance()->CtrlHandlerFunction(ctrlType);
}
LRESULT WINAPI TerminationSignalHandler::WindowMessageProcedure(HWND hWnd, UINT uMsg, WPARAM wParam, LPARAM lParam)
@@ -118,7 +130,7 @@ namespace AppInstaller::ShutdownMonitoring
{
case WM_QUERYENDSESSION:
AICLI_LOG(CLI, Verbose, << "Received WM_QUERYENDSESSION");
- Instance().StartAppShutdown();
+ Instance()->StartAppShutdown();
return TRUE;
case WM_ENDSESSION:
case WM_CLOSE:
@@ -294,12 +306,12 @@ namespace AppInstaller::ShutdownMonitoring
ServerShutdownSynchronization::ServerShutdownSynchronization()
{
- TerminationSignalHandler::Instance().AddListener(this);
+ TerminationSignalHandler::Instance()->AddListener(this);
}
ServerShutdownSynchronization::~ServerShutdownSynchronization()
{
- TerminationSignalHandler::Instance().RemoveListener(this);
+ TerminationSignalHandler::Instance()->RemoveListener(this);
if (m_shutdownThread.joinable())
{
m_shutdownThread.detach();
diff --git a/src/AppInstallerCLICore/pch.h b/src/AppInstallerCLICore/pch.h
@@ -57,4 +57,5 @@
#pragma warning( pop )
#include <wrl/client.h>
+#include <wrl/implements.h>
#include <AppxPackaging.h>
diff --git a/src/AppInstallerCLIE2ETests/AppShutdownTests.cs b/src/AppInstallerCLIE2ETests/AppShutdownTests.cs
@@ -7,6 +7,7 @@
namespace AppInstallerCLIE2ETests
{
using System;
+ using System.Diagnostics;
using System.IO;
using System.Threading;
using System.Threading.Tasks;
@@ -35,6 +36,12 @@ namespace AppInstallerCLIE2ETests
Assert.Ignore("This test won't work on Window Server as non-admin");
}
+ if (!Environment.Is64BitProcess)
+ {
+ // My guess is that HAM terminates us faster after the CTRL-C on x86...
+ Assert.Ignore("This test is flaky when run as x86.");
+ }
+
if (string.IsNullOrEmpty(TestSetup.Parameters.AICLIPackagePath))
{
throw new NullReferenceException("AICLIPackagePath");
@@ -95,5 +102,24 @@ namespace AppInstallerCLIE2ETests
// Look for the output.
Assert.True(testCmdTask.Result.StdOut.Contains("Succeeded waiting for app shutdown event"));
}
+
+ /// <summary>
+ /// Runs winget test can-unload-now expecting that it cannot be unloaded.
+ /// </summary>
+ [Test]
+ public void CanUnloadNowTest()
+ {
+ var result = TestCommon.RunAICLICommand("test", "can-unload-now --verbose");
+
+ var lines = result.StdOut.Split('\n', StringSplitOptions.TrimEntries | StringSplitOptions.RemoveEmptyEntries);
+
+ Assert.AreEqual(5, lines.Length);
+ Assert.True(lines[0].Contains("Internal objects:"));
+ Assert.False(lines[0].Contains("Internal objects: 0"));
+ Assert.True(lines[1].Contains("External objects: 0"));
+ Assert.True(lines[2].Contains("DllCanUnloadNow"));
+ Assert.True(lines[3].Contains("Internal objects: 0"));
+ Assert.True(lines[4].Contains("External objects: 0"));
+ }
}
}
diff --git a/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp b/src/AppInstallerRepositoryCore/Microsoft/PredefinedInstalledSourceFactory.cpp
@@ -6,7 +6,7 @@
#include "Microsoft/SQLiteIndex.h"
#include "Microsoft/SQLiteIndexSource.h"
#include <winget/ManifestInstaller.h>
-
+#include <winget/COMStaticStorage.h>
#include <winget/Registry.h>
#include <AppInstallerArchitecture.h>
#include <winget/ExperimentalFeature.h>
@@ -270,41 +270,9 @@ namespace AppInstaller::Repository::Microsoft
struct CachedInstalledIndex
{
- // https://devblogs.microsoft.com/oldnewthing/20210215-00/?p=104865
- struct Singleton
+ struct Singleton : public WinRT::COMStaticStorageBase<CachedInstalledIndex>
{
- struct Holder : public winrt::implements<Holder, winrt::Windows::Foundation::IInspectable>
- {
- static constexpr std::wstring_view Guid{ L"{48c47064-4fff-4eca-812c-dbb4f33a8fcb}" };
- std::shared_ptr<CachedInstalledIndex> m_shared{ std::make_shared<CachedInstalledIndex>() };
- };
-
- std::weak_ptr<CachedInstalledIndex> m_weak;
- winrt::slim_mutex m_lock;
-
- std::shared_ptr<CachedInstalledIndex> Get()
- {
- {
- const std::shared_lock lock{ m_lock };
- if (auto cachedIndex = m_weak.lock())
- {
- return cachedIndex;
- }
- }
-
- auto value = winrt::make_self<Holder>();
-
- const std::shared_lock lock{ m_lock };
- if (auto cachedIndex = m_weak.lock())
- {
- return cachedIndex;
- }
-
- winrt::Windows::ApplicationModel::Core::CoreApplication::Properties().Insert(Holder::Guid, value.as<winrt::Windows::Foundation::IInspectable>());
-
- m_weak = value->m_shared;
- return value->m_shared;
- }
+ Singleton() : COMStaticStorageBase(L"WindowsPackageManager.CachedInstalledIndex") {}
};
CachedInstalledIndex()
diff --git a/src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj b/src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj
@@ -349,6 +349,7 @@
<ClInclude Include="Public\winget\AsyncTokens.h" />
<ClInclude Include="Public\winget\Certificates.h" />
<ClInclude Include="Public\winget\Compression.h" />
+ <ClInclude Include="Public\winget\COMStaticStorage.h" />
<ClInclude Include="Public\winget\ConfigurationSetProcessorHandlers.h" />
<ClInclude Include="Public\winget\Filesystem.h" />
<ClInclude Include="Public\winget\GroupPolicy.h" />
@@ -379,6 +380,7 @@
<ClCompile Include="AppInstallerStrings.cpp" />
<ClCompile Include="Certificates.cpp" />
<ClCompile Include="Compression.cpp" />
+ <ClCompile Include="COMStaticStorage.cpp" />
<ClCompile Include="DateTime.cpp" />
<ClCompile Include="Errors.cpp" />
<ClCompile Include="Filesystem.cpp" />
@@ -436,4 +438,4 @@
<Error Condition="!Exists('$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.230706.1\build\native\Microsoft.Windows.CppWinRT.props')" Text="$([System.String]::Format('$(ErrorText)', '$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.230706.1\build\native\Microsoft.Windows.CppWinRT.props'))" />
<Error Condition="!Exists('$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.230706.1\build\native\Microsoft.Windows.CppWinRT.targets')" Text="$([System.String]::Format('$(ErrorText)', '$(SolutionDir)\packages\Microsoft.Windows.CppWinRT.2.0.230706.1\build\native\Microsoft.Windows.CppWinRT.targets'))" />
</Target>
-</Project>
+</Project>+
\ No newline at end of file
diff --git a/src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj.filters b/src/AppInstallerSharedLib/AppInstallerSharedLib.vcxproj.filters
@@ -140,6 +140,9 @@
<ClInclude Include="Public\winget\ModuleCountBase.h">
<Filter>Public\winget</Filter>
</ClInclude>
+ <ClInclude Include="Public\winget\COMStaticStorage.h">
+ <Filter>Public\winget</Filter>
+ </ClInclude>
</ItemGroup>
<ItemGroup>
<ClCompile Include="pch.cpp">
@@ -229,6 +232,9 @@
<ClCompile Include="SQLiteDynamicStorage.cpp">
<Filter>SQLite</Filter>
</ClCompile>
+ <ClCompile Include="COMStaticStorage.cpp">
+ <Filter>Source Files</Filter>
+ </ClCompile>
</ItemGroup>
<ItemGroup>
<None Include="PropertySheet.props" />
diff --git a/src/AppInstallerSharedLib/COMStaticStorage.cpp b/src/AppInstallerSharedLib/COMStaticStorage.cpp
@@ -0,0 +1,38 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+#include "pch.h"
+#include "Public/winget/COMStaticStorage.h"
+
+namespace AppInstaller::WinRT
+{
+ COMStaticStorageStatics& COMStaticStorageStatics::Instance()
+ {
+ static COMStaticStorageStatics s_instance;
+ return s_instance;
+ }
+
+ void COMStaticStorageStatics::AddStaticStorageItem(const winrt::hstring& name, const winrt::Windows::Foundation::IInspectable& item)
+ {
+ COMStaticStorageStatics& instance = Instance();
+ const winrt::slim_lock_guard lock{ instance.m_lock };
+ winrt::Windows::ApplicationModel::Core::CoreApplication::Properties().Insert(name, item);
+ instance.m_items.emplace(std::wstring{ name });
+ }
+
+ void COMStaticStorageStatics::ResetAll() try
+ {
+ COMStaticStorageStatics& instance = Instance();
+ std::set<std::wstring> localItems;
+
+ {
+ const winrt::slim_lock_guard lock{ instance.m_lock };
+ instance.m_items.swap(localItems);
+ }
+
+ for (const auto& item : localItems)
+ {
+ winrt::Windows::ApplicationModel::Core::CoreApplication::Properties().TryRemove(item);
+ }
+ }
+ CATCH_LOG();
+}
diff --git a/src/AppInstallerSharedLib/Public/winget/COMStaticStorage.h b/src/AppInstallerSharedLib/Public/winget/COMStaticStorage.h
@@ -0,0 +1,83 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+#pragma once
+#include <winrt/Windows.ApplicationModel.Core.h>
+#include <memory>
+#include <shared_mutex>
+#include <set>
+#include <string>
+#include <string_view>
+
+namespace AppInstaller::WinRT
+{
+ // Contains registration for static storage so that they can be cleared.
+ struct COMStaticStorageStatics
+ {
+ // Adds a static storage key to the set of known items.
+ static void AddStaticStorageItem(const winrt::hstring& name, const winrt::Windows::Foundation::IInspectable& item);
+
+ // Removes all known static storage items.
+ static void ResetAll();
+
+ private:
+ COMStaticStorageStatics() = default;
+
+ static COMStaticStorageStatics& Instance();
+
+ winrt::slim_mutex m_lock;
+ std::set<std::wstring> m_items;
+ };
+
+ // https://devblogs.microsoft.com/oldnewthing/20210215-00/?p=104865
+ // Base class for an object that needs to live in the COM static store.
+ // An object needs to use this if it has:
+ // - static lifetime
+ // - references to externally implemented COM objects
+ //
+ // Additionally, it should *not* contain references to WRL counted objects implemented by this module.
+ // If it does, it will prevent the module from being unloaded until COM is uninitialized, which is often never.
+ template <typename DataType>
+ struct COMStaticStorageBase
+ {
+ private:
+ struct DataHolder : public winrt::implements<DataHolder, winrt::Windows::Foundation::IInspectable>
+ {
+ std::shared_ptr<DataType> m_shared{ std::make_shared<DataType>() };
+ };
+
+ std::weak_ptr<DataType> m_weak;
+ winrt::slim_mutex m_lock;
+ winrt::hstring m_name;
+
+ public:
+ COMStaticStorageBase(std::wstring_view name) : m_name(name) {}
+
+ std::shared_ptr<DataType> Get()
+ {
+ {
+ const std::shared_lock lock{ m_lock };
+ if (auto cached = m_weak.lock())
+ {
+ return cached;
+ }
+ }
+
+ auto value = winrt::make_self<DataHolder>();
+
+ const winrt::slim_lock_guard lock{ m_lock };
+ if (auto cached = m_weak.lock())
+ {
+ return cached;
+ }
+
+ COMStaticStorageStatics::AddStaticStorageItem(m_name, value.as<winrt::Windows::Foundation::IInspectable>());
+ m_weak = value->m_shared;
+ return value->m_shared;
+ }
+
+ void Reset()
+ {
+ winrt::Windows::ApplicationModel::Core::CoreApplication::Properties().TryRemove(m_name);
+ }
+ };
+}
diff --git a/src/AppInstallerSharedLib/pch.h b/src/AppInstallerSharedLib/pch.h
@@ -58,8 +58,10 @@
#include <wil/filesystem.h>
#pragma warning( pop )
-#include <wil/cppwinrt.h>
+#include <wil/cppwinrt.h>
+#include <winrt/Windows.ApplicationModel.Core.h>
#include <winrt/Windows.ApplicationModel.Resources.h>
#include <winrt/Windows.Foundation.h>
+#include <winrt/Windows.Foundation.Collections.h>
#include <winrt/Windows.Globalization.h>
#include <winrt/Windows.System.Profile.h>
diff --git a/src/WindowsPackageManager/main.cpp b/src/WindowsPackageManager/main.cpp
@@ -16,6 +16,7 @@
#include <AppInstallerErrors.h>
#include <winget/GroupPolicy.h>
#include <ShutdownMonitoring.h>
+#include <winget/COMStaticStorage.h>
#include <ComClsids.h>
using namespace winrt::Microsoft::Management::Deployment;
@@ -100,12 +101,21 @@ extern "C"
{
try
{
- return ::Microsoft::WRL::Module<::Microsoft::WRL::ModuleType::InProc>::GetModule().Terminate();
- }
- catch (...)
- {
- return false;
+ // The WRL object count is used to track externally visible objects, which largely means objects created with the `wil::details::module_count_wrapper` type wrapper.
+ // Configuration objects use a composition based tracking that is similar in nature (only when OOP).
+ //
+ // In-proc DllCanUnloadNow should not be blocked by our internal objects, but they must be destroyed on unload or a future reload will attempt to destroy them
+ // and our module may have moved. So when we don't have any more objects that we gave to callers, remove all of our static lifetime objects and indicate
+ // that we can now be unloaded.
+ if (::Microsoft::WRL::Module<::Microsoft::WRL::ModuleType::InProc>::GetModule().Terminate())
+ {
+ AppInstaller::WinRT::COMStaticStorageStatics::ResetAll();
+ return true;
+ }
}
+ catch (...) {}
+
+ return false;
}
WINDOWS_PACKAGE_MANAGER_API WindowsPackageManagerInProcModuleGetClassObject(
