TaskCompletionSource throws "An attempt was made to transition a task to a final state when it had already completed"

Hossein Narimani Rad picture Hossein Narimani Rad · Aug 3, 2015 · Viewed 15.1k times · Source

I want to use TaskCompletionSource to wrap MyService which is a simple service:

public static Task<string> ProcessAsync(MyService service, int parameter)
{
    var tcs = new TaskCompletionSource<string>();
    //Every time ProccessAsync is called this assigns to Completed!
    service.Completed += (sender, e)=>{ tcs.SetResult(e.Result); };   
    service.RunAsync(parameter);
    return tcs.Task;
}

This code is working well for the first time. But the second time I call ProcessAsync simply the event handler for the Completed is assign again (the same service variable is used every time) and thus it will execute twice! and the second time it throws this exception:

attempt transition task final state when already completed

I'm not sure, should I declare the tcs as a class level variable like this:

TaskCompletionSource<string> tcs;

public static Task<string> ProccessAsync(MyService service, int parameter)
{
    tcs = new TaskCompletionSource<string>();
    service.Completed -= completedHandler; 
    service.Completed += completedHandler;
    return tcs.Task;    
}

private void completedHandler(object sender, CustomEventArg e)
{
    tcs.SetResult(e.Result); 
}

I have to wrap many methods with different return types and this way I have to write lost of code, variables, event handlers so I'm not sure if this is the best practice in this scenarios. So is there any better way of doing this job?

Answer

i3arnon picture i3arnon · Aug 3, 2015

The issue here is that the Completed event is raised on each action but the TaskCompletionSource can only be completed once.

You can still use a local TaskCompletionSource (and you should). You just need to unregister the callback before completing the TaskCompletionSource. That way this specific callback with this specific TaskCompletionSource will never be called again:

public static Task<string> ProcessAsync(MyService service, int parameter)
{
    var tcs = new TaskCompletionSource<string>();
    EventHandler<CustomEventArg> callback = null;
    callback = (sender, e) => 
    {
        service.Completed -= callback;
        tcs.SetResult(e.Result); 
    };
    service.Completed += callback;
    service.RunAsync(parameter);
    return tcs.Task;
}

This will also solve the possible memory leak that you have when your service keeps references to all these delegates.

You should keep in mind though that you can't have multiple of these operations running concurrently. At least not unless you have a way to match requests and responses.