-
Notifications
You must be signed in to change notification settings - Fork 281
Port crash-resistant event handler as wil::winrt_event<T, Traits>
#619
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 3 commits
d8c7b48
f759ac8
75eb8d9
808b460
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -334,3 +334,4 @@ ASALocalRun/ | |
|
|
||
| # CMake/Build output | ||
| build/ | ||
| _codeql_detected_source_root | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| . |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -251,6 +251,92 @@ struct typed_event : wil::details::event_base<winrt::Windows::Foundation::TypedE | |
| { | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Traits for wil::winrt_event that swallows exceptions thrown by event handlers. | ||
| * All handlers are called even if some throw; exceptions are discarded. | ||
| * This provides crash-resistant behavior and is the default for wil::winrt_event. | ||
| */ | ||
| struct swallow_event_errors_traits | ||
| { | ||
| static void on_handler_exception(std::exception_ptr) noexcept | ||
| { | ||
| // Intentionally discard the exception to allow remaining handlers to run. | ||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Traits for wil::winrt_event that propagates exceptions thrown by event handlers. | ||
| * If a handler throws, the exception propagates out of invoke() and remaining handlers | ||
| * are not called. This matches the behavior of winrt::event. | ||
| */ | ||
| struct propagate_event_errors_traits | ||
| { | ||
| static void on_handler_exception(std::exception_ptr ep) | ||
| { | ||
| std::rethrow_exception(ep); | ||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * @brief Traits for wil::winrt_event that calls FAIL_FAST when an event handler throws. | ||
| * Requires wil/result.h or wil/result_macros.h to be included. | ||
| */ | ||
| struct failfast_event_errors_traits | ||
| { | ||
| [[noreturn]] static void on_handler_exception(std::exception_ptr) noexcept | ||
| { | ||
| FAIL_FAST(); | ||
| } | ||
| }; | ||
|
|
||
| /** | ||
| * @brief A WinRT event with configurable exception handling for individual handlers. | ||
| * @tparam T The delegate type (e.g., winrt::Windows::Foundation::TypedEventHandler<S, A>). | ||
| * @tparam Traits Traits class controlling what happens when a handler throws an exception. | ||
| * Defaults to swallow_event_errors_traits for crash-resistant behavior where | ||
| * all handlers are invoked regardless of whether earlier ones threw. | ||
| * @details Usage example: | ||
| * @code | ||
| * // Crash-resistant event (default): exceptions from handlers are swallowed | ||
| * wil::winrt_event<winrt::Windows::Foundation::TypedEventHandler<IFoo, IBar>> MyEvent; | ||
| * | ||
| * // Event that propagates exceptions (same behavior as winrt::event): | ||
| * wil::winrt_event<winrt::Windows::Foundation::TypedEventHandler<IFoo, IBar>, | ||
| * wil::propagate_event_errors_traits> MyStrictEvent; | ||
| * @endcode | ||
| */ | ||
| template <typename T, typename Traits = swallow_event_errors_traits> | ||
| struct winrt_event | ||
| { | ||
| winrt::event_token operator()(T const& handler) | ||
| { | ||
| return m_handler.add([handler](auto&&... args) { | ||
| try | ||
| { | ||
| handler(std::forward<decltype(args)>(args)...); | ||
| } | ||
| catch (...) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot update this so the traits by-default use "wil::LogCaughtException" or whatever it's called, but that can be disabled with a flag.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Updated in 808b460. |
||
| { | ||
| Traits::on_handler_exception(std::current_exception()); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| void operator()(winrt::event_token const& token) noexcept | ||
| { | ||
| m_handler.remove(token); | ||
| } | ||
|
|
||
| template <typename... TArgs> | ||
| void invoke(TArgs&&... args) | ||
| { | ||
| m_handler(std::forward<TArgs>(args)...); | ||
| } | ||
|
|
||
| private: | ||
| winrt::event<T> m_handler; | ||
| }; | ||
|
|
||
| #endif // !defined(__WIL_CPPWINRT_AUTHORING_INCLUDED_FOUNDATION) && defined(WINRT_Windows_Foundation_H) | ||
|
|
||
| #if (!defined(__WIL_CPPWINRT_AUTHORING_INCLUDED_XAML_DATA) && (defined(WINRT_Microsoft_UI_Xaml_Data_H) || defined(WINRT_Windows_UI_Xaml_Data_H))) || \ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -266,6 +266,102 @@ TEST_CASE("CppWinRTAuthoringTests::EventsAndCppWinRt", "[property]") | |
| test.Closed(token); | ||
| } | ||
|
|
||
| TEST_CASE("CppWinRTAuthoringTests::WinrtEvent", "[winrt_event]") | ||
| { | ||
| // Basic smoke test: register, invoke, unregister | ||
| struct Test | ||
| { | ||
| wil::winrt_event<winrt::Windows::Foundation::EventHandler<int>> MyEvent; | ||
| } test; | ||
|
|
||
| int value = 0; | ||
| auto token = test.MyEvent([&](const winrt::Windows::Foundation::IInspectable&, int args) { | ||
| value = args; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @copilot This should have a test that verifies the "caught, not leaked" behavior. Be sure to verify the "no filter" and "some filter" and "all filter" behavior from above.
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added comprehensive tests in 808b460:
|
||
| }); | ||
| test.MyEvent.invoke(nullptr, 42); | ||
| REQUIRE(value == 42); | ||
| test.MyEvent(token); | ||
| } | ||
|
|
||
| TEST_CASE("CppWinRTAuthoringTests::WinrtEventSwallowExceptions", "[winrt_event]") | ||
| { | ||
| // Default (swallow) traits: all handlers are called even if some throw | ||
| struct Test | ||
| { | ||
| wil::winrt_event<winrt::Windows::Foundation::EventHandler<int>> MyEvent; | ||
| } test; | ||
|
|
||
| int callCount = 0; | ||
|
|
||
| // First handler throws | ||
| auto token1 = test.MyEvent([&](const winrt::Windows::Foundation::IInspectable&, int) { | ||
| ++callCount; | ||
| throw winrt::hresult_not_implemented{}; | ||
| }); | ||
|
|
||
| // Second handler should still be called | ||
| auto token2 = test.MyEvent([&](const winrt::Windows::Foundation::IInspectable&, int args) { | ||
| ++callCount; | ||
| REQUIRE(args == 42); | ||
| }); | ||
|
|
||
| // invoke() must not throw even though first handler throws | ||
| REQUIRE_NOTHROW(test.MyEvent.invoke(nullptr, 42)); | ||
|
|
||
| // Both handlers must have been called | ||
| REQUIRE(callCount == 2); | ||
|
|
||
| test.MyEvent(token1); | ||
| test.MyEvent(token2); | ||
| } | ||
|
|
||
| TEST_CASE("CppWinRTAuthoringTests::WinrtEventPropagateExceptions", "[winrt_event]") | ||
| { | ||
| // Propagate traits: first exception propagates and remaining handlers are skipped | ||
| struct Test | ||
| { | ||
| wil::winrt_event<winrt::Windows::Foundation::EventHandler<int>, wil::propagate_event_errors_traits> MyEvent; | ||
| } test; | ||
|
|
||
| int callCount = 0; | ||
|
|
||
| auto token1 = test.MyEvent([&](const winrt::Windows::Foundation::IInspectable&, int) { | ||
| ++callCount; | ||
| throw winrt::hresult_not_implemented{}; | ||
| }); | ||
|
|
||
| // This handler should NOT be called because the first one throws | ||
| auto token2 = test.MyEvent([&](const winrt::Windows::Foundation::IInspectable&, int) { | ||
| ++callCount; | ||
| }); | ||
|
|
||
| // invoke() must throw | ||
| REQUIRE_THROWS_AS(test.MyEvent.invoke(nullptr, 42), winrt::hresult_not_implemented); | ||
|
|
||
| // Only the first handler ran | ||
| REQUIRE(callCount == 1); | ||
|
|
||
| test.MyEvent(token1); | ||
| test.MyEvent(token2); | ||
| } | ||
|
|
||
| TEST_CASE("CppWinRTAuthoringTests::WinrtEventTypedDelegate", "[winrt_event]") | ||
| { | ||
| // winrt_event works with TypedEventHandler delegates | ||
| struct Test | ||
| { | ||
| wil::winrt_event<winrt::Windows::Foundation::TypedEventHandler<winrt::Windows::Foundation::IInspectable, int>> MyEvent; | ||
| } test; | ||
|
|
||
| int value = 0; | ||
| auto token = test.MyEvent([&](const winrt::Windows::Foundation::IInspectable&, int args) { | ||
| value = args; | ||
| }); | ||
| test.MyEvent.invoke(nullptr, 99); | ||
| REQUIRE(value == 99); | ||
| test.MyEvent(token); | ||
| } | ||
|
|
||
| #include <winrt/Windows.UI.Xaml.Hosting.h> | ||
|
|
||
| TEST_CASE("CppWinRTAuthoringTests::NotifyPropertyChanged", "[property]") | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.