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

Question

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?

i3arnon · Accepted Answer · 2015-08-03T13:28:37.757

35

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.

edited Aug 03 '15 at 13:28

answered Aug 03 '15 at 13:06

i3arnon

113,022
33
324
344

@HosseinNarimaniRad Not necessarily. You can still use a local TCS with a lambda expression, though it's not that simple. Look at the code in my answer. – i3arnon Aug 03 '15 at 13:11
It say cannot implicitly convert Action<...,...> to EventHandler<...> at `service.Completed-=callback;` so I fix it by changing the type. – Hossein Narimani Rad Aug 03 '15 at 13:20
It works. but the `service.Completed -=callback` looks like a magic. How it compare this newly created variable with the previously assigned one and match them? – Hossein Narimani Rad Aug 03 '15 at 13:28
@HosseinNarimaniRad the callback is an instance just like any other object. It gets captured by the lambda expression just like the tcs does. It doesn't use a variable. It uses the instance inside that variable. – i3arnon Aug 03 '15 at 13:30

score 5 · Answer 2 · edited May 23 '17 at 12:07

5

It appears that MyService will raise the Completed event more than once. this causes SetResult to be called more than once which causes your error.

You have 3 options that I see. Change the Completed event to only be raised once (Seems odd that you can complete more than once), change SetResult to TrySetResult so it does not throw a exception when you try to set it a 2nd time (this does introduce a small memory leak as the event still gets called and the completion source still tries to be set), or unsubscribe from the event (i3arnon's answer)

edited May 23 '17 at 12:07

Community

1
1

answered Aug 03 '15 at 13:34

Scott Chamberlain

124,994
33
282
431

Yes, I prefer the 3rd option. But honestly I cannot figure out how the newly instantiated `callback` match the previous one and using `service.Completed -=callback` it will be removed. I thought everytime we create a new `callback` it does not refer to the same handler so I thought I have to declared the handler in the class body. – Hossein Narimani Rad Aug 03 '15 at 13:43
1

It is a variable capture, it is the same "problem" that you get when you use the `i` [from a `for` loop](http://stackoverflow.com/questions/8116709/odd-lambda-behavior) inside a lambda. The variable `callback` inside the lambda is the same variable so it can be used inside the lambda even though it has not been initialized yet. You are correct that it creates a new callback every time, but you are keeping a reference to the anonymous callback so you can unsubscribe later (and that "later moment" just happens to be inside the callback itself). – Scott Chamberlain Aug 03 '15 at 13:46
You means the reference to this anonymous callback is the same for every execution of `ProcessAsync`? – Hossein Narimani Rad Aug 03 '15 at 13:53
2

@HosseinNarimaniRad no, it isn't. There's a new reference each time that line is executed. Just like there's a new CTS for each call to that method. – i3arnon Aug 03 '15 at 13:57
@ScottChamberlain Oh Yes. Now I understand the idea correctly. Thanks for your explanation. – Hossein Narimani Rad Aug 03 '15 at 13:59

score 3 · Answer 3 · edited May 23 '17 at 11:45

An alternative solution to i3arnon's answer would be:

public async static Task<string> ProcessAsync(MyService service, int parameter)
{
    var tcs = new TaskCompletionSource<string>();

    EventHandler<CustomEventArg> callback = 
        (s, e) => tcs.SetResult(e.Result);

    try
    {
        contacts.Completed  += callback;

        contacts.RunAsync(parameter);

        return await tcs.Task;
    }
    finally
    {
        contacts.Completed  -= callback;
    }
}

However, this solution will have a compiler generated state machine. It will use more memory and CPU.

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

3 Answers3

Linked