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?

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.