Add diagnostics to hostfxrutil (#951)

src/AspNetCoreModuleV2/AspNetCore/src/applicationinfo.cpp

@@ -191,15 +191,14 @@ APPLICATION_INFO::EnsureApplicationCreated(
     IHttpContext *pHttpContext
 )
 {
-    HRESULT             hr = S_OK;
     IAPPLICATION       *pApplication = NULL;
     STRU                struExeLocation;
-    STACK_STRU(struFileName, 300);  // >MAX_PATH
     STRU                struHostFxrDllLocation;
+    STACK_STRU(struFileName, 300);  // >MAX_PATH
 
     if (m_pApplication != NULL)
     {
-        goto Finished;
+        return S_OK;
     }
 
     if (m_pApplication == NULL)
@@ -207,7 +206,7 @@ APPLICATION_INFO::EnsureApplicationCreated(
         SRWExclusiveLock lock(m_srwLock);
         if (m_pApplication != NULL)
         {
-            goto Finished;
+            return S_OK;
         }
 
         //
@@ -221,27 +220,19 @@ APPLICATION_INFO::EnsureApplicationCreated(
             // FindRequestHandlerAssembly obtains a global lock, but after releasing the lock,
             // there is a period where we could call
 
-            hr = FindRequestHandlerAssembly(struExeLocation);
+            RETURN_IF_FAILED(FindRequestHandlerAssembly(struExeLocation));
-            if (FAILED(hr))
-            {
-                goto Finished;
-            }
 
             if (m_pfnAspNetCoreCreateApplication == NULL)
             {
-                hr = HRESULT_FROM_WIN32(ERROR_INVALID_FUNCTION);
+                RETURN_IF_FAILED(HRESULT_FROM_WIN32(ERROR_INVALID_FUNCTION));
-                goto Finished;
             }
 
-            hr = m_pfnAspNetCoreCreateApplication(m_pServer, pHttpContext, struExeLocation.QueryStr(), &pApplication);
+            RETURN_IF_FAILED(m_pfnAspNetCoreCreateApplication(m_pServer, pHttpContext, struExeLocation.QueryStr(), &pApplication));
-
             m_pApplication = pApplication;
         }
     }
 
-Finished:
+    return S_OK;
-
-    return hr;
 }
 
 HRESULT
@@ -288,20 +279,17 @@ APPLICATION_INFO::FindRequestHandlerAssembly(STRU& location)
             {
                 std::unique_ptr<HOSTFXR_OPTIONS> options;
 
-                if (FAILED(hr = HOSTFXR_OPTIONS::Create(
+                FINISHED_IF_FAILED(HOSTFXR_OPTIONS::Create(
                     NULL,
                     m_pConfiguration->QueryProcessPath()->QueryStr(),
                     m_pConfiguration->QueryApplicationPhysicalPath()->QueryStr(),
                     m_pConfiguration->QueryArguments()->QueryStr(),
                     g_hEventLog,
-                    options)))
+                    options));
-                {
-                    goto Finished;
-                }
 
-                location.Copy(options->GetExeLocation());
+                FINISHED_IF_FAILED(location.Copy(options->GetExeLocation()));
 
-                if (FAILED(hr = FindNativeAssemblyFromHostfxr(options.get(), pstrHandlerDllName, &struFileName)))
+                if (FAILED_LOG(hr = FindNativeAssemblyFromHostfxr(options.get(), pstrHandlerDllName, &struFileName)))
                 {
                     UTILITY::LogEventF(g_hEventLog,
                             EVENTLOG_ERROR_TYPE,
@@ -314,7 +302,7 @@ APPLICATION_INFO::FindRequestHandlerAssembly(STRU& location)
             }
             else
             {
-                if (FAILED(hr = FindNativeAssemblyFromGlobalLocation(pstrHandlerDllName, &struFileName)))
+                if (FAILED_LOG(hr = FindNativeAssemblyFromGlobalLocation(pstrHandlerDllName, &struFileName)))
                 {
                     UTILITY::LogEventF(g_hEventLog,
                         EVENTLOG_ERROR_TYPE,
@@ -326,7 +314,7 @@ APPLICATION_INFO::FindRequestHandlerAssembly(STRU& location)
                 }
             }
 
-            WDebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, L"Loading request handler: %s", struFileName.QueryStr());
+            WLOG_INFOF(L"Loading request handler: %s", struFileName.QueryStr());
 
             g_hAspnetCoreRH = LoadLibraryW(struFileName.QueryStr());
 

src/AspNetCoreModuleV2/AspNetCore/src/proxymodule.cxx

@@ -97,8 +97,8 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler(
             &m_pApplicationInfo);
         if (FAILED(hr))
         {
-        goto Finished;
+            goto Finished;
-    }
+        }
 
     if (!m_pApplicationInfo->QueryAllowStart())
     {
@@ -172,7 +172,7 @@ ASPNET_CORE_PROXY_MODULE::OnExecuteRequestHandler(
     }
     catch (...)
     {
-        hr = CaughtExceptionHResult();
+        hr = OBSERVE_CAUGHT_EXCEPTION();
     }
 
 Finished:
@@ -214,8 +214,7 @@ ASPNET_CORE_PROXY_MODULE::OnAsyncCompletion(
     }
     catch (...)
     {
-        // Log exception
+        OBSERVE_CAUGHT_EXCEPTION();
-        CaughtExceptionHResult();
         return RQ_NOTIFICATION_FINISH_REQUEST;
     }
 }

src/AspNetCoreModuleV2/CommonLib/config_utility.h

@@ -17,90 +17,38 @@ public:
     HRESULT
     FindHandlerVersion(IAppHostElement* pElement, STRU* strHandlerVersionValue)
     {
-        HRESULT hr = S_OK;
+        HRESULT hr;
-        IAppHostElement                *pHandlerSettings = NULL;
+        CComPtr<IAppHostElement>           pHandlerSettings = nullptr;
-        IAppHostElementCollection      *pHandlerSettingsCollection = NULL;
+        CComPtr<IAppHostElementCollection> pHandlerSettingsCollection = nullptr;
-        ENUM_INDEX                      index;
+        CComPtr<IAppHostElement>           pHandlerVar = nullptr;
-        IAppHostElement                *pHandlerVar = NULL;
+        ENUM_INDEX                         index {};
         STRU strHandlerName;
         STRU strHandlerValue;
 
-        hr = GetElementChildByName(pElement,
+        RETURN_IF_FAILED(GetElementChildByName(pElement, CS_ASPNETCORE_HANDLER_SETTINGS,&pHandlerSettings));
-            CS_ASPNETCORE_HANDLER_SETTINGS,
+        RETURN_IF_FAILED(pHandlerSettings->get_Collection(&pHandlerSettingsCollection));
-            &pHandlerSettings);
+
-        if (FAILED(hr))
+        RETURN_IF_FAILED(hr = FindFirstElement(pHandlerSettingsCollection, &index, &pHandlerVar));
+
+        while (hr != S_FALSE)
         {
-            goto Finished;
+            RETURN_IF_FAILED(GetElementStringProperty(pHandlerVar, CS_ASPNETCORE_HANDLER_SETTINGS_NAME, &strHandlerName));
-        }
+            RETURN_IF_FAILED(GetElementStringProperty(pHandlerVar, CS_ASPNETCORE_HANDLER_SETTINGS_VALUE, &strHandlerValue));
-
-        hr = pHandlerSettings->get_Collection(&pHandlerSettingsCollection);
-        if (FAILED(hr))
-        {
-            goto Finished;
-        }
-
-        for (hr = FindFirstElement(pHandlerSettingsCollection, &index, &pHandlerVar);
-            SUCCEEDED(hr);
-            hr = FindNextElement(pHandlerSettingsCollection, &index, &pHandlerVar))
-        {
-            if (hr == S_FALSE)
-            {
-                hr = S_OK;
-                break;
-            }
-
-            hr = GetElementStringProperty(pHandlerVar,
-                CS_ASPNETCORE_HANDLER_SETTINGS_NAME,
-                &strHandlerName);
-
-            if (FAILED(hr))
-            {
-                goto Finished;
-            }
-
-            hr = GetElementStringProperty(pHandlerVar,
-                CS_ASPNETCORE_HANDLER_SETTINGS_VALUE,
-                &strHandlerValue);
-
-            if (FAILED(hr))
-            {
-                goto Finished;
-
-            }
 
             if (strHandlerName.Equals(CS_ASPNETCORE_HANDLER_VERSION, TRUE))
             {
-                hr = strHandlerVersionValue->Copy(strHandlerValue);
+                RETURN_IF_FAILED(strHandlerVersionValue->Copy(strHandlerValue));
-                goto Finished;
+                break;
             }
 
             strHandlerName.Reset();
             strHandlerValue.Reset();
+            pHandlerVar.Release();
 
-            pHandlerVar->Release();
+            RETURN_IF_FAILED(hr = FindNextElement(pHandlerSettingsCollection, &index, &pHandlerVar));
-            pHandlerVar = NULL;
-        }
-    Finished:
-
-        if (pHandlerVar != NULL)
-        {
-            pHandlerVar->Release();
-            pHandlerVar = NULL;
         }
 
-        if (pHandlerSettingsCollection != NULL)
+        return S_OK;
-        {
-            pHandlerSettingsCollection->Release();
-            pHandlerSettingsCollection = NULL;
-        }
-
-        if (pHandlerSettings != NULL)
-        {
-            pHandlerSettings->Release();
-            pHandlerSettings = NULL;
-        }
-
-        return hr;
     }
 };
 

src/AspNetCoreModuleV2/CommonLib/debugutil.cpp

@@ -132,7 +132,7 @@ DebugPrintf(
 VOID
 WDebugPrintf(
     DWORD   dwFlag,
-    LPWSTR  szFormat,
+    LPCWSTR   szFormat,
     ...
     )
 {

src/AspNetCoreModuleV2/CommonLib/debugutil.h

@@ -10,6 +10,18 @@
 #define ASPNETCORE_DEBUG_FLAG_ERROR         DEBUG_FLAG_ERROR
 #define ASPNETCORE_DEBUG_FLAG_CONSOLE       0x00000008
 
+#define LOG_INFO(...) DebugPrint(ASPNETCORE_DEBUG_FLAG_INFO, __VA_ARGS__)
+#define LOG_INFOF(...) DebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, __VA_ARGS__)
+#define WLOG_INFOF(...) WDebugPrintf(ASPNETCORE_DEBUG_FLAG_INFO, __VA_ARGS__)
+
+#define LOG_WARN(...) DebugPrint(ASPNETCORE_DEBUG_FLAG_WARNING, __VA_ARGS__)
+#define LOG_WARNF(...) DebugPrintf(ASPNETCORE_DEBUG_FLAG_WARNING, __VA_ARGS__)
+#define WLOG_WARNF(...) WDebugPrintf(ASPNETCORE_DEBUG_FLAG_WARNING, __VA_ARGS__)
+
+#define LOG_ERROR(...) DebugPrint(ASPNETCORE_DEBUG_FLAG_ERROR, __VA_ARGS__)
+#define LOG_ERRORF(...) DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, __VA_ARGS__)
+#define WLOG_ERRORF(...) WDebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, __VA_ARGS__)
+
 VOID
 DebugInitialize();
 
@@ -34,6 +46,6 @@ DebugPrintf(
 VOID
 WDebugPrintf(
     DWORD   dwFlag,
-    LPWSTR  szFormat,
+    LPCWSTR  szFormat,
     ...
     );

src/AspNetCoreModuleV2/CommonLib/exceptions.h

@@ -7,17 +7,54 @@
 
 #include "debugutil.h"
 
-inline VOID ReportUntypedException()
+#define LOCATION_INFO_ENABLED TRUE
+
+#if LOCATION_INFO_ENABLED
+#define LOCATION_FORMAT                                         "%s:%d "
+#define LOCATION_ARGUMENTS_ONLY                                 _In_opt_ PCSTR fileName, unsigned int lineNumber
+#define LOCATION_ARGUMENTS                                      LOCATION_ARGUMENTS_ONLY,
+#define LOCATION_CALL_ONLY                                      fileName, lineNumber
+#define LOCATION_CALL                                           LOCATION_CALL_ONLY,
+#define LOCATION_INFO                                           __FILE__, __LINE__
+#else
+#define LOCATION_FORMAT
+#define LOCATION_ARGUMENTS_ONLY
+#define LOCATION_ARGUMENTS
+#define LOCATION_CALL_ONLY
+#define LOCATION_CALL
+#define LOCATION_INFO
+#endif
+
+#define OBSERVE_CAUGHT_EXCEPTION()                              CaughtExceptionHResult(LOCATION_INFO);
+#define RETURN_CAUGHT_EXCEPTION()                               return CaughtExceptionHResult(LOCATION_INFO);
+#define CATCH_RETURN()                                          catch (...) { RETURN_CAUGHT_EXCEPTION(); }
+#define THROW_IF_NULL_ALLOC(ptr)                                Throw_IfNullAlloc(ptr)
+#define RETURN_IF_FAILED(hr)                                    do { HRESULT __hrRet = hr; if (FAILED(__hrRet)) { LogHResultFailed(LOCATION_INFO, __hrRet); return __hrRet; }} while (0, 0)
+#define FINISHED_IF_FAILED(hrr)                                 do { HRESULT __hrRet = hrr; if (FAILED(__hrRet)) { LogHResultFailed(LOCATION_INFO, __hrRet); hr = __hrRet; goto Finished; }} while (0, 0)
+#define LOG_IF_FAILED(hr)                                       LogHResultFailed(LOCATION_INFO, hr)
+#define SUCCEEDED_LOG(hr)                                       SUCCEEDED(LOG_IF_FAILED(hr))
+#define FAILED_LOG(hr)                                          FAILED(LOG_IF_FAILED(hr))
+
+ __declspec(noinline) inline VOID ReportUntypedException(LOCATION_ARGUMENTS_ONLY)
 {
-    DebugPrint(ASPNETCORE_DEBUG_FLAG_ERROR, "Unhandled non-standard exception");
+    DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, LOCATION_FORMAT "Unhandled non-standard exception", LOCATION_CALL_ONLY);
 }
 
-inline VOID ReportException(std::exception& exception)
+ __declspec(noinline) inline VOID ReportException(LOCATION_ARGUMENTS std::exception& exception)
 {
-    DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, "Unhandled exception: %s", exception.what());
+    DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR, LOCATION_FORMAT "Unhandled exception: %s", LOCATION_CALL exception.what());
 }
 
-inline __declspec(noinline) HRESULT CaughtExceptionHResult()
+ __declspec(noinline) inline HRESULT LogHResultFailed(LOCATION_ARGUMENTS HRESULT hr)
+{
+    if (FAILED(hr))
+    {
+        DebugPrintf(ASPNETCORE_DEBUG_FLAG_ERROR,  "Failed HRESULT returned: 0x%x at " LOCATION_FORMAT, hr, LOCATION_CALL_ONLY);
+    }
+    return hr;
+}
+
+__declspec(noinline) inline HRESULT CaughtExceptionHResult(LOCATION_ARGUMENTS_ONLY)
 {
     try
     {
@@ -29,17 +66,17 @@ inline __declspec(noinline) HRESULT CaughtExceptionHResult()
     }
     catch (std::system_error& exception)
     {
-        ReportException(exception);
+        ReportException(LOCATION_CALL exception);
         return exception.code().value();
     }
     catch (std::exception& exception)
     {
-        ReportException(exception);
+        ReportException(LOCATION_CALL exception);
         return HRESULT_FROM_WIN32(ERROR_UNHANDLED_EXCEPTION);
     }
     catch (...)
     {
-        ReportUntypedException();
+        ReportUntypedException(LOCATION_CALL_ONLY);
         return HRESULT_FROM_WIN32(ERROR_UNHANDLED_EXCEPTION);
     }
 }
@@ -52,8 +89,3 @@ template <typename PointerT> auto Throw_IfNullAlloc(PointerT pointer)
     }
     return pointer;
 }
-
-
-#define RETURN_CAUGHT_EXCEPTION()                               return CaughtExceptionHResult();
-#define CATCH_RETURN()                                          catch (...) { RETURN_CAUGHT_EXCEPTION(); }
-#define THROW_IF_NULL_ALLOC(ptr)                                Throw_IfNullAlloc(ptr)

src/AspNetCoreModuleV2/CommonLib/hostfxr_utility.cpp

@@ -29,23 +29,31 @@ HOSTFXR_UTILITY::GetStandaloneHostfxrParameters(
         _Out_ BSTR**		ppwzArgv
 )
 {
-    HRESULT hr = S_OK;
+    WLOG_INFOF(L"Resolving standalone hostfxr parameters for application: %s arguments: %s path: %s",
+        pwzExeAbsolutePath,
+        pcwzArguments,
+        pwzExeAbsolutePath);
 
     const fs::path exePath(pwzExeAbsolutePath);
 
     if (!exePath.has_extension())
     {
+        WLOG_INFOF(L"Exe path has not extension, returning");
+
         return false;
     }
 
     const fs::path physicalPath(pcwzApplicationPhysicalPath);
     const fs::path hostFxrLocation = physicalPath / "hostfxr.dll";
 
+    WLOG_INFOF(L"Checking hostfxr.dll at %s", hostFxrLocation.c_str());
+
     if (!is_regular_file(hostFxrLocation))
     {
         fs::path runtimeConfigLocation = exePath;
         runtimeConfigLocation.replace_extension(L".runtimeconfig.json");
 
+        WLOG_INFOF(L"Checking runtimeconfig.json at %s", runtimeConfigLocation.c_str());
         if (!is_regular_file(runtimeConfigLocation))
         {
             EVENTLOG(hEventLog, INPROCESS_FULL_FRAMEWORK_APP, pcwzApplicationPhysicalPath, 0);
@@ -61,23 +69,23 @@ HOSTFXR_UTILITY::GetStandaloneHostfxrParameters(
 
     if (!is_regular_file(dllPath))
     {
+        WLOG_INFOF(L"Application dll at %s was not found", dllPath.c_str());
         return E_FAIL;
     }
 
     auto arguments = std::wstring(dllPath) + L" " + pcwzArguments;
 
-    if (FAILED(hr = pStruHostFxrDllLocation->Copy(hostFxrLocation.c_str())))
+    RETURN_IF_FAILED(pStruHostFxrDllLocation->Copy(hostFxrLocation.c_str()));
-    {
-        return hr;
-    }
 
-    return ParseHostfxrArguments(
+    RETURN_IF_FAILED(ParseHostfxrArguments(
         arguments.c_str(),
         pwzExeAbsolutePath,
         pcwzApplicationPhysicalPath,
         hEventLog,
         pdwArgCount,
-        ppwzArgv);
+        ppwzArgv));
+
+    return S_OK;
 }
 
 BOOL
@@ -102,6 +110,11 @@ HOSTFXR_UTILITY::GetHostFxrParameters(
 {
     HRESULT                     hr = S_OK;
 
+    WLOG_INFOF(L"Resolving hostfxr parameters for application: %s arguments: %s path: %s",
+        pcwzProcessPath,
+        pcwzArguments,
+        pcwzApplicationPhysicalPath);
+
     const fs::path applicationPhysicalPath = pcwzApplicationPhysicalPath;
     fs::path processPath = ExpandEnvironmentVariables(pcwzProcessPath);
     std::wstring arguments = ExpandEnvironmentVariables(pcwzArguments);
@@ -114,6 +127,7 @@ HOSTFXR_UTILITY::GetHostFxrParameters(
     // Check if the absolute path is to dotnet or not.
     if (IsDotnetExecutable(processPath))
     {
+        WLOG_INFOF(L"Process path %s is dotnet, treating application as portable", processPath.c_str());
         //
         // The processPath ends with dotnet.exe or dotnet
         // like: C:\Program Files\dotnet\dotnet.exe, C:\Program Files\dotnet\dotnet, dotnet.exe, or dotnet.
@@ -134,29 +148,21 @@ HOSTFXR_UTILITY::GetHostFxrParameters(
             return E_FAIL;
         }
 
-        if (FAILED(hr = HOSTFXR_UTILITY::ParseHostfxrArguments(
+        RETURN_IF_FAILED(HOSTFXR_UTILITY::ParseHostfxrArguments(
             arguments.c_str(),
             processPath.c_str(),
             pcwzApplicationPhysicalPath,
             hEventLog,
             pdwArgCount,
-            pbstrArgv)))
+            pbstrArgv));
-        {
-            return hr;
-        }
 
-        if (FAILED(hr = pStruHostFxrDllLocation->Copy(hostFxrPath->c_str())))
+        RETURN_IF_FAILED(pStruHostFxrDllLocation->Copy(hostFxrPath->c_str()));
-        {
+        RETURN_IF_FAILED(pStruExeAbsolutePath->Copy(processPath.c_str()));
-            return hr;
-        }
-
-        if (FAILED(hr = pStruExeAbsolutePath->Copy(processPath.c_str())))
-        {
-            return hr;
-        }
     }
     else
     {
+        WLOG_INFOF(L"Process path %s is not dotnet, treating application as standalone", processPath.c_str());
+
         //
         // The processPath is a path to the application executable
         // like: C:\test\MyApp.Exe or MyApp.Exe
@@ -164,23 +170,16 @@ HOSTFXR_UTILITY::GetHostFxrParameters(
         //
         if (is_regular_file(processPath))
         {
-            if (FAILED(hr = GetStandaloneHostfxrParameters(
+            RETURN_IF_FAILED(GetStandaloneHostfxrParameters(
                 processPath.c_str(),
                 pcwzApplicationPhysicalPath,
                 arguments.c_str(),
                 hEventLog,
                 pStruHostFxrDllLocation,
                 pdwArgCount,
-                pbstrArgv)))
+                pbstrArgv));
-            {
-                return hr;
-            }
-
-            if (FAILED(hr = pStruExeAbsolutePath->Copy(processPath.c_str())))
-            {
-                return hr;
-            }
 
+            RETURN_IF_FAILED(pStruExeAbsolutePath->Copy(processPath.c_str()));
         }
         else
         {
@@ -189,7 +188,7 @@ HOSTFXR_UTILITY::GetHostFxrParameters(
             // then it is an invalid argument.
             //
             hr = HRESULT_FROM_WIN32(ERROR_FILE_NOT_FOUND);
-            UTILITY::LogEventF(hEventLog, ASPNETCORE_EVENT_INVALID_PROCESS_PATH_LEVEL, ASPNETCORE_EVENT_INVALID_PROCESS_PATH, ASPNETCORE_EVENT_INVALID_PROCESS_PATH_MSG, processPath.c_str(), hr);
+            EVENTLOG(hEventLog, INVALID_PROCESS_PATH, processPath.c_str(), hr);
             return hr;
         }
     }
@@ -228,6 +227,11 @@ HOSTFXR_UTILITY::ParseHostfxrArguments(
     STRU        struTempPath;
     INT         intArgsProcessed = 0;
 
+    WLOG_INFOF(L"Resolving hostfxr_main arguments application: %s arguments: %s path: %s",
+        pwzExePath,
+        pwzArgumentsFromConfig,
+        pcwzApplicationPhysicalPath);
+
     // If we call CommandLineToArgvW with an empty string, argc is 5 for some interesting reason.
     // Protectively guard against this by check if the string is null or empty.
     if (pwzArgumentsFromConfig == NULL || wcscmp(pwzArgumentsFromConfig, L"") == 0)
@@ -240,7 +244,7 @@ HOSTFXR_UTILITY::ParseHostfxrArguments(
 
     if (pwzArgs == NULL)
     {
-        hr = HRESULT_FROM_WIN32(GetLastError());
+        hr = LOG_IF_FAILED(HRESULT_FROM_WIN32(GetLastError()));
         goto Failure;
     }
 
@@ -283,6 +287,10 @@ HOSTFXR_UTILITY::ParseHostfxrArguments(
                 goto Failure;
             }
         }
+
+        WLOG_INFOF(L"Argument[%d] = %s",
+            intArgsProcessed + 1,
+            argv[intArgsProcessed + 1]);
     }
 
     *pbstrArgv = argv;
@@ -317,11 +325,15 @@ HOSTFXR_UTILITY::GetAbsolutePathToDotnet(
      const fs::path & requestedPath
 )
 {
+    WLOG_INFOF(L"Resolving absolute path do dotnet.exe from %s", requestedPath.c_str());
+
     //
     // If we are given an absolute path to dotnet.exe, we are done
     //
     if (is_regular_file(requestedPath))
     {
+        WLOG_INFOF(L"Found dotnet.exe at %s", requestedPath.c_str());
+
         return std::make_optional(requestedPath);
     }
 
@@ -330,6 +342,8 @@ HOSTFXR_UTILITY::GetAbsolutePathToDotnet(
 
     if (is_regular_file(pathWithExe))
     {
+        WLOG_INFOF(L"Found dotnet.exe at %s", pathWithExe.c_str());
+
         return std::make_optional(pathWithExe);
     }
 
@@ -345,10 +359,21 @@ HOSTFXR_UTILITY::GetAbsolutePathToDotnet(
     const auto dotnetViaWhere = InvokeWhereToFindDotnet();
     if (dotnetViaWhere.has_value())
     {
+        WLOG_INFOF(L"Found dotnet.exe wia where.exe invocation at %s", dotnetViaWhere.value().c_str());
+
         return dotnetViaWhere;
     }
 
-    return GetAbsolutePathToDotnetFromProgramFiles();
+    const auto programFilesLocation = GetAbsolutePathToDotnetFromProgramFiles();
+    if (programFilesLocation.has_value())
+    {
+        WLOG_INFOF(L"Found dotnet.exe in Program Files at %s", programFilesLocation.value().c_str());
+
+        return programFilesLocation;
+    }
+
+    WLOG_INFOF(L"dotnet.exe not found");
+    return std::nullopt;
 }
 
 std::optional<fs::path>
@@ -360,6 +385,8 @@ HOSTFXR_UTILITY::GetAbsolutePathToHostFxr(
     std::vector<std::wstring> versionFolders;
     const auto hostFxrBase = dotnetPath.parent_path() / "host" / "fxr";
 
+    WLOG_INFOF(L"Resolving absolute path do hostfxr.dll from %s", dotnetPath.c_str());
+
     if (!is_directory(hostFxrBase))
     {
         EVENTLOG(hEventLog, HOSTFXR_DIRECTORY_NOT_FOUND, hostFxrBase.c_str(), HRESULT_FROM_WIN32(ERROR_BAD_ENVIRONMENT));
@@ -387,6 +414,7 @@ HOSTFXR_UTILITY::GetAbsolutePathToHostFxr(
         return std::nullopt;
     }
 
+    WLOG_INFOF(L"hostfxr.dll located at %s", hostFxrPath.c_str());
     return std::make_optional(hostFxrPath);
 }
 
@@ -643,8 +671,7 @@ HOSTFXR_UTILITY::FindDotNetFolders(
 
     do
     {
-        std::wstring folder(data.cFileName);
+        pvFolders.emplace_back(data.cFileName);
-        pvFolders.push_back(folder);
     } while (FindNextFileW(handle, &data));
 
     FindClose(handle);

src/AspNetCoreModuleV2/CommonLib/hostfxroptions.cpp

@@ -11,22 +11,21 @@ HRESULT HOSTFXR_OPTIONS::Create(
         _In_ HANDLE         hEventLog,
         _Out_ std::unique_ptr<HOSTFXR_OPTIONS>& ppWrapper)
 {
-    HRESULT hr = S_OK;
     STRU struHostFxrDllLocation;
     STRU struExeAbsolutePath;
     STRU struExeLocation;
     BSTR* pwzArgv;
     DWORD dwArgCount;
 
-    if (pcwzExeLocation != NULL && FAILED(struExeLocation.Copy(pcwzExeLocation)))
+    if (pcwzExeLocation != NULL)
     {
-        goto Finished;
+        RETURN_IF_FAILED(struExeLocation.Copy(pcwzExeLocation));
     }
 
     // If the exe was not provided by the shim, reobtain the hostfxr parameters (which finds dotnet).
     if (struExeLocation.IsEmpty())
     {
-        if (FAILED(hr = HOSTFXR_UTILITY::GetHostFxrParameters(
+        RETURN_IF_FAILED(HOSTFXR_UTILITY::GetHostFxrParameters(
             hEventLog,
             pcwzProcessPath,
             pcwzApplicationPhysicalPath,
@@ -34,48 +33,34 @@ HRESULT HOSTFXR_OPTIONS::Create(
             &struHostFxrDllLocation,
             &struExeAbsolutePath,
             &dwArgCount,
-            &pwzArgv)))
+            &pwzArgv));
-        {
-            goto Finished;
-        }
     }
     else if (HOSTFXR_UTILITY::IsDotnetExecutable(struExeLocation.QueryStr()))
     {
-        if (FAILED(hr = HOSTFXR_UTILITY::ParseHostfxrArguments(
+        RETURN_IF_FAILED(HOSTFXR_UTILITY::ParseHostfxrArguments(
             pcwzArguments,
             pcwzExeLocation,
             pcwzApplicationPhysicalPath,
             hEventLog,
             &dwArgCount,
-            &pwzArgv)))
+            &pwzArgv));
-        {
-            goto Finished;
-        }
     }
     else
     {
-        if (FAILED(hr = HOSTFXR_UTILITY::GetStandaloneHostfxrParameters(
+        RETURN_IF_FAILED(HOSTFXR_UTILITY::GetStandaloneHostfxrParameters(
             pcwzExeLocation,
             pcwzApplicationPhysicalPath,
             pcwzArguments,
             hEventLog,
             &struHostFxrDllLocation,
             &dwArgCount,
-            &pwzArgv)))
+            &pwzArgv));
-        {
-            goto Finished;
-        }
     }
 
     ppWrapper = std::make_unique<HOSTFXR_OPTIONS>();
-    if (FAILED(hr = ppWrapper->Populate(struHostFxrDllLocation.QueryStr(), struExeAbsolutePath.QueryStr(), dwArgCount, pwzArgv)))
+    RETURN_IF_FAILED(ppWrapper->Populate(struHostFxrDllLocation.QueryStr(), struExeAbsolutePath.QueryStr(), dwArgCount, pwzArgv));
-    {
-        goto Finished;
-    }
 
-Finished:
+    return S_OK;
-
-    return hr;
 }
 
 

src/AspNetCoreModuleV2/InProcessRequestHandler/inprocessapplication.cpp

@@ -557,7 +557,7 @@ IN_PROCESS_APPLICATION::ExecuteApplication(
     VOID
 )
 {
-    HRESULT             hr = S_OK;
+    HRESULT             hr;
     HMODULE             hModule = nullptr;
     DWORD               hostfxrArgc = 0;
     BSTR               *hostfxrArgv = NULL;
@@ -576,7 +576,7 @@ IN_PROCESS_APPLICATION::ExecuteApplication(
         if (hModule == NULL)
         {
             // .NET Core not installed (we can log a more detailed error message here)
-            hr = ERROR_BAD_ENVIRONMENT;
+            hr = LOG_IF_FAILED(ERROR_BAD_ENVIRONMENT);
             goto Finished;
         }
 
@@ -584,29 +584,23 @@ IN_PROCESS_APPLICATION::ExecuteApplication(
         pProc = (hostfxr_main_fn)GetProcAddress(hModule, "hostfxr_main");
         if (pProc == NULL)
         {
-            hr = ERROR_BAD_ENVIRONMENT;
+            hr = LOG_IF_FAILED(ERROR_BAD_ENVIRONMENT);
             goto Finished;
         }
 
-        if (FAILED(hr = HOSTFXR_OPTIONS::Create(
+        FINISHED_IF_FAILED(hr = HOSTFXR_OPTIONS::Create(
             m_struExeLocation.QueryStr(),
             m_pConfig->QueryProcessPath()->QueryStr(),
             m_pConfig->QueryApplicationPhysicalPath()->QueryStr(),
             m_pConfig->QueryArguments()->QueryStr(),
             g_hEventLog,
             hostFxrOptions
-            )))
+            ));
-        {
-            goto Finished;
-        }
 
         hostfxrArgc = hostFxrOptions->GetArgc();
         hostfxrArgv = hostFxrOptions->GetArgv();
 
-        if (FAILED(hr = SetEnvironementVariablesOnWorkerProcess()))
+        FINISHED_IF_FAILED(SetEnvironementVariablesOnWorkerProcess());
-        {
-            goto Finished;
-        }
     }
 
     // There can only ever be a single instance of .NET Core

test/CommonLibTests/ConfigUtilityTests.cpp

@@ -18,8 +18,6 @@ namespace ConfigUtilityTests
             IAppHostElement* retElement = NULL;
 
             STRU handlerVersion;
-            BSTR bstrKey = SysAllocString(key.c_str());
-            BSTR bstrValue = SysAllocString(value.c_str());
 
             // NiceMock removes warnings about "uninteresting calls",
             auto element = std::make_unique<NiceMock<MockElement>>();
@@ -39,15 +37,13 @@ namespace ConfigUtilityTests
             ON_CALL(*nameElement, GetPropertyByName(_, _))
                 .WillByDefault(DoAll(testing::SetArgPointee<1>(mockProperty.get()), testing::Return(S_OK)));
             EXPECT_CALL(*mockProperty, get_StringValue(_))
-                .WillOnce(DoAll(testing::SetArgPointee<0>(bstrKey), testing::Return(S_OK)))
+                .WillOnce(DoAll(testing::SetArgPointee<0>(SysAllocString(key.c_str())), testing::Return(S_OK)))
-                .WillOnce(DoAll(testing::SetArgPointee<0>(bstrValue), testing::Return(S_OK)));
+                .WillOnce(DoAll(testing::SetArgPointee<0>(SysAllocString(value.c_str())), testing::Return(S_OK)));
 
             HRESULT hr = ConfigUtility::FindHandlerVersion(element.get(), &handlerVersion);
 
+            EXPECT_EQ(hr, S_OK);
             EXPECT_STREQ(handlerVersion.QueryStr(), expected.c_str());
-
-            SysFreeString(bstrKey);
-            SysFreeString(bstrValue);
         }
     };
 
@@ -66,11 +62,6 @@ namespace ConfigUtilityTests
         IAppHostElement* retElement = NULL;
         STRU handlerVersion;
 
-        BSTR bstrKey = SysAllocString(L"key");
-        BSTR bstrValue = SysAllocString(L"value");
-        BSTR bstrKey2 = SysAllocString(L"handlerVersion");
-        BSTR bstrValue2  = SysAllocString(L"value2");
-
         auto element = std::make_unique<NiceMock<MockElement>>();
         auto innerElement = std::make_unique<NiceMock<MockElement>>();
         auto collection = std::make_unique<NiceMock<MockCollection>>();
@@ -88,18 +79,14 @@ namespace ConfigUtilityTests
         ON_CALL(*nameElement, GetPropertyByName(_, _))
             .WillByDefault(DoAll(testing::SetArgPointee<1>(mockProperty.get()), testing::Return(S_OK)));
         EXPECT_CALL(*mockProperty, get_StringValue(_))
-            .WillOnce(DoAll(testing::SetArgPointee<0>(bstrKey), testing::Return(S_OK)))
+            .WillOnce(DoAll(testing::SetArgPointee<0>(SysAllocString(L"key")), testing::Return(S_OK)))
-            .WillOnce(DoAll(testing::SetArgPointee<0>(bstrValue), testing::Return(S_OK)))
+            .WillOnce(DoAll(testing::SetArgPointee<0>(SysAllocString(L"value")), testing::Return(S_OK)))
-            .WillOnce(DoAll(testing::SetArgPointee<0>(bstrKey2), testing::Return(S_OK)))
+            .WillOnce(DoAll(testing::SetArgPointee<0>(SysAllocString(L"handlerVersion")), testing::Return(S_OK)))
-            .WillOnce(DoAll(testing::SetArgPointee<0>(bstrValue2), testing::Return(S_OK)));
+            .WillOnce(DoAll(testing::SetArgPointee<0>(SysAllocString(L"value2")), testing::Return(S_OK)));
 
         HRESULT hr = ConfigUtility::FindHandlerVersion(element.get(), &handlerVersion);
 
+        EXPECT_EQ(hr, S_OK);
         EXPECT_STREQ(handlerVersion.QueryStr(), L"value2");
-
-        SysFreeString(bstrKey);
-        SysFreeString(bstrValue);
-        SysFreeString(bstrKey2);
-        SysFreeString(bstrValue2);
     }
 }

test/CommonLibTests/inprocess_application_tests.cpp

@@ -8,8 +8,6 @@
 // Externals defined in inprocess
 BOOL       g_fProcessDetach;
 HANDLE     g_hEventLog;
-PCSTR g_szDebugLabel;
-DWORD g_dwDebugFlags;
 
 namespace InprocessTests
 {

test/CommonLibTests/main.cpp

@@ -3,6 +3,8 @@
 
 #include "stdafx.h"
 
+DECLARE_DEBUG_PRINT_OBJECT2("tests", ASPNETCORE_DEBUG_FLAG_INFO | ASPNETCORE_DEBUG_FLAG_CONSOLE);
+
 #if defined(_WIN32)
 int wmain(int argc, wchar_t* argv[])
 #else