Event watchers might want to be passive

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,29 +30,33 @@
                 }
             },
             "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": [
         {
             "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": [

include/wil/resource.h

@@ -3800,15 +3800,20 @@ using unique_mapview_ptr = wistd::unique_ptr<details::ensure_trivially_destructi
 // globalStateWatcher.SetEvent(); // signal observers so they can update
 
 /// @cond
+enum class event_watcher_options
+{
+    none = 0x0,
+    reset_event = 0x1, // Reset the event during the watch callback
+};
+
+DEFINE_ENUM_FLAG_OPERATORS(event_watcher_options);
+
 namespace details
 {
     struct event_watcher_state
     {
-        event_watcher_state(unique_event_nothrow&& eventHandle, wistd::function<void()>&& callback) :
-            m_callback(wistd::move(callback)), m_event(wistd::move(eventHandle))
-        {
-        }
         wistd::function<void()> 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,53 +3846,55 @@ class event_watcher_t : public storage_t
     template <typename from_err_policy>
     event_watcher_t(
         unique_any_t<event_t<details::unique_storage<details::handle_resource_policy>, from_err_policy>>&& eventHandle,
-        wistd::function<void()>&& callback)
+        wistd::function<void()>&& callback,
+        event_watcher_options options = event_watcher_options::reset_event)
     {
         static_assert(wistd::is_same<void, result>::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<void()>&& callback)
+    event_watcher_t(_In_ HANDLE eventHandle, wistd::function<void()>&& callback, event_watcher_options options = event_watcher_options::reset_event)
     {
         static_assert(wistd::is_same<void, result>::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<void()>&& callback)
+    event_watcher_t(wistd::function<void()>&& callback, event_watcher_options options = event_watcher_options::reset_event)
     {
         static_assert(wistd::is_same<void, result>::value, "this constructor requires exceptions or fail fast; use the create method");
-        create(wistd::move(callback));
+        create(wistd::move(callback), options);
     }
 
     template <typename event_err_policy>
     result create(
         unique_any_t<event_t<details::unique_storage<details::handle_resource_policy>, event_err_policy>>&& eventHandle,
-        wistd::function<void()>&& callback)
+        wistd::function<void()>&& 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<void()>&& callback)
+    result create(wistd::function<void()>&& 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.
         if (FAILED(hr))
         {
             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<void()>&& callback)
+    result create(_In_ HANDLE eventHandle, wistd::function<void()>&& 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<details::event_watcher_state*>(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<void()>&& callback)
+    HRESULT create_take_hevent_ownership(_In_ HANDLE rawHandleOwnershipTaken, wistd::function<void()>&& callback, event_watcher_options options)
     {
         __FAIL_FAST_ASSERT__(rawHandleOwnershipTaken != nullptr); // invalid parameter
         unique_event_nothrow eventHandle(rawHandleOwnershipTaken);
         wistd::unique_ptr<details::event_watcher_state> 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,49 @@ typedef unique_any_t<event_watcher_t<details::unique_storage<details::event_watc
 template <typename err_policy>
 unique_event_watcher_nothrow make_event_watcher_nothrow(
     unique_any_t<event_t<details::unique_storage<details::handle_resource_policy>, err_policy>>&& eventHandle,
-    wistd::function<void()>&& callback) WI_NOEXCEPT
+    wistd::function<void()>&& 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<void()>&& callback) WI_NOEXCEPT
+inline unique_event_watcher_nothrow make_event_watcher_nothrow(
+    _In_ HANDLE eventHandle, wistd::function<void()>&& 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<void()>&& callback) WI_NOEXCEPT
+inline unique_event_watcher_nothrow make_event_watcher_nothrow(
+    wistd::function<void()>&& 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 <typename err_policy>
 unique_event_watcher_failfast make_event_watcher_failfast(
     unique_any_t<event_t<details::unique_storage<details::handle_resource_policy>, err_policy>>&& eventHandle,
-    wistd::function<void()>&& callback)
+    wistd::function<void()>&& 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<void()>&& callback)
+inline unique_event_watcher_failfast make_event_watcher_failfast(
+    _In_ HANDLE eventHandle, wistd::function<void()>&& 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<void()>&& callback)
+inline unique_event_watcher_failfast make_event_watcher_failfast(
+    wistd::function<void()>&& 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 +3996,16 @@ typedef unique_any_t<event_watcher_t<details::unique_storage<details::event_watc
 template <typename err_policy>
 unique_event_watcher make_event_watcher(
     unique_any_t<event_t<details::unique_storage<details::handle_resource_policy>, err_policy>>&& eventHandle,
-    wistd::function<void()>&& callback)
+    wistd::function<void()>&& 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<void()>&& callback)
+inline unique_event_watcher make_event_watcher(
+    _In_ HANDLE eventHandle, wistd::function<void()>&& 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<void()>&& callback)
@@ -4385,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);
         }

include/wil/wistd_type_traits.h

@@ -1568,7 +1568,9 @@ struct wi_is_convertible
                    (is_same<typename remove_cv<_T1>::type, typename remove_cv<typename remove_reference<_T2>::type>::type>::value ||
                     is_base_of<typename remove_reference<_T2>::type, _T1>::value))
 #endif
-          >{};
+          >
+{
+};
 
 template <class _T1, class _T2>
 struct wi_is_convertible<_T1, _T2, 0, 1> : public false_type

tests/WatcherTests.cpp

@@ -4,7 +4,7 @@
 #include <wil/registry.h>
 #include <wil/resource.h>
 
-#include <memory> // For shared_event_watcher
+#include <memory>         // For shared_event_watcher
 #include <wil/resource.h> // NOLINT(readability-duplicate-include): Intentionally testing "light up" code
 
 #include "common.h"
@@ -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();