AutoResetEventAsync, am I missing something?

Question

So I wrote an asynchronous version of AutoResetEvent:

public sealed class AutoResetEventAsync {
 private readonly Task<bool> falseResult = Task.FromResult(false);
 private readonly ConcurrentQueue<Handler> handlers = new ConcurrentQueue<Handler>();
 private readonly Task<bool> trueResult = Task.FromResult(true);
 private int isSet;
 public AutoResetEventAsync(bool initialState) {
 this.isSet = initialState ? 1 : 0;
 }
 public void Set() {
 this.isSet = 1;
 Handler handler;
 // Notify first alive handler
 while (this.handlers.TryDequeue(out handler))
 if (handler.TryUnsubscribe()) {
 handler.Invoke();
 break;
 }
 }
 public Task<bool> WaitAsync() {
 return this.WaitAsync(CancellationToken.None);
 }
 public Task<bool> WaitAsync(TimeSpan timeout) {
 if (timeout == Timeout.InfiniteTimeSpan)
 return this.WaitAsync();
 var tokenSource = new CancellationTokenSource(timeout);
 return this.WaitAsync(tokenSource.Token);
 }
 public Task<bool> WaitAsync(CancellationToken cancellationToken) {
 // Short path
 if (this.TryReset())
 return this.trueResult;
 if (cancellationToken.IsCancellationRequested)
 return this.falseResult;
 // Wait for a signal
 var completionSource = new TaskCompletionSource<bool>(false);
 var handler = new Handler(() => {
 if (this.TryReset())
 completionSource.TrySetResult(true);
 });
 // Subscribe
 this.handlers.Enqueue(handler);
 if (this.TryReset()) {
 // Unsubscribe
 handler.TryUnsubscribe();
 return this.trueResult;
 }
 cancellationToken.Register(() => {
 // Try to unsubscribe, if success then the handler wasn't invoked so we could set false result,
 // else the handler was invoked and the result is already set. 
 if (handler.TryUnsubscribe())
 completionSource.TrySetResult(false);
 });
 return completionSource.Task;
 }
 private bool TryReset() {
 return Interlocked.CompareExchange(ref this.isSet, 0, 1) == 1;
 }
 private sealed class Handler {
 private readonly Action handler;
 private int isAlive = 1;
 public Handler(Action handler) {
 this.handler = handler;
 }
 public void Invoke() {
 this.handler();
 }
 public bool TryUnsubscribe() {
 return Interlocked.CompareExchange(ref this.isAlive, 0, 1) == 1;
 }
 }
}

In my application it works well.

The question is: am I missing some concurrency issue?

I tested concurrent invocation of WaitAsync method, so, maybe you could suggest a better test scenario?

• svick: The code is missing any public documentation, so one is left to wonder what exactly is the intended behavior. For example: it WaitAsync() supposed to be FIFO?

Answer 1

Your code does have race conditions, so it won't work correctly. I think your main problem is that you make some "destructive" operation (e.g. dequeue a handler from the queue) without knowing that the followup operation will actually succeed and not handling the eventual failure in any way.

For example, the following sequence of events could happen:

1. The class is created in unset state.
2. WaitAsync() is called, creating a Handler, adding it to the queue and returning a Task.
3. Set() is called, setting isSet to 1.
4. The handler from step 2 is dequeued.
5. WaitAsync() is called from another thread. Since isSet is 1, it sets isSet back to 0 and immediately returns a completed Task.
6. Back on the previous thread, TryUnsubscribe() is called in the handler from step 4 and succeeds.
7. The handler is Invoke()d, but TryReset() fails.
8. Set() now returns.

After this, the Task from step 2 is now orphaned, it will never be completed, because its handler is not in the queue anymore.

---

I suggest you don't write code like this by yourself, but instead use code written by others with more experience in concurrent programming. Specifically, you could use Stephen Toub's AsyncAutoResetEvent or AsyncAutoResetEvent from Stephen Cleary's AsyncEx library.

• SHSE: Thank you for that. I saw AsyncAutoResetEvent that you mention, but it uses lock. I want to make a lock-less implementation of AutoResetEvent to get an experience in concurrent programming. I also spot another concurrency issue in WaitAsync method where it ignores the result of TryUnsubscribe method.
• svick: @SHSE Yeah, I wouldn't be surprised if there were other issues in your code. I just answered with the first significant race I discovered.
• svick: @SHSE Also, I'm not sure trying to be lock-less is worth it here, since this code is not likely to be in a tight loop. Of course, if you're doing this to learn, that might not matter much.
• SHSE: I ended up with this code, it isn't lock-less but it hasn't any instance wide locks.