Add helpers for implementing Out-Of-Proc COM Server with cppwinrt only (without WRL) (#440)

* Fix tests cmake not working with msvc + msbuild * Initial impl of #439 * Make com server test a separate executable * Fix not able to create non-projected implementation class * Add tests * Fix format * Fix cannot set notifier directly * Add test for defining module lock manually * Rename default class factory Co-authored-by: Duncan Horn <40036384+dunhor@users.noreply.github.com> * Fix not using CppWinRTClassFactory elsewhere * Clear tokens after revoking * Make com_server_revoker non-copyable * Do not record defaulted registration token * Fix revoker not constructible * Remove C++17 guard * Handle server registration failure Align behavior with WRL. Specifically, failing any server registration undo all registrations. Fix revoke can throw, making dtor of com_server_revoker throw (which is bad) * Remove C++/17 guard in tests * De-templatize notifiable_module_lock Make CustomModuleLockTest test user using another lock type * Require user to define WINRT_CUSTOM_MODULE_LOCK Detect and warn users to include notifiable_module_lock _before_ including winrt headers * Fix format * Update include/wil/cppwinrt_notifiable_module_lock.h * Update include/wil/cppwinrt_register_com_server.h * Add cppwinrt-com-server* tests to runtests.cmd * Explain why push_back is safe * Use wil::unique_com_class_object_cookie as revoker * Ensure module lock's count is initialized Co-authored-by: Duncan Horn <40036384+dunhor@users.noreply.github.com> * Remove redundant try-catch It was necessary, but now that we use wil's unique_com_class_cookie we no longer need to catch, clean up and re-throw * Try-catch CreateInstance winrt::hresult doesn't seem to be recognized so it's handled separately * Let wil handle winrt::hresult_error * Add test for register_com_server failure * Fix format * Make notifiable_module_lock singleton #439 * Update activation after module starts waiting test * Add notifiable_module_lock_base --------- Co-authored-by: Duncan Horn <40036384+dunhor@users.noreply.github.com>
2024-03-29 01:53:35 +08:00 · 2024-03-29 01:53:35 +08:00 · 9632635431
--- a/include/wil/common.h
+++ b/include/wil/common.h
@ -810,7 +810,7 @@ _Post_satisfies_(return == error) inline T verify_win32(T error) WI_NOEXCEPT
 // Implementation details for macros and helper functions... do not use directly.
 namespace details
 {
-// Use size-specific casts to avoid sign extending numbers -- avoid warning C4310: cast truncates constant value
+    // Use size-specific casts to avoid sign extending numbers -- avoid warning C4310: cast truncates constant value
 #define __WI_MAKE_UNSIGNED(val) \
    (__pragma(warning(push)) __pragma(warning(disable : 4310 4309))( \
        sizeof(val) == 1   ? static_cast<unsigned char>(val) \
--- a/include/wil/cppwinrt_notifiable_module_lock.h
+++ b/include/wil/cppwinrt_notifiable_module_lock.h
@ -0,0 +1,104 @@
+//*********************************************************
+//
+//    Copyright (c) Microsoft. All rights reserved.
+//    This code is licensed under the MIT License.
+//    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF
+//    ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED
+//    TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A
+//    PARTICULAR PURPOSE AND NONINFRINGEMENT.
+//
+//*********************************************************
+//! @file
+//! Utilities for implementing OOP COM server with cppwinrt only
+
+#ifndef __WIL_CPPWINRT_NOTIFIABLE_MODULE_LOCK_INCLUDED
+#define __WIL_CPPWINRT_NOTIFIABLE_MODULE_LOCK_INCLUDED
+
+#ifdef WINRT_BASE_H
+#error You must include this header before including any winrt header
+#endif
+
+#ifndef WINRT_CUSTOM_MODULE_LOCK
+#error You must define 'WINRT_CUSTOM_MODULE_LOCK' at the project level
+#endif
+
+#include <atomic>
+#include <cstdint>
+
+namespace wil
+{
+// Adopted from cppwinrt
+struct notifiable_module_lock_base
+{
+    notifiable_module_lock_base() = default;
+
+    notifiable_module_lock_base(uint32_t count) : m_count(count)
+    {
+    }
+
+    uint32_t operator=(uint32_t count) noexcept
+    {
+        return m_count = count;
+    }
+
+    uint32_t operator++() noexcept
+    {
+        return m_count.fetch_add(1, std::memory_order_relaxed) + 1;
+    }
+
+    uint32_t operator--() noexcept
+    {
+        auto const remaining = m_count.fetch_sub(1, std::memory_order_release) - 1;
+
+        if (remaining == 0)
+        {
+            std::atomic_thread_fence(std::memory_order_acquire);
+            notifier();
+        }
+        else if (remaining < 0)
+        {
+            abort();
+        }
+
+        return remaining;
+    }
+
+    operator uint32_t() const noexcept
+    {
+        return m_count;
+    }
+
+    template <typename Func>
+    void set_notifier(Func&& func)
+    {
+        notifier = std::forward<Func>(func);
+    }
+
+private:
+    std::atomic<int32_t> m_count{0};
+    std::function<void()> notifier{};
+};
+
+struct notifiable_module_lock final : notifiable_module_lock_base
+{
+    static notifiable_module_lock& instance() noexcept
+    {
+        static notifiable_module_lock lock;
+        return lock;
+    }
+};
+} // namespace wil
+
+#ifndef WIL_CPPWINRT_COM_SERVER_CUSTOM_MODULE_LOCK
+
+namespace winrt
+{
+auto& get_module_lock()
+{
+    return wil::notifiable_module_lock::instance();
+}
+} // namespace winrt
+
+#endif
+
+#endif
--- a/include/wil/cppwinrt_register_com_server.h
+++ b/include/wil/cppwinrt_register_com_server.h
@ -0,0 +1,76 @@
+//*********************************************************
+//
+//    Copyright (c) Microsoft. All rights reserved.
+//    This code is licensed under the MIT License.
+//    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF
+//    ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED
+//    TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A
+//    PARTICULAR PURPOSE AND NONINFRINGEMENT.
+//
+//*********************************************************
+//! @file
+//! Utilities for making managing COM server easier
+
+#ifndef __WIL_CPPWINRT_REGISTER_COM_SERVER_INCLUDED
+#define __WIL_CPPWINRT_REGISTER_COM_SERVER_INCLUDED
+
+#include <winrt/base.h>
+#include <wil/resource.h>
+#include <wil/cppwinrt.h>
+#include <vector>
+
+namespace wil::details
+{
+template <typename T>
+struct CppWinRTClassFactory : winrt::implements<CppWinRTClassFactory<T>, IClassFactory, winrt::no_module_lock>
+{
+    HRESULT __stdcall CreateInstance(IUnknown* outer, GUID const& iid, void** result) noexcept final
+    try
+    {
+        *result = nullptr;
+
+        if (outer)
+        {
+            return CLASS_E_NOAGGREGATION;
+        }
+        return winrt::make_self<T>().as(iid, result);
+    }
+    CATCH_RETURN()
+
+    HRESULT __stdcall LockServer(BOOL) noexcept final
+    {
+        return S_OK;
+    }
+};
+
+template <typename T = void, typename... Rest>
+void register_com_server(std::vector<wil::unique_com_class_object_cookie>& registrations)
+{
+    DWORD registration{};
+    winrt::check_hresult(CoRegisterClassObject(
+        winrt::guid_of<T>(), winrt::make<CppWinRTClassFactory<T>>().get(), CLSCTX_LOCAL_SERVER, REGCLS_MULTIPLEUSE, &registration));
+    // This emplace_back is no-throw as wil::register_com_server already reserves enough capacity
+    registrations.emplace_back(registration);
+    register_com_server<Rest...>(registrations);
+}
+
+template <>
+void register_com_server<void>(std::vector<unique_com_class_object_cookie>&)
+{
+}
+} // namespace wil::details
+
+namespace wil
+{
+template <typename T, typename... Rest>
+[[nodiscard]] std::vector<wil::unique_com_class_object_cookie> register_com_server()
+{
+    std::vector<wil::unique_com_class_object_cookie> registrations;
+    registrations.reserve(sizeof...(Rest) + 1);
+    details::register_com_server<T, Rest...>(registrations);
+    // C++17 doesn't provide guaranteed copy ellision, but the copy should be elided nonetheless.
+    return registrations;
+}
+} // namespace wil
+
+#endif
--- a/include/wil/result_macros.h
+++ b/include/wil/result_macros.h
@ -93,7 +93,7 @@ typedef _Return_type_success_(return >= 0) LONG NTSTATUS;
 #endif
 #ifndef __NTSTATUS_FROM_WIN32
 #define __NTSTATUS_FROM_WIN32(x) \
-    ((NTSTATUS)(x) <= 0 ? ((NTSTATUS)(x)) : ((NTSTATUS)(((x)&0x0000FFFF) | (FACILITY_WIN32 << 16) | ERROR_SEVERITY_ERROR)))
+    ((NTSTATUS)(x) <= 0 ? ((NTSTATUS)(x)) : ((NTSTATUS)(((x) & 0x0000FFFF) | (FACILITY_WIN32 << 16) | ERROR_SEVERITY_ERROR)))
 #endif

 #ifndef WIL_AllocateMemory
@ -2770,7 +2770,7 @@ namespace details
            {
                status =
                    ((NTSTATUS)(hr) <= 0 ? ((NTSTATUS)(hr))
-                                         : ((NTSTATUS)(((hr)&0x0000FFFF) | (FACILITY_SSPI << 16) | ERROR_SEVERITY_ERROR)));
+                                         : ((NTSTATUS)(((hr) & 0x0000FFFF) | (FACILITY_SSPI << 16) | ERROR_SEVERITY_ERROR)));
            }
            else
            {
--- a/include/wil/wistd_type_traits.h
+++ b/include/wil/wistd_type_traits.h
@ -1946,7 +1946,7 @@ struct __numeric_type<void>

 // __promote

-template <class _A1, class _A2 = void, class _A3 = void, bool = __numeric_type<_A1>::value&& __numeric_type<_A2>::value&& __numeric_type<_A3>::value>
+template <class _A1, class _A2 = void, class _A3 = void, bool = __numeric_type<_A1>::value && __numeric_type<_A2>::value && __numeric_type<_A3>::value>
 class __promote_imp
 {
 public:
--- a/scripts/runtests.cmd
+++ b/scripts/runtests.cmd
@ -43,6 +43,10 @@ call :execute_test app witest.app.exe
 if %ERRORLEVEL% NEQ 0 ( goto :execute_tests_done )
 call :execute_test cpplatest witest.cpplatest.exe
 if %ERRORLEVEL% NEQ 0 ( goto :execute_tests_done )
+call :execute_test cppwinrt-com-server witest.cppwinrt-com-server.exe
+if %ERRORLEVEL% NEQ 0 ( goto :execute_tests_done )
+call :execute_test cppwinrt-com-server-custom-module-lock witest.cppwinrt-com-server-custom-module-lock.exe
+if %ERRORLEVEL% NEQ 0 ( goto :execute_tests_done )
 call :execute_test noexcept witest.noexcept.exe
 if %ERRORLEVEL% NEQ 0 ( goto :execute_tests_done )
 call :execute_test normal witest.exe
--- a/tests/CMakeLists.txt
+++ b/tests/CMakeLists.txt
@ -53,7 +53,7 @@ endif()
 # For some unknown reason, 'RelWithDebInfo' compiles with '/Ob1' as opposed to '/Ob2' which prevents inlining of
 # functions not marked 'inline'. The reason we prefer 'RelWithDebInfo' over 'Release' is to get debug info, so manually
 # revert to the desired (and default) inlining behavior as that exercises more interesting code paths
-if (MSVC AND ${CMAKE_BUILD_TYPE} STREQUAL "RelWithDebInfo")
+if (MSVC AND "${CMAKE_BUILD_TYPE}" STREQUAL "RelWithDebInfo")
    # TODO: This is currently blocked by an apparent Clang bug: https://github.com/llvm/llvm-project/issues/59690
    # replace_cxx_flag("/Ob1" "/Ob2")
 endif()
@ -92,6 +92,8 @@ link_libraries(${DETOURS_LIBRARY} Catch2::Catch2WithMain)

 add_subdirectory(app)
 add_subdirectory(cpplatest)
+add_subdirectory(cppwinrt-com-server)
+add_subdirectory(cppwinrt-com-server-custom-module-lock)
 add_subdirectory(noexcept)
 add_subdirectory(normal)
 add_subdirectory(win7)
@ -99,10 +101,10 @@ add_subdirectory(win7)
 set(DEBUG_BUILD FALSE)
 set(HAS_DEBUG_INFO FALSE)

-if (${CMAKE_BUILD_TYPE} STREQUAL "Debug")
+if ("${CMAKE_BUILD_TYPE}" STREQUAL "Debug")
    set(DEBUG_BUILD TRUE)
    set(HAS_DEBUG_INFO TRUE)
-elseif(${CMAKE_BUILD_TYPE} STREQUAL "RelWithDebInfo")
+elseif("${CMAKE_BUILD_TYPE}" STREQUAL "RelWithDebInfo")
    set(HAS_DEBUG_INFO TRUE)
 endif()
 # Release & MinSizeRel => keep all false
--- a/tests/CppWinRTComServerCustomModuleLockTests.cpp
+++ b/tests/CppWinRTComServerCustomModuleLockTests.cpp
@ -0,0 +1,72 @@
+#include "pch.h"
+
+#include "common.h"
+#undef GetCurrentTime
+#define WINRT_CUSTOM_MODULE_LOCK
+#define WIL_CPPWINRT_COM_SERVER_CUSTOM_MODULE_LOCK
+#include <wil/cppwinrt_notifiable_module_lock.h>
+struct custom_lock : wil::notifiable_module_lock_base
+{
+    bool called{};
+    uint32_t operator++() noexcept
+    {
+        auto result = wil::notifiable_module_lock_base::operator++();
+        called = true;
+        return result;
+    }
+};
+namespace winrt
+{
+inline auto& get_module_lock()
+{
+    static custom_lock lock;
+    return lock;
+}
+} // namespace winrt
+#include <winrt/Windows.Foundation.h>
+#include <wil/cppwinrt_register_com_server.h>
+#include <wil/resource.h>
+#include <string_view>
+#include <thread>
+#include <chrono>
+
+using namespace std::string_view_literals;
+
+wil::unique_event _comExit;
+
+void notifier()
+{
+    _comExit.SetEvent();
+}
+
+struct MyServer : winrt::implements<MyServer, winrt::Windows::Foundation::IStringable>
+{
+    winrt::hstring ToString()
+    {
+        return L"MyServer from Server";
+    }
+};
+
+auto create_my_server_instance()
+{
+    return winrt::create_instance<winrt::Windows::Foundation::IStringable>(winrt::guid_of<MyServer>(), CLSCTX_LOCAL_SERVER);
+}
+
+TEST_CASE("CppWinRTComServerTests::CustomNotifiableModuleLock", "[cppwinrt_com_server]")
+{
+    _comExit.create();
+
+    winrt::get_module_lock().set_notifier(notifier);
+
+    winrt::init_apartment();
+
+    {
+        auto server{winrt::make<MyServer>()};
+        REQUIRE(winrt::get_module_lock().called);
+        REQUIRE(winrt::get_module_lock() == 1);
+    }
+
+    _comExit.wait();
+
+    REQUIRE(!winrt::get_module_lock());
+}
--- a/tests/CppWinRTComServerTests.cpp
+++ b/tests/CppWinRTComServerTests.cpp
@ -0,0 +1,160 @@
+#include "pch.h"
+
+#include "common.h"
+#undef GetCurrentTime
+#define WINRT_CUSTOM_MODULE_LOCK
+#include <wil/cppwinrt_notifiable_module_lock.h>
+#include <winrt/Windows.Foundation.h>
+#include <wil/cppwinrt_register_com_server.h>
+#include <wil/resource.h>
+#include <string_view>
+#include <thread>
+#include <chrono>
+
+using namespace std::string_view_literals;
+
+wil::unique_event _comExit;
+
+void notifier()
+{
+    _comExit.SetEvent();
+}
+
+struct MyServer : winrt::implements<MyServer, winrt::Windows::Foundation::IStringable>
+{
+    winrt::hstring ToString()
+    {
+        return L"MyServer from Server";
+    }
+};
+
+struct BuggyServer : winrt::implements<BuggyServer, winrt::Windows::Foundation::IStringable>
+{
+    BuggyServer()
+    {
+        throw winrt::hresult_access_denied{};
+    }
+    winrt::hstring ToString()
+    {
+        return L"BuggyServer from Server";
+    }
+};
+
+auto create_my_server_instance()
+{
+    return winrt::create_instance<winrt::Windows::Foundation::IStringable>(winrt::guid_of<MyServer>(), CLSCTX_LOCAL_SERVER);
+}
+
+TEST_CASE("CppWinRTComServerTests::DefaultNotifiableModuleLock", "[cppwinrt_com_server]")
+{
+    _comExit.create();
+
+    wil::notifiable_module_lock::instance().set_notifier(notifier);
+
+    winrt::init_apartment();
+
+    {
+        auto server{winrt::make<MyServer>()};
+        REQUIRE(winrt::get_module_lock() == 1);
+    }
+
+    _comExit.wait();
+
+    REQUIRE(!winrt::get_module_lock());
+}
+
+TEST_CASE("CppWinRTComServerTests::RegisterComServer", "[cppwinrt_com_server]")
+{
+    winrt::init_apartment();
+
+    {
+        auto revoker = wil::register_com_server<MyServer>();
+        auto instance = create_my_server_instance();
+        REQUIRE(winrt::get_module_lock() == 1);
+    }
+    REQUIRE(!winrt::get_module_lock());
+    try
+    {
+        auto instance = create_my_server_instance();
+    }
+    catch (winrt::hresult_error const& e)
+    {
+        REQUIRE(e.code() == REGDB_E_CLASSNOTREG);
+    }
+}
+
+TEST_CASE("CppWinRTComServerTests::RegisterComServerThrowIsSafe", "[cppwinrt_com_server]")
+{
+    winrt::init_apartment();
+
+    {
+        auto revoker = wil::register_com_server<BuggyServer>();
+        try
+        {
+            auto instance =
+                winrt::create_instance<winrt::Windows::Foundation::IStringable>(winrt::guid_of<BuggyServer>(), CLSCTX_LOCAL_SERVER);
+            REQUIRE(false);
+        }
+        catch (winrt::hresult_error const& e)
+        {
+            REQUIRE(e.code() == E_ACCESSDENIED);
+        }
+    }
+}
+
+TEST_CASE("CppWinRTComServerTests::AnyRegisterFailureClearAllRegistrations", "[cppwinrt_com_server]")
+{
+    winrt::init_apartment();
+
+    witest::detoured_thread_function<&::CoRegisterClassObject> detour(
+        [](REFCLSID clsid, LPUNKNOWN obj, DWORD ctxt, DWORD flags, LPDWORD cookie) mutable {
+            if (winrt::guid{clsid} == winrt::guid_of<BuggyServer>())
+            {
+                *cookie = 0;
+                return E_UNEXPECTED;
+            }
+            return ::CoRegisterClassObject(clsid, obj, ctxt, flags, cookie);
+        });
+    try
+    {
+        auto revoker = wil::register_com_server<MyServer, BuggyServer>();
+        REQUIRE(false);
+    }
+    catch (winrt::hresult_error const& e)
+    {
+        REQUIRE(e.code() == E_UNEXPECTED);
+    }
+    REQUIRE(!winrt::get_module_lock());
+}
+
+TEST_CASE("CppWinRTComServerTests::NotifierAndRegistration", "[cppwinrt_com_server]")
+{
+    wil::unique_event moduleEvent(wil::EventOptions::None);
+    wil::unique_event coroutineRunning(wil::EventOptions::None);
+    wil::unique_event coroutineContinue(wil::EventOptions::None);
+
+    wil::notifiable_module_lock::instance().set_notifier([&]() {
+        moduleEvent.SetEvent();
+    });
+
+    winrt::init_apartment();
+
+    auto revoker = wil::register_com_server<MyServer>();
+
+    [&]() -> winrt::Windows::Foundation::IAsyncAction {
+        co_await winrt::resume_background();
+        coroutineRunning.SetEvent();
+
+        coroutineContinue.wait();
+        auto instance = create_my_server_instance();
+        REQUIRE(winrt::get_module_lock() == 2);
+    }();
+
+    coroutineRunning.wait();
+    REQUIRE(winrt::get_module_lock() == 1); // Coroutine bumped count
+
+    coroutineContinue.SetEvent();
+    moduleEvent.wait();
+
+    REQUIRE(!winrt::get_module_lock());
+}
--- a/tests/cppwinrt-com-server-custom-module-lock/CMakeLists.txt
+++ b/tests/cppwinrt-com-server-custom-module-lock/CMakeLists.txt
@ -0,0 +1,13 @@
+
+add_executable(witest.cppwinrt-com-server-custom-module-lock)
+
+target_precompile_headers(witest.cppwinrt-com-server-custom-module-lock PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../pch.h)
+
+# Compilers often don't use the latest C++ standard as the default. Periodically update this value (possibly conditioned
+# on compiler) as new standards are ratified/support is available
+target_compile_features(witest.cppwinrt-com-server-custom-module-lock PRIVATE cxx_std_20)
+
+target_sources(witest.cppwinrt-com-server-custom-module-lock PRIVATE
+    ${CMAKE_CURRENT_SOURCE_DIR}/../main.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../CppWinRTComServerCustomModuleLockTests.cpp
+    )
--- a/tests/cppwinrt-com-server/CMakeLists.txt
+++ b/tests/cppwinrt-com-server/CMakeLists.txt
@ -0,0 +1,13 @@
+
+add_executable(witest.cppwinrt-com-server)
+
+target_precompile_headers(witest.cppwinrt-com-server PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}/../pch.h)
+
+# Compilers often don't use the latest C++ standard as the default. Periodically update this value (possibly conditioned
+# on compiler) as new standards are ratified/support is available
+target_compile_features(witest.cppwinrt-com-server PRIVATE cxx_std_20)
+
+target_sources(witest.cppwinrt-com-server PRIVATE
+    ${CMAKE_CURRENT_SOURCE_DIR}/../main.cpp
+    ${CMAKE_CURRENT_SOURCE_DIR}/../CppWinRTComServerTests.cpp
+    )