Event watchers might want to be passive

[jonwis]

Some watchers only want to ... watch? Allow passing a flag down to control whether an event watcher calls ResetEvent during its callback. The default is still "yes, reset" so existing code compiles and works the same.

This fixes #482

[oldnewthing]

I don't think this works. If the watcher is passive, then it will just fire continuously because nobody is resetting the event.

[jonwis]

I don't think this works. If the watcher is passive, then it will just fire continuously because nobody is resetting the event.

The issue is because a user-owned process wanted to react to a doorknock auto-reset event owned by a service. The service handed out a SYNCHRONIZE-rights handle (or it was opened by name and had an ACL on it, I don't remember.) Attempting to ResetEvent from the process failed and failfasted.

For watcher-owned events, I agree that reset-and-rearm is likely correct. For events the watcher is asked to watch, it seems like a flag to control "should you reset" and "should you rearm" might be good.

I had a version of this that exposed both "reset the event" and "rearm the wait" ... that variant would need a way to request that the watcher rearm.

What do you think?

[dunhor]

Two comments:

1. Putting the flags as the last argument allows specifying a default argument, but breaks the general "function arguments go last" guidance so that passing a lambda looks cleaner (no "random" arguments appearing after the lambda). This would require new overloads - currently 6 by my count - which isn't too bad, but 12 creation functions borders the line of too many.
2. I like the idea of having both reset and re-arm options. Provides an easy way of creating a "single shot" watcher

[oldnewthing]

The issue is because a user-owned process wanted to react to a doorknock auto-reset event owned by a service. ...

Ah, I see. This is basically "Is this a manual-reset event or an auto-reset event that I'm watching?"

If it's a manual-reset event, then I have to reset it myself.

If it's an auto-reset event, then I should allow the wait completion do the reset.

So maybe the flag is

enum class event_watcher_options
{
    is_auto_reset_event = 0x0, // allow the wait completion to reset the event automatically
    is_manual_reset_event = 0x1, // Reset the event during the watch callback
};

And if we also want to have the re-arm flag in there

enum class event_watcher_flags
{
    none = 0x0,
    manual_reset = 0x1,
    single_shot = 0x2, // or "one_time"
};
DECLARE_ENUM_FLAG_NONSENSE(event_watcher_flags);

[dunhor]

Interesting... This had a new almost certainly unrelated failure here:

-------------------------------------------------------------------------------
EnumThreadWindows
-------------------------------------------------------------------------------
C:\a\wil\wil\tests\WindowingTests.cpp(71)
...............................................................................

C:\a\wil\wil\tests\WindowingTests.cpp(147): FAILED:
  REQUIRE_THROWS_AS( wil::for_each_thread_window( thread_id, [](HWND) { throw std::exception(); }), std::exception )
because no exception was thrown where one was expected:

[jonwis]

OK, I'm going to try again thusly:

1. Flags of manual_reset (ie: don't call ResetEvent), manual_start (ie: don't call SetThreadpoolWait)
2. Can only be used if the caller is passing in an event handle (not for self-created ones)
3. Adding a "start()" that calls SetThreadpoolWait for manual_start events
4. Limited overloads

[jonwis]

Interesting... This had a new almost certainly unrelated failure here:

-------------------------------------------------------------------------------
EnumThreadWindows
-------------------------------------------------------------------------------
C:\a\wil\wil\tests\WindowingTests.cpp(71)
...............................................................................

C:\a\wil\wil\tests\WindowingTests.cpp(147): FAILED:
  REQUIRE_THROWS_AS( wil::for_each_thread_window( thread_id, [](HWND) { throw std::exception(); }), std::exception )
because no exception was thrown where one was expected:

Yeah... I don't have an arm64 machine at home, I'll check tomorrow. Super strange.

Also still seeing the ASAN warnings for amd64 not understanding a new instruction sequence in win11's ntdll. :|

[dunhor]

Yeah... I don't have an arm64 machine at home, I'll check tomorrow. Super strange.

No need to check. Basically all CI runs started randomly failing in the same place yesterday. I'll have to take a look and maybe disable the test temporarily

Also still seeing the ASAN warnings for amd64 not understanding a new instruction sequence in win11's ntdll. :|

Sounds like an LLVM bug & unlikely at play here

[dunhor]

Okay, I've created #605 to address the test stability