Implement timeout methods in AsyncManualResetEvent

[jamesmcguirepro]

Looking at ManaulResetEvent, there are several WaitOne method overloads supporting a timeout argument. It would be ideal if AsyncManualResetEvent could support these signatures as well:

• WaitAsync(Int32 timeout)
• WaitAsync(Int32 timeout, CancellationToken cancellationToken)
• WaitAsync(TimeSpan timeout)
• WaitAsync(TimeSpan timeout, CancellationToken cancellationToken)
• Wait(Int32 timeout)
• Wait(TimeSpan timeout)

[StephenCleary]

I disagree. Using timeouts for synchronization primitives is problematic to begin with (they really encourage questionable code patterns).

I don't see any way I'll agree with adding int overloads, since those are essentially a holdover from p/Invoke signatures. I could possibly be persuaded to add TimeSpan signatures, but there would have to be a really good argument in favor of them. What's your use case?

[jamesmcguirepro]

I'm fine with no Int32 timeouts.

A few use cases:

1. We don't want to halt code execution in the case of a bug, so we have a very long timeout and report the issue to a monitoring service for further investigation.
2. We do not want long running unit tests. Failing quickly lets us know that our code is not working properly so we can address the issue.

[StephenCleary]

Next question: there are already CancellationToken overloads, which allow timeouts via cancellation tokens. Is there a reason these do not supply your needs?

[jamesmcguirepro]

1. ManualResetEvent does support a timeout param

2. Yes, in theory you could use a CancellationToken, but I cannot think of a way to get the timer to start at the correct time. Here's my best stab at it - maybe I'm just being dense, but it quickly turned into an ugly hack.

async Task doSomethingAsync(CancellationToken cancellationToken)
{
    var timeout = TimeSpan.FromMinutes(2);
    var resetEvent = new AsyncManualResetEvent(false);

    // do something and hooks into `resetEvent`

    var receivedSignal = await resetEvent.WaitAsync(cancellationToken, timeout).ConfigureAwait(false);
}

vs

async Task doSomethingAsync(CancellationToken cancellationToken)
{
    var timeout = TimeSpan.FromMinutes(2);
    var resetEvent = new AsyncManualResetEvent(false);

    // do something.  Hooks into `resetEvent`

    var cancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
    Task.Run(() => 
    {
        await Task.Delay(timeout).ConfigureAwait(false);
        cancellationTokenSource..Cancel();
    });

    bool receivedSignal = false;

    try 
    {
        await resetEvent.WaitAsync(cancellationToken).ConfigureAwait(false);
        receivedSignal = true;
    }
    catch (OperationCanceledException ex)
    {
        receivedSignal = false;
    }
}

[Pretasoc]

The CancellationTokenSource has a CancelAfter method. So you could write

async Task DoSomethingAsync(CancellationToken cancellationToken)
{
var timeout = TimeSpan.FromMinutes(2);
    var resetEvent = new AsyncManualResetEvent(false);

    // do something.  Hooks into `resetEvent`

    var cancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
    bool receivedSignal = false;

    try 
    {
        cancellationTokenSource.CancelAfter(timeout);
        await resetEvent.WaitAsync(cancellationToken).ConfigureAwait(false);
        receivedSignal = true;
    }
    catch (OperationCanceledException ex)
    {
        receivedSignal = false;
    }
}

[jamesmcguirepro]

Ok, so

async Task doSomethingAsync(CancellationToken cancellationToken)
{
    var timeout = TimeSpan.FromMinutes(2);
    var resetEvent = new AsyncManualResetEvent(false);

    // do something and hooks into `resetEvent`

    var receivedSignal = await resetEvent.WaitAsync(cancellationToken, timeout).ConfigureAwait(false);
}

vs.

async Task DoSomethingAsync(CancellationToken cancellationToken)
{
    var timeout = TimeSpan.FromMinutes(2);
    var resetEvent = new AsyncManualResetEvent(false);

    // do something.  Hooks into `resetEvent`

    var cancellationTokenSource = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken);
    bool receivedSignal = false;

    try 
    {
        cancellationTokenSource.CancelAfter(timeout);
        await resetEvent.WaitAsync(cancellationToken).ConfigureAwait(false);
        receivedSignal = true;
    }
    catch (OperationCanceledException ex)
    {
        receivedSignal = false;
    }
}

Slightly less hacky. Shouldn't a library abstract that clutter away from the consumer?

[Soon5]

Well, you could add the whole logic into an Extension Method of AsyncManualResetEvent. That way you have the "ugly" code only once, and I don't think it's that ugly.

Null checking asside, here some code.

public static async Task<bool> WaitAsync(this AsyncManualResetEvent mEvent, TimeSpan timeout, CancellationToken token = default)
        {
            var timeOut = new CancellationTokenSource(timeout);
            var comp = CancellationTokenSource.CreateLinkedTokenSource(timeOut.Token, token);

            try
            {
                await mEvent.WaitAsync(comp.Token).ConfigureAwait(false);
                return true;
            }
            catch (OperationCanceledException e)
            {
                if (token.IsCancellationRequested)
                    throw; //Forward OperationCanceledException from external Token
                return false; //Here the OperationCanceledException was raised by Timeout
            }
        }

Now you can do Exactly the Call that you want to do.

[xuejmnet]

 public async Task<bool> WaitAsync(this AsyncManualResetEvent asyncManualResetEvent,TimeSpan timeout, CancellationToken cancellationToken = default)
    {

        var eventTask = asyncManualResetEvent.WaitAsync(cancellationToken);
        if (await Task.WhenAny(eventTask, Task.Delay(timeout, cancellationToken)) == eventTask)
        {
           return true;
        }
       //complete task
        asyncManualResetEvent.Set();
        return false;
    }

[daniel-lerch]

@Soon5 in your code you dispose neither timeOut nor comp although CancellationTokenSource implements IDisposable. Are you sure that doesn't introduce a memory leak?

My modified code with using statements and without rethrowing exceptions:

    public static async Task<bool> WaitAsync(this AsyncManualResetEvent mEvent, TimeSpan timeout, CancellationToken token = default)
    {
        using var timeOut = new CancellationTokenSource(timeout);
        using var combined = CancellationTokenSource.CreateLinkedTokenSource(timeOut.Token, token);

        try
        {
            await mEvent.WaitAsync(combined.Token).ConfigureAwait(false);
            return true;
        }
        // Don't catch the OperationCanceledException from external Token
        catch (OperationCanceledException) when (!token.IsCancellationRequested) 
        {
            return false; //Here the OperationCanceledException was raised by Timeout
        }
    }

[Soon5]

You are right. Thanks for updating the code. Three years ago when I wrote that code, the using statements were very new and didn't yet make it into my usual behaviour. Also I read an article a while ago, that in some circumstances not disposing CTS can introduce Memory leaks, so I also now dispose them.

That code was never intended to be used out of the box, it was mere an Idea on how the requested feature could look like, that's why also omitted the usual input checking and stuff. Still it's good that you point out that missing stuff as well.