AutoResetEventAsync, am I missing something?

So I wrote an asynchronous version of AutoResetEvent:

```
public sealed class AutoResetEventAsync {
private readonly Task falseResult = Task.FromResult(false);
private readonly ConcurrentQueue handlers = new ConcurrentQueue();
private readonly Task 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 WaitAsync() {
return this.WaitAsync(CancellationToken.None);
}

public Task WaitAsync(TimeSpan timeout) {
if (timeout == Timeout.InfiniteTimeSpan)
return this.WaitAsync();

var tokenSource = new CancellationTokenSource(timeout);
return this.WaitAsync(tokenSource.Token);
}

public Task 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(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.Tr

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:

• The class is created in unset state.

• WaitAsync() is called, creating a Handler, adding it to the queue and returning a Task.

• Set() is called, setting isSet to 1.

• The handler from step 2 is dequeued.

• WaitAsync() is called from another thread. Since isSet is 1, it sets isSet back to 0 and immediately returns a completed Task.

• Back on the previous thread, TryUnsubscribe() is called in the handler from step 4 and succeeds.

• The handler is Invoke()d, but TryReset() fails.

• 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.

StackExchange Code Review Q#23522, answer score: 5