Wrapping a library that uses the Event-Based Asyncronous Pattern, for use with Async/Await

Question

I'm using the async/await pattern throughout my code. However, there's one API which used the event-based asyncronous pattern. I've read on MSDN, and several StackOverflow answers, that the way to do this is to use a TaskCompletionSource.

My code:

public static Task<string> Process(Stream data)
{
    var client = new ServiceClient();
    var tcs = new TaskCompletionSource<string>();

    client.OnResult += (sender, e) =>
    {
        tcs.SetResult(e.Result);
    };

    client.OnError += (sender, e) =>
    {
        tcs.SetException(new Exception(e.ErrorMessage));
    };

    client.Send(data);

    return tcs.Task;
}

And called as:

string result = await Process(data);

Or, for testing:

string result = Process(data).Result;

The method always returns very quickly, but neither of the events get triggered.

If I add tcs.Task.Await(); just before the return statement, it works, but this doesn't provide the asyncronous behavior I want.

I've compared with the various samples I've seen around the internet, but don't see any difference.

Answer 1

The problem lays in the fact that after your Process method terminates, your ServiceClient local variable is eligible for garbage collection and may be collected prior to the event firing, hence a race condition is in place.

To avoid this, I'd define ProcessAsync as an extension method on the type:

public static class ServiceClientExtensions
{
    public static Task<string> ProcessAsync(this ServiceClient client, Stream data)
    {
        var tcs = new TaskCompletionSource<string>();

        EventHandler resultHandler = null;
        resultHandler = (sender, e) => 
        {
            client.OnResult -= resultHandler;
            tcs.SetResult(e.Result);
        }

        EventHandler errorHandler = null;
        errorHandler = (sender, e) =>
        {
            client.OnError -= errorHandler;
            tcs.SetException(new Exception(e.ErrorMessage));
        };

        client.OnResult += resultHandler;
        client.OnError += errorHandler;

        client.Send(data);
        return tcs.Task;
    }
}

And consume it like this:

public async Task ProcessAsync()
{
    var client = new ServiceClient();
    string result = await client.ProcessAsync(stream);
}

Edit: @usr points out that usually, IO operations should be the ones keeping the reference to whom invoked them alive, which isn't the case as we're seeing here. I agree with him that this behavior is a bit peculiar, and should probably signal of some sort of design/implementation problem of the ServiceClient object. I'd advise, if possible, to look up the implementation and see if there is anything that might be causing the reference to not stay rooted.