Implement isopen() for Base.Event #54503
Conversation
This is useful to know if the event is signaled or not without checking the `set` field, and it matches the API of `AsyncCondition` and `Timer`.
| !!! compat "Julia 1.12" | ||
| `isopen(::Event)` requires at least Julia 1.12. | ||
| """ |
There was a problem hiding this comment.
This compat notice should be part of an Event-specific docstring on the isopen definition below. The generic isopen has a docstring that isn't really applicable to Event, since it talks about objects that produce events.
There was a problem hiding this comment.
Right, on second thoughts this is actually a confusing implementation since Event can produce multiple events with autoreset=True. Changed the implementation in 70f719e to match isopen() better.
There was a problem hiding this comment.
This has time-traveling behavior that makes it mostly not usable due to the lack of use of atomics, that I don't really think is desirable. But maybe just making it seq_cst is enough to make it useable as an API? It must document the threading synchronization guarantees whatever they are as well
| tasks will no longer block when waiting for it, until `reset` is called. | ||
| tasks will no longer block when waiting for it, until [`reset(::Event)`](@ref) | ||
| is called. Use [`isopen`](@ref) to check if it can still be signaled without | ||
| having to call [`reset(::Event)`](@ref). |
There was a problem hiding this comment.
I don't know what to is means, as an event can be signaled again even if not reset, and the wording seems to imply some sort of seq_cst behavior, which isn't something that multiple operations (check and reset) is valid terminology to have independently. The behavior when autoreset also seems contradictory to the statements here, as it talks about isopen being a property of writing (which is commonly true), but then associates it with reset, which is a reader operation and I don't think that synchronizes with writes, broadly speaking
There was a problem hiding this comment.
Maybe signaled is the wrong word, what I mean is that isopen can be used to check if the event will ever change from a un-set state to a set state again without having to explicitly reset it. Or put another way, if calling notify on the Event will have any effect.
The behavior when autoreset also seems contradictory to the statements here, as it talks about isopen being a property of writing (which is commonly true), but then associates it with reset, which is a reader operation and I don't think that synchronizes with writes, broadly speaking
I'm not sure I quite understand what you mean, but I think the confusion might be because of the phrase "be signaled" coming across as 'you can signal this' instead of 'the state can change to signaled'. An alternative wording could be something like "Use isopen to check if it will ever block again while waiting without having to call reset"?
There was a problem hiding this comment.
"will have" sounds like a guarantee of some sort though, but it comes with so many asterisks (that it is not autoreset, that there is only one producer calling notify, and that the consumer does not call reset), I find the combination seems confusing how it would be used in practice
Ah, should I be using |
|
yes, just |
This is useful to know if the event is signaled or not without checking the, and it matches the API ofsetfieldAsyncConditionandTimer.