4

This is the original code that had been running fine for a few weeks. In a test I just did, it failed 0 out of 100 attempts.

using (var httpClient = new HttpClient())
{
    var tasks = new List<Task>();

    tasks.Add(httpClient.GetAsync(new Uri("..."))
        .ContinueWith(request =>
        {
            request.Result.Content.ReadAsAsync<IEnumerable<Foo>>()
                .ContinueWith(response =>
                {
                    foos = response.Result;
                });
        }));

    tasks.Add(httpClient.GetAsync(new Uri("..."))
        .ContinueWith(request =>
        {
            request.Result.Content.ReadAsAsync<Bar>()
                .ContinueWith(response =>
                {
                    bar = response.Result;
                });
        }));

    await Task.WhenAll(tasks);
}

This code failed 9 out of 100 attempts, where one or both of the tuple values is null.

var APIresponses = await HttpClientHelper.GetAsync
    <
        IEnumerable<Foo>,
        Bar
    >
    (
        new Uri("..."),
        new Uri("...")
    );

foos = APIresponses.Item1;
bar = APIresponses.Item2;
private static Task GetAsync<T>(HttpClient httpClient, Uri URI, Action<Task<T>> continuationAction)
{
    return httpClient.GetAsync(URI)
        .ContinueWith(request =>
        {
            request.Result.EnsureSuccessStatusCode();

            request.Result.Content.ReadAsAsync<T>()
                .ContinueWith(continuationAction);
        });
}

public static async Task<Tuple<T1, T2>> GetAsync<T1, T2>(Uri URI1, Uri URI2)
{
    T1 item1 = default(T1);
    T2 item2 = default(T2);

    var httpClient = new HttpClient();
    var tasks = new List<Task>()
    {
        GetAsync<T1>(httpClient, URI1, response =>
        {
            item1 = response.Result;
        }),
        GetAsync<T2>(httpClient, URI2, response =>
        {
            item2 = response.Result;
        })
    };

    await Task.WhenAll(tasks);

    return Tuple.Create(item1, item2);
}

Modify the code to look like this, and it will again fail 0 out of 100 attempts.

    await Task.WhenAll(tasks);
    System.Diagnostics.Debug.WriteLine("tasks complete");
    System.Diagnostics.Debug.WriteLine(item1);
    System.Diagnostics.Debug.WriteLine(item2);

    return Tuple.Create(item1, item2);
}

I've been looking at this for over half an hour but I don't see where the mistake is. Does anyone see it?

user247702
  • 23,641
  • 15
  • 110
  • 157
  • sounds like some kind of race condition... (sorry I can't be more helpful!) – DaveDev Oct 04 '13 at 07:45
  • @DaveDev No worries, at least you're confirming what my colleague said :) – user247702 Oct 04 '13 at 07:51
  • `HttpClient` is not thread-safe. Have you tried using separate instance of it for each request? – svick Oct 04 '13 at 11:11
  • @svick According to [MSDN](http://msdn.microsoft.com/en-us/library/system.net.http.httpclient.aspx), `GetAsync` is one of the methods that are thread-safe. I'll see what happens with separate instances. – user247702 Oct 04 '13 at 11:22
  • @Stijn Oh, I looked at the “Any instance members are not guaranteed to be thread safe.” part and didn't notice that list of thread-safe methods. They could have made this clearer. – svick Oct 04 '13 at 11:23
  • @svick I tried just to be sure, but it only took a few attempts to have it crash again, with separate `HttpClient` instances. – user247702 Oct 04 '13 at 11:27
  • Not familiar with async/await, but I guess it might have to do with TaskCreationOptions, see http://stackoverflow.com/q/16499542/1236044 – jbl Oct 04 '13 at 12:02
  • @jbl That might be the key to the solution. I tried playing around with `Task.Factory` but couldn't get it to work, I'm not familiar enough with async yet, I'm afraid. – user247702 Oct 04 '13 at 13:21
  • What are the types of item1 and item2 are they value types? – Matt Smith Oct 04 '13 at 13:37
  • @MattSmith No, they're reference types. – user247702 Oct 04 '13 at 14:40

3 Answers3

2

This code:

        request.Result.Content.ReadAsAsync<T>()
            .ContinueWith(continuationAction);

returns a task, but that task is never awaited (and no Continuation is added to it). So the item's might not get set before Task.WhenAll returns.

However, the original solution seems to have the same problem.

My guess is that you are dealing with value types, and that both have a race condition, but in the 2nd example, you copy the value types early enough (while they are still their default value) into the Tuple. Where as in your other examples you wait long enough before copying them or using them such that the problem continuation that sets the values has run.

Matt Smith
  • 17,026
  • 7
  • 53
  • 103
  • 1
    They're reference types, but I think you're on the right track. Together with what @jbl said, I think `Task.WhenAll` does wait on the root tasks, but not on their continuations. I'll do some more research. – user247702 Oct 04 '13 at 14:47
2

To address the comment from to your other question, you very rarely need to mix async/await with ContinueWith. You can do the "fork" logic with help of async lambdas, e.g., the code from the question may look like this:

using (var httpClient = new HttpClient())
{
    Func<Task<IEnumerable<Foo>>> doTask1Async = async () =>
    {
        var request = await httpClient.GetAsync(new Uri("..."));
        return response.Content.ReadAsAsync<IEnumerable<Foo>>();
    };

    Func<Task<IEnumerable<Bar>>> doTask2Async = async () =>
    {
        var request = await httpClient.GetAsync(new Uri("..."));
        return response.Content.ReadAsAsync<IEnumerable<Bar>>();
    };

    var task1 = doTask1Async();
    var task2 = doTask2Async();

    await Task.WhenAll(task1, task2);

    var result1 = task1.Result;
    var result2 = task2.Result;

    // ...
}
Community
  • 1
  • 1
noseratio
  • 59,932
  • 34
  • 208
  • 486
1

Edit: unaccepting my own answer, but leaving it for reference. The code works, with a catch: ContinueWith loses the SynchronizationContext


Thanks to @jbl and @MattSmith for putting me on the right track.

The problem was indeed that Task.WhenAll does not wait on the continuations. The solution is to set TaskContinuationOptions.AttachedToParent.

So this

private static Task GetAsync<T>(HttpClient httpClient, Uri URI, Action<Task<T>> continuationAction)
{
    return httpClient.GetAsync(URI)
        .ContinueWith(request =>
        {
            request.Result.EnsureSuccessStatusCode();

            request.Result.Content.ReadAsAsync<T>()
                .ContinueWith(continuationAction);
        });
}

becomes this

private static Task GetAsync<T>(HttpClient httpClient, Uri URI, Action<Task<T>> continuationAction)
{
    return httpClient.GetAsync(URI)
        .ContinueWith(request =>
        {
            request.Result.EnsureSuccessStatusCode();

            request.Result.Content.ReadAsAsync<T>()
                .ContinueWith(continuationAction, TaskContinuationOptions.AttachedToParent);
        }, TaskContinuationOptions.AttachedToParent);
}

More info available on MSDN: Nested Tasks and Child Tasks

Community
  • 1
  • 1
user247702
  • 23,641
  • 15
  • 110
  • 157