From 7a48b4c1604c2cbefe91b0f09610f42ab864c0a0 Mon Sep 17 00:00:00 2001 From: Jon Wiswall Date: Wed, 31 Dec 2025 14:44:01 -0800 Subject: [PATCH 1/2] Enable non-resetting watchers --- CMakePresets.json | 29 +++++++------- include/wil/resource.h | 87 +++++++++++++++++++++++------------------- tests/WatcherTests.cpp | 30 +++++++++++++++ 3 files changed, 93 insertions(+), 53 deletions(-) diff --git a/CMakePresets.json b/CMakePresets.json index 61f23932..836f5905 100644 --- a/CMakePresets.json +++ b/CMakePresets.json @@ -14,6 +14,7 @@ "installDir": "${sourceDir}/build/install/${presetName}", "toolchainFile": "$env{VCPKG_ROOT}/scripts/buildsystems/vcpkg.cmake", "cacheVariables": { + "WIL_ENABLE_ASAN": true, "CMAKE_CONFIGURATION_TYPES": "Debug;RelWithDebInfo;Release;MinSizeRel", "CMAKE_CXX_COMPILER": "cl", "CMAKE_C_COMPILER": "cl" @@ -29,9 +30,19 @@ } }, "cacheVariables": { + "WIL_ENABLE_ASAN": false, "CMAKE_CXX_COMPILER": "clang-cl", "CMAKE_C_COMPILER": "clang-cl" } + }, + { + "name": "clang-release", + "inherits": "clang", + "hidden": false, + "cacheVariables": { + "WIL_ENABLE_ASAN": true, + "WIL_ENABLE_UBSAN": true + } } ], "buildPresets": [ @@ -39,19 +50,13 @@ "name": "msvc-debug", "displayName": "MSVC Debug", "configurePreset": "msvc", - "configuration": "Debug", - "cacheVariables": { - "WIL_ENABLE_ASAN": true - } + "configuration": "Debug" }, { "name": "msvc-release", "displayName": "MSVC Release (debuggable)", "configurePreset": "msvc", - "configuration": "RelWithDebInfo", - "cacheVariables": { - "WIL_ENABLE_ASAN": true - } + "configuration": "RelWithDebInfo" }, { "name": "clang-debug", @@ -62,12 +67,8 @@ { "name": "clang-release", "displayName": "clang Release (debuggable)", - "configurePreset": "clang", - "configuration": "RelWithDebInfo", - "cacheVariables": { - "WIL_ENABLE_ASAN": true, - "WIL_ENABLE_UBSAN": true - } + "configurePreset": "clang-release", + "configuration": "RelWithDebInfo" } ], "testPresets": [ diff --git a/include/wil/resource.h b/include/wil/resource.h index eb70fc30..82015308 100644 --- a/include/wil/resource.h +++ b/include/wil/resource.h @@ -3800,15 +3800,20 @@ using unique_mapview_ptr = wistd::unique_ptr&& callback) : - m_callback(wistd::move(callback)), m_event(wistd::move(eventHandle)) - { - } wistd::function m_callback; + event_watcher_options m_options; unique_event_nothrow m_event; // The thread pool must be last to ensure that the other members are valid // when it is destructed as it will reference them. @@ -3841,34 +3846,36 @@ class event_watcher_t : public storage_t template event_watcher_t( unique_any_t, from_err_policy>>&& eventHandle, - wistd::function&& callback) + wistd::function&& callback, + event_watcher_options options = event_watcher_options::reset_event) { static_assert(wistd::is_same::value, "this constructor requires exceptions or fail fast; use the create method"); - create(wistd::move(eventHandle), wistd::move(callback)); + create(wistd::move(eventHandle), wistd::move(callback), options); } - event_watcher_t(_In_ HANDLE eventHandle, wistd::function&& callback) + event_watcher_t(_In_ HANDLE eventHandle, wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) { static_assert(wistd::is_same::value, "this constructor requires exceptions or fail fast; use the create method"); - create(eventHandle, wistd::move(callback)); + create(eventHandle, wistd::move(callback), options); } - event_watcher_t(wistd::function&& callback) + event_watcher_t(wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) { static_assert(wistd::is_same::value, "this constructor requires exceptions or fail fast; use the create method"); - create(wistd::move(callback)); + create(wistd::move(callback), options); } template result create( unique_any_t, event_err_policy>>&& eventHandle, - wistd::function&& callback) + wistd::function&& callback, + event_watcher_options options = event_watcher_options::reset_event) { - return err_policy::HResult(create_take_hevent_ownership(eventHandle.release(), wistd::move(callback))); + return err_policy::HResult(create_take_hevent_ownership(eventHandle.release(), wistd::move(callback), options)); } // Creates the event that you will be watching. - result create(wistd::function&& callback) + result create(wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) { unique_event_nothrow eventHandle; HRESULT hr = eventHandle.create(EventOptions::ManualReset); // auto-reset is supported too. @@ -3876,18 +3883,18 @@ class event_watcher_t : public storage_t { return err_policy::HResult(hr); } - return err_policy::HResult(create_take_hevent_ownership(eventHandle.release(), wistd::move(callback))); + return err_policy::HResult(create_take_hevent_ownership(eventHandle.release(), wistd::move(callback), options)); } // Input is an event handler that is duplicated into this class. - result create(_In_ HANDLE eventHandle, wistd::function&& callback) + result create(_In_ HANDLE eventHandle, wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) { unique_event_nothrow ownedHandle; if (!DuplicateHandle(GetCurrentProcess(), eventHandle, GetCurrentProcess(), &ownedHandle, 0, FALSE, DUPLICATE_SAME_ACCESS)) { return err_policy::LastError(); } - return err_policy::HResult(create_take_hevent_ownership(ownedHandle.release(), wistd::move(callback))); + return err_policy::HResult(create_take_hevent_ownership(ownedHandle.release(), wistd::move(callback), options)); } // Provide access to the inner event and the very common SetEvent() method on it. @@ -3901,26 +3908,27 @@ class event_watcher_t : public storage_t } private: - // Had to move this from a Lambda so it would compile in C++/CLI (which thought the Lambda should be a managed function for some reason). static void CALLBACK wait_callback(PTP_CALLBACK_INSTANCE, void* context, TP_WAIT* pThreadPoolWait, TP_WAIT_RESULT) { auto pThis = static_cast(context); - // Manual events must be re-set to avoid missing the last notification. - pThis->m_event.ResetEvent(); - // Call the client before re-arming to ensure that multiple callbacks don't - // run concurrently. + if (WI_IsFlagSet(pThis->m_options, event_watcher_options::reset_event)) + { + // Manual events must be re-set to avoid missing the last notification. + pThis->m_event.ResetEvent(); + } + // Call the client before re-arming to ensure that multiple callbacks don't run concurrently. pThis->m_callback(); SetThreadpoolWait(pThreadPoolWait, pThis->m_event.get(), nullptr); // valid params ensure success } // To avoid template expansion (if unique_event/unique_event_nothrow forms were used) this base // create function takes a raw handle and assumes its ownership, even on failure. - HRESULT create_take_hevent_ownership(_In_ HANDLE rawHandleOwnershipTaken, wistd::function&& callback) + HRESULT create_take_hevent_ownership(_In_ HANDLE rawHandleOwnershipTaken, wistd::function&& callback, event_watcher_options options) { __FAIL_FAST_ASSERT__(rawHandleOwnershipTaken != nullptr); // invalid parameter unique_event_nothrow eventHandle(rawHandleOwnershipTaken); wistd::unique_ptr watcherState( - new (std::nothrow) details::event_watcher_state(wistd::move(eventHandle), wistd::move(callback))); + new (std::nothrow) details::event_watcher_state{wistd::move(callback), options, wistd::move(eventHandle)}); RETURN_IF_NULL_ALLOC(watcherState); watcherState->m_threadPoolWait.reset(CreateThreadpoolWait(wait_callback, watcherState.get(), nullptr)); @@ -3937,43 +3945,44 @@ typedef unique_any_t unique_event_watcher_nothrow make_event_watcher_nothrow( unique_any_t, err_policy>>&& eventHandle, - wistd::function&& callback) WI_NOEXCEPT + wistd::function&& callback, + event_watcher_options options = event_watcher_options::reset_event) WI_NOEXCEPT { unique_event_watcher_nothrow watcher; - watcher.create(wistd::move(eventHandle), wistd::move(callback)); + watcher.create(wistd::move(eventHandle), wistd::move(callback), options); return watcher; // caller must test for success using if (watcher) } -inline unique_event_watcher_nothrow make_event_watcher_nothrow(_In_ HANDLE eventHandle, wistd::function&& callback) WI_NOEXCEPT +inline unique_event_watcher_nothrow make_event_watcher_nothrow(_In_ HANDLE eventHandle, wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) WI_NOEXCEPT { unique_event_watcher_nothrow watcher; - watcher.create(eventHandle, wistd::move(callback)); + watcher.create(eventHandle, wistd::move(callback), options); return watcher; // caller must test for success using if (watcher) } -inline unique_event_watcher_nothrow make_event_watcher_nothrow(wistd::function&& callback) WI_NOEXCEPT +inline unique_event_watcher_nothrow make_event_watcher_nothrow(wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) WI_NOEXCEPT { unique_event_watcher_nothrow watcher; - watcher.create(wistd::move(callback)); + watcher.create(wistd::move(callback), options); return watcher; // caller must test for success using if (watcher) } template unique_event_watcher_failfast make_event_watcher_failfast( unique_any_t, err_policy>>&& eventHandle, - wistd::function&& callback) + wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) { - return unique_event_watcher_failfast(wistd::move(eventHandle), wistd::move(callback)); + return unique_event_watcher_failfast(wistd::move(eventHandle), wistd::move(callback), options); } -inline unique_event_watcher_failfast make_event_watcher_failfast(_In_ HANDLE eventHandle, wistd::function&& callback) +inline unique_event_watcher_failfast make_event_watcher_failfast(_In_ HANDLE eventHandle, wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) { - return unique_event_watcher_failfast(eventHandle, wistd::move(callback)); + return unique_event_watcher_failfast(eventHandle, wistd::move(callback), options); } -inline unique_event_watcher_failfast make_event_watcher_failfast(wistd::function&& callback) +inline unique_event_watcher_failfast make_event_watcher_failfast(wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) { - return unique_event_watcher_failfast(wistd::move(callback)); + return unique_event_watcher_failfast(wistd::move(callback), options); } #ifdef WIL_ENABLE_EXCEPTIONS @@ -3982,14 +3991,14 @@ typedef unique_any_t unique_event_watcher make_event_watcher( unique_any_t, err_policy>>&& eventHandle, - wistd::function&& callback) + wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) { - return unique_event_watcher(wistd::move(eventHandle), wistd::move(callback)); + return unique_event_watcher(wistd::move(eventHandle), wistd::move(callback), options); } -inline unique_event_watcher make_event_watcher(_In_ HANDLE eventHandle, wistd::function&& callback) +inline unique_event_watcher make_event_watcher(_In_ HANDLE eventHandle, wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) { - return unique_event_watcher(eventHandle, wistd::move(callback)); + return unique_event_watcher(eventHandle, wistd::move(callback), options); } inline unique_event_watcher make_event_watcher(wistd::function&& callback) diff --git a/tests/WatcherTests.cpp b/tests/WatcherTests.cpp index e53a9915..66ca6545 100644 --- a/tests/WatcherTests.cpp +++ b/tests/WatcherTests.cpp @@ -85,6 +85,36 @@ static auto make_event(wil::EventOptions options = wil::EventOptions::None) return result; } +TEST_CASE("EventWatcherTests::DoNotResetEvent", "[resource][event_watcher]") +{ + // Create an event with all access. Use DuplicateHandle to create a second handle without + // the EVENT_MODIFY_STATE right, so we can test that the event was not reset by the watcher. + auto notificationReceived = make_event(wil::EventOptions::ManualReset); + auto watchedEvent = make_event(wil::EventOptions::ManualReset); + wil::unique_event_nothrow watchedEventSynchronize; + REQUIRE(DuplicateHandle( + GetCurrentProcess(), + watchedEvent.get(), + GetCurrentProcess(), + &watchedEventSynchronize, + SYNCHRONIZE, // no EVENT_MODIFY_STATE + FALSE, + 0)); + int volatile countObserved = 0; + auto watcher = wil::make_event_watcher_nothrow( + wistd::move(watchedEventSynchronize), + [&] { + countObserved = countObserved + 1; + notificationReceived.SetEvent(); + }, + wil::event_watcher_options::none); + + REQUIRE(watcher != nullptr); + watchedEvent.SetEvent(); + REQUIRE(notificationReceived.wait(5000)); // 5 second max wait + REQUIRE(watchedEvent.is_signaled()); // event should still be signaled +} + TEST_CASE("EventWatcherTests::VerifyDelivery", "[resource][event_watcher]") { auto notificationReceived = make_event(); From 4ef762284807c2d82465ea44cc04aae86c5443a0 Mon Sep 17 00:00:00 2001 From: Jon Wiswall Date: Wed, 31 Dec 2025 14:45:09 -0800 Subject: [PATCH 2/2] Clang-format --- include/wil/resource.h | 23 +++++++++++++++-------- include/wil/wistd_type_traits.h | 4 +++- tests/WatcherTests.cpp | 4 ++-- 3 files changed, 20 insertions(+), 11 deletions(-) diff --git a/include/wil/resource.h b/include/wil/resource.h index 82015308..1f5f4285 100644 --- a/include/wil/resource.h +++ b/include/wil/resource.h @@ -3953,14 +3953,16 @@ unique_event_watcher_nothrow make_event_watcher_nothrow( return watcher; // caller must test for success using if (watcher) } -inline unique_event_watcher_nothrow make_event_watcher_nothrow(_In_ HANDLE eventHandle, wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) WI_NOEXCEPT +inline unique_event_watcher_nothrow make_event_watcher_nothrow( + _In_ HANDLE eventHandle, wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) WI_NOEXCEPT { unique_event_watcher_nothrow watcher; watcher.create(eventHandle, wistd::move(callback), options); return watcher; // caller must test for success using if (watcher) } -inline unique_event_watcher_nothrow make_event_watcher_nothrow(wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) WI_NOEXCEPT +inline unique_event_watcher_nothrow make_event_watcher_nothrow( + wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) WI_NOEXCEPT { unique_event_watcher_nothrow watcher; watcher.create(wistd::move(callback), options); @@ -3970,17 +3972,20 @@ inline unique_event_watcher_nothrow make_event_watcher_nothrow(wistd::function unique_event_watcher_failfast make_event_watcher_failfast( unique_any_t, err_policy>>&& eventHandle, - wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) + wistd::function&& callback, + event_watcher_options options = event_watcher_options::reset_event) { return unique_event_watcher_failfast(wistd::move(eventHandle), wistd::move(callback), options); } -inline unique_event_watcher_failfast make_event_watcher_failfast(_In_ HANDLE eventHandle, wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) +inline unique_event_watcher_failfast make_event_watcher_failfast( + _In_ HANDLE eventHandle, wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) { return unique_event_watcher_failfast(eventHandle, wistd::move(callback), options); } -inline unique_event_watcher_failfast make_event_watcher_failfast(wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) +inline unique_event_watcher_failfast make_event_watcher_failfast( + wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) { return unique_event_watcher_failfast(wistd::move(callback), options); } @@ -3991,12 +3996,14 @@ typedef unique_any_t unique_event_watcher make_event_watcher( unique_any_t, err_policy>>&& eventHandle, - wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) + wistd::function&& callback, + event_watcher_options options = event_watcher_options::reset_event) { return unique_event_watcher(wistd::move(eventHandle), wistd::move(callback), options); } -inline unique_event_watcher make_event_watcher(_In_ HANDLE eventHandle, wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) +inline unique_event_watcher make_event_watcher( + _In_ HANDLE eventHandle, wistd::function&& callback, event_watcher_options options = event_watcher_options::reset_event) { return unique_event_watcher(eventHandle, wistd::move(callback), options); } @@ -4394,7 +4401,7 @@ struct hlocal_secure_deleter { if (ptr) { -#pragma warning(suppress : 26006 26007) // LocalSize() ensures proper buffer length +#pragma warning(suppress : 26006 26007) // LocalSize() ensures proper buffer length ::SecureZeroMemory(ptr, ::LocalSize(ptr)); // this is safe since LocalSize() returns 0 on failure ::LocalFree(ptr); } diff --git a/include/wil/wistd_type_traits.h b/include/wil/wistd_type_traits.h index 3966e931..fb2bbad7 100644 --- a/include/wil/wistd_type_traits.h +++ b/include/wil/wistd_type_traits.h @@ -1568,7 +1568,9 @@ struct wi_is_convertible (is_same::type, typename remove_cv::type>::type>::value || is_base_of::type, _T1>::value)) #endif - >{}; + > +{ +}; template struct wi_is_convertible<_T1, _T2, 0, 1> : public false_type diff --git a/tests/WatcherTests.cpp b/tests/WatcherTests.cpp index 66ca6545..122958ee 100644 --- a/tests/WatcherTests.cpp +++ b/tests/WatcherTests.cpp @@ -4,7 +4,7 @@ #include #include -#include // For shared_event_watcher +#include // For shared_event_watcher #include // NOLINT(readability-duplicate-include): Intentionally testing "light up" code #include "common.h" @@ -112,7 +112,7 @@ TEST_CASE("EventWatcherTests::DoNotResetEvent", "[resource][event_watcher]") REQUIRE(watcher != nullptr); watchedEvent.SetEvent(); REQUIRE(notificationReceived.wait(5000)); // 5 second max wait - REQUIRE(watchedEvent.is_signaled()); // event should still be signaled + REQUIRE(watchedEvent.is_signaled()); // event should still be signaled } TEST_CASE("EventWatcherTests::VerifyDelivery", "[resource][event_watcher]")