1
\$\begingroup\$

I'm trying to construct something which would work a bit like Zip for two tasks, but I'm a bit worried about race conditions in this code.

public static Task<R> ContinueWhenBoth<T1, T2, R>(this TaskFactory<R> factory, Task<T1> t1, 
                                                 Task<T2> t2, Func<T1, T2, R> f)
{
    var result = new TaskCompletionSource<R>();

    t1.ContinueWith(t => { if(t2.IsCompleted) result.TrySetResult(f(t1.Result, t2.Result)); });
    t2.ContinueWith(t => { if(t1.IsCompleted) result.TrySetResult(f(t1.Result, t2.Result)); });

    return result.Task;
}

Is this a good way to go?

public static Task<R> ContinueWhenBoth<T1, T2, R>(this TaskFactory<R> factory, Task<T1> t1, 
                                                 Task<T2> t2, Func<T1, T2, R> f)
{
    var result = new TaskCompletionSource<R>();

    int tasksLeft = 2;
    t1.ContinueWith(t => 
                    {
                    if(Interlocked.Decrement(ref tasksLeft) == 0 && t2.IsCompleted) 
                        result.TrySetResult(f(t1.Result, t2.Result)); 
                    });
    t2.ContinueWith(t => 
                    {
                    if(Interlocked.Decrement(ref tasksLeft) == 0 && t1.IsCompleted) 
                        result.TrySetResult(f(t1.Result, t2.Result)); 
                    });

    return result.Task;
}
\$\endgroup\$
1
\$\begingroup\$

You don't need to write this code yourself, there is already a method that does something very similar: Task.Factory.ContinueWhenAll().

ContinueWhenAll() doesn't directly support working with Tasks that return different types, but you can create a helper method that does:

public static Task<R> ContinueWhenBoth<T1, T2, R>(
    this TaskFactory<R> factory, Task<T1> t1, Task<T2> t2, Func<T1, T2, R> f)
{
    return factory.ContinueWhenAll(new Task[] { t1, t2 }, _ => f(t1.Result, t2.Result));
}

If you didn't want to use closure for performance reasons, you could use the array of Tasks and casting:

return factory.ContinueWhenAll(
    new Task[] { t1, t2 }, tasks => f(((Task<T1>)tasks[0]).Result, ((Task<T2>)t2).Result));

t1.ContinueWith(t => { if(t2.IsCompleted) result.TrySetResult(f(t1.Result, t2.Result)); });
t2.ContinueWith(t => { if(t1.IsCompleted) result.TrySetResult(f(t1.Result, t2.Result)); });

This code has a race condition: if t1 and t2 complete at the same time, then f might be executed twice. I don't think that's a good idea, since f might take a long time, or, worse, it might have side-effects.

if(Interlocked.Decrement(ref tasksLeft) == 0 && t2.IsCompleted) 

The Decrement() here avoids the race condition from the previous code. But it also means that the check of t2.IsCompleted will always succeed, so you should remove it.


But your code also has a significant issue: if t1 or t2 fail with an exception, your resulting Task will never complete. (And on .Net 4.0, it will also crash the process some time in the future, because there is an uncaught exception in a Task.)

\$\endgroup\$
4
  • \$\begingroup\$ Can it combine input tasks of different types? \$\endgroup\$
    – Benjol
    Jun 19 '13 at 8:06
  • \$\begingroup\$ @Benjol See edit. \$\endgroup\$
    – svick
    Jun 19 '13 at 8:15
  • \$\begingroup\$ OK, I see. I'll try that. One thing I don't understand is you say that t2.IsCompleted will always succeed: surely only if t2 completes first? \$\endgroup\$
    – Benjol
    Jun 19 '13 at 8:37
  • \$\begingroup\$ @Benjol I mean, if the first condition succeeds, the second one will too, so there is no reason to check it. \$\endgroup\$
    – svick
    Jun 19 '13 at 9:15
0
\$\begingroup\$

Any chance you are using .NET 4.5? It contains the method that does exactly what you need: Task.WhenAll.

\$\endgroup\$
1
  • \$\begingroup\$ No quite (I think), because my tasks have different types \$\endgroup\$
    – Benjol
    Jun 19 '13 at 7:09

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.