search

StephenCleary / AsyncEx

A helper library for async/await.
MIT License
3.53k stars 356 forks source link

AsyncAutoResetEvent doesn't support Reset? #27

Closed ejsmith closed 9 years ago

ejsmith commented 9 years ago

I need to reset the event, but there isn't a Reset method like the normal AutoResetEvent has. Any reason for this?

StephenCleary commented 9 years ago

There isn't a "Reset" because the reset is... automatic. :)

I believe AutoResetEvent.Reset is due to dubious API design. That's what the Win32 docs say, anyway (and I've never called ResetEvent on an auto-reset event, myself).

Remember that in the event world, "set" means "available" and "unset" means "taken". In particular, "reset" does not mean "restore the original state"; it means "do not allow any other code to take this event".

So, if by "reset" you mean "move the event to the taken state", the best way to do that is to acquire the event by (a)waiting on it.

But if by "reset" you mean "restore the original state", the best way to do that is to actually replace the AsyncAutoResetEvent instance with a new one.

Does this make sense, or is there some useful semantic to Reset that I'm missing?

ejsmith commented 9 years ago

Here is where I am using the reset:

https://github.com/exceptionless/Foundatio/blob/master/src/Core/Queues/InMemoryQueue.cs#L134

The problem is that I am waiting for either of 2 things to occur. A new item to be added to the queue or a timeout. So if the queue count is greater than 0 then I just go right through and the event never gets reset because nothing waited for it. Then the queue item gets processed and the next dequeue call is called with 0 items and it checks the wait handle and it's already been triggered. Does that make sense?

StephenCleary commented 9 years ago

The code as it currently stands I believe has a race condition bug. Reset "acquires" the event in a way that it doesn't act as any kind of mutual exclusion.

A manual-reset event (or better yet, a monitor or condition variable) would be a better match for a queue.

ejsmith commented 9 years ago

I am currently using a manual reset event. You think that is a better fit, eh? I'm not using it for synchronization, I'm just using it to wait for a signal that a new item has been added to the queue. I am definitely open to suggestions. I am far from an expert in this area.

StephenCleary commented 9 years ago

You can use events for signals, but the more common approach (for a queue) is to use monitors. Or, in the non-.NET world, two condition variables with a lock (the BCL does not have the concept of condition variables, but a monitor is exactly the same as a condition variable with a lock).

I have a simple producer/consumer queue that uses condition variables with locks here: https://github.com/StephenCleary/AsyncEx/blob/master/Source/Nito.AsyncEx%20(NET45%2C%20Win8%2C%20WP8%2C%20WPA81)/AsyncProducerConsumerQueue.cs