Improve COM inproc initialization

src/AppInstallerCLICore/COMContext.cpp

@@ -86,21 +86,23 @@ namespace AppInstaller::CLI::Execution
         Logging::Log().SetEnabledChannels(channel.has_value() ? channel.value() : Settings::User().Get<Settings::Setting::LoggingChannelPreference>());
         Logging::Log().SetLevel(level.has_value() ? level.value() : Settings::User().Get<Settings::Setting::LoggingLevelPreference>());
 
-        // TODO: Log to file for COM API calls only when debugging in visual studio
-        Logging::FileLogger::Add(s_comLogFileNamePrefix);
-
-        Logging::OutputDebugStringLogger::Remove();
+        Logging::TraceLogger::Add();
+        Logging::EnableWilFailureTelemetry();
 
+        // File logging must not prevent COM functionality.
+        try
+        {
+            Logging::FileLogger::Add(s_comLogFileNamePrefix);
 #ifndef AICLI_DISABLE_TEST_HOOKS
-        if (!Settings::User().Get<Settings::Setting::KeepAllLogFiles>())
+            if (!Settings::User().Get<Settings::Setting::KeepAllLogFiles>())
 #endif
-        {
-            // Initiate the background cleanup of the log file location.
-            Logging::FileLogger::BeginCleanup();
+            {
+                // Initiate the background cleanup of the log file location.
+                Logging::FileLogger::BeginCleanup();
+            }
         }
+        CATCH_LOG();
 
-        Logging::TraceLogger::Add();
-
-        Logging::EnableWilFailureTelemetry();
+        Logging::OutputDebugStringLogger::Remove();
     }
 }

src/AppInstallerCLIE2ETests/InprocTestbedTests.cs

@@ -6,6 +6,7 @@
 
 namespace AppInstallerCLIE2ETests
 {
+    using System.Collections.Generic;
     using System.IO;
     using System.Reflection;
     using AppInstallerCLIE2ETests.Helpers;
@@ -93,6 +94,30 @@ public void DefaultTest()
             this.RunInprocTestbed(new TestbedParameters() { WorkTestSleepInterval = 1000 });
         }
 
+        /// <summary>
+        /// Verifies that an unusable file logging path does not prevent activation or settings calls.
+        /// </summary>
+        [Test]
+        public void FileLoggingFailureDoesNotPreventActivation()
+        {
+            // An existing file cannot serve as the parent directory for the unpackaged log path.
+            var environmentVariables = new Dictionary<string, string>()
+            {
+                ["TMP"] = Path.GetFullPath(this.InprocTestbedPath),
+                ["TEMP"] = Path.GetFullPath(this.InprocTestbedPath),
+            };
+
+            var result = TestCommon.RunProcess(
+                this.InprocTestbedPath,
+                string.Empty,
+                "-itr 2",
+                null,
+                60000,
+                true,
+                environmentVariables);
+            Assert.That(result.ExitCode, Is.Zero, result.StdOut + result.StdErr);
+        }
+
         /// <summary>
         /// Tests using the CLSID with CoCreateInstance.
         /// </summary>

src/AppInstallerCLITests/FileLogger.cpp

@@ -2,8 +2,11 @@
 // Licensed under the MIT License.
 #include "pch.h"
 #include "TestCommon.h"
+#include "TestHooks.h"
 #include <AppInstallerFileLogger.h>
 #include <AppInstallerStrings.h>
+#include <COMContext.h>
+#include <winget/ThreadGlobals.h>
 #include <future>
 #include <set>
 
@@ -201,6 +204,41 @@ namespace
     }
 }
 
+TEST_CASE("COMFileLogging_InitializationFailureIsNonFatal", "[logging]")
+{
+    TestUserSettings settings;
+    AppInstaller::ThreadLocalStorage::WingetThreadGlobals threadGlobals;
+    auto previousThreadGlobals = threadGlobals.SetForCurrentThread();
+
+    TempFile blocker{ "FileLogger_BlockedParent", ".tmp" };
+    {
+        std::ofstream file{ blocker.GetPath() };
+        file << "Not a directory";
+        REQUIRE(file.good());
+    }
+
+    auto logPath = blocker.GetPath() / "test.log";
+    REQUIRE_THROWS(FileLogger{ logPath });
+    REQUIRE_THROWS(FileLogger::Add(logPath));
+    REQUIRE_FALSE(Log().ContainsLogger(FileLogger::GetNameForPath(logPath)));
+
+    using namespace AppInstaller::Runtime;
+    auto restorePaths = wil::scope_exit([]()
+        {
+            TestHook_ClearPathOverrides();
+            SetTestPathOverrides();
+        });
+    AppInstaller::Filesystem::PathDetails blockedPath;
+    blockedPath.Path = blocker.GetPath() / "logs";
+    blockedPath.Create = GENERATE(false, true);
+    TestHook_SetPathOverride(PathName::DefaultLogLocation, blockedPath);
+
+    REQUIRE_NOTHROW(AppInstaller::CLI::Execution::COMContext::SetLoggers(AppInstaller::Logging::Channel::All, Level::Verbose));
+    REQUIRE_FALSE(Log().ContainsLogger("file"));
+    REQUIRE(Log().ContainsLogger("Trace"));
+    REQUIRE_FALSE(Log().ContainsLogger("OutputDebugStringLogger"));
+}
+
 TEST_CASE("FileLogger_MaximumSize", "[logging]")
 {
     auto tagState = GENERATE(TagState_Unset, TagState_SetAtStart, TagState_SetAfterLogging);

src/Microsoft.Management.Deployment.InProc/dllmain.cpp

@@ -3,23 +3,28 @@
 #include "pch.h"
 #include <WindowsPackageManager.h>
 #include <hstring.h>
+#include <mutex>
+
+namespace
+{
+    HRESULT EnsureInitialized() try
+    {
+        static std::once_flag initializeOnceFlag;
+        std::call_once(initializeOnceFlag,
+            []()
+            {
+                THROW_IF_FAILED(WindowsPackageManagerInProcModuleInitialize());
+            });
+        return S_OK;
+    }
+    CATCH_RETURN();
+}
 
 EXTERN_C BOOL WINAPI DllMain(
     HMODULE /* hModule */,
-    DWORD reason,
+    DWORD /* reason */,
     LPVOID /* lpReserved */)
 {
-    switch (reason)
-    {
-    case DLL_PROCESS_ATTACH:
-    {
-        if (FAILED(WindowsPackageManagerInProcModuleInitialize()))
-        {
-            return FALSE;
-        }
-    }
-    break;
-    }
     return TRUE;
 }
 
@@ -28,6 +33,9 @@ STDAPI DllGetClassObject(
     REFIID riid,
     LPVOID* ppv)
 {
+    RETURN_HR_IF_NULL(E_POINTER, ppv);
+    *ppv = nullptr;
+    RETURN_IF_FAILED(EnsureInitialized());
     RETURN_HR(WindowsPackageManagerInProcModuleGetClassObject(rclsid, riid, ppv));
 }
 
@@ -38,5 +46,8 @@ STDAPI DllCanUnloadNow()
 
 STDAPI DllGetActivationFactory(HSTRING classId, void** factory)
 {
+    RETURN_HR_IF_NULL(E_POINTER, factory);
+    *factory = nullptr;
+    RETURN_IF_FAILED(EnsureInitialized());
     return WindowsPackageManagerInProcModuleGetActivationFactory(classId, factory);
 }

src/WindowsPackageManager/ConfigurationStaticFunctions.cpp

@@ -90,7 +90,11 @@ namespace ConfigurationShim
             auto& diagnosticsLogger = m_threadGlobals.GetDiagnosticLogger();
             diagnosticsLogger.SetEnabledChannels(AppInstaller::Logging::Channel::All);
             diagnosticsLogger.SetLevel(AppInstaller::Logging::Level::Verbose);
-            diagnosticsLogger.AddLogger(std::make_unique<AppInstaller::Logging::FileLogger>("WinGetCFG"sv));
+            try
+            {
+                diagnosticsLogger.AddLogger(std::make_unique<AppInstaller::Logging::FileLogger>("WinGetCFG"sv));
+            }
+            CATCH_LOG();
 
             if (IsConfigurationAvailable())
             {

src/WindowsPackageManager/WindowsPackageManager.h

@@ -34,7 +34,7 @@ extern "C"
     // Creates an out-of-proc instance for manual activation scenarios.
     WINDOWS_PACKAGE_MANAGER_API WindowsPackageManagerServerCreateInstance(REFCLSID rclsid, REFIID riid, void** out);
 
-    // Creates module for in-proc COM invocation.
+    // Initializes the module and CLI infrastructure for in-proc COM invocation. Must be called outside DllMain.
     WINDOWS_PACKAGE_MANAGER_API WindowsPackageManagerInProcModuleInitialize();
 
     // Try to terminate the module for in-proc COM. Returns false if there's still active objects.