Task.WaitAll gets stuck

Question

I have a piece of code that looks like this:

    var taskList = new Task<string>[masterResult.D.Count];
    for (int i = 0; i < masterResult.D.Count; i++)        //Go through all the lists we need to pull (based on master list) and create a task-list
    {
        using (var client = new WebClient())
        {
            Task<string> getDownloadsTask = client.DownloadStringTaskAsync(new Uri(agilityApiUrl + masterResult.D[i].ReferenceIdOfCollection + "?$format=json"));
            taskList[i] = getDownloadsTask;
        }
    }

    Task.WaitAll(taskList.Cast<Task>().ToArray());      //Wait for all results to come back

The code freezes after Task.WaitAll... I have an idea why, it's because client is already disposed at the time of calling, is it possible to delay its disposal until later? Can you recommend another approach?

Well, *test* the hypothesis: what happens when the `using` is moved around everything else? Does this confirm or reject the "idea"? — user2864740, Feb 16 '16 at 20:03
Is it running in an async function? Is it async all the way "up"? — Dave, Feb 16 '16 at 20:03
What is the outer code? It's important it's async all the way down the call stack. — weston, Feb 16 '16 at 20:04
The outer method is presumably irrelevant (and definitely not async) since `Task.WaitAll` is called. — user2864740, Feb 16 '16 at 20:04
You should push the `using` of the `client` to the outermost scope (around the whole thing, including the `WaitAll` — porges, Feb 16 '16 at 20:08
this was actually my initial approach, but sadly WebClient can only handle one request per instance — FailedUnitTest, Feb 16 '16 at 20:09
Since `taskList` is already a `Task[]` and arrays are co-variant, you don't need the `.Cast().ToArray()` — juharr, Feb 16 '16 at 20:10

Dave · Accepted Answer · 2016-02-16T20:25:27.947

3

You need to create and dispose the WebClient within your task. I don't have a way to test this, but see if points you in the right direction:

    var taskList = new Task<string>[masterResult.D.Count];
    for (int i = 0; i < masterResult.D.Count; i++)        //Go through all the lists we need to pull (based on master list) and create a task-list
    {
        taskList[i] = Task.Run(() =>
        {
            using (var client = new WebClient())
            {
                return client.DownloadStringTaskAsync(new Uri(agilityApiUrl + masterResult.D[i].ReferenceIdOfCollection + "?$format=json"));

            }
        });
    }

    Task.WaitAll(taskList.Cast<Task>().ToArray());

edited Feb 16 '16 at 20:25

answered Feb 16 '16 at 20:19

Dave

4,375
3
24
30

I got this method to work, except you will need to reassign the loop index to a new variable otherwise you will get an OutOfRange exception. More info here: http://stackoverflow.com/questions/2741870/for-loop-index-out-of-range-argumentoutofrangeexception-when-multithreading – FailedUnitTest Feb 18 '16 at 15:38
Yeah, when I did something like this I just used a `List` and then used `.ToArray()`. Not surprising something would go a little wonky with the loop index. – Dave Feb 18 '16 at 15:47

score 1 · Answer 2 · edited May 23 '17 at 11:50

1

I don't see how that code would ever work, since you dispose the WebClient before the task was run.

You want to do something like this:

var taskList = new Task<string>[masterResult.D.Count];
for (int i = 0; i < masterResult.D.Count; i++)        //Go through all the lists we need to pull (based on master list) and create a task-list
{
    var client = new WebClient();
    Task<string> task = client.DownloadStringTaskAsync(new Uri(agilityApiUrl + masterResult.D[i].ReferenceIdOfCollection + "?$format=json"));
    task.ContinueWith(x => client.Dispose());
    taskList[i] = task;
}

Task.WaitAll(taskList.Cast<Task>().ToArray());      //Wait for all results to come back

i.e. if you dispose the WebClient in the first loop, it's not allocated when you trigger the tasks by using Task.WaitAll. The ContinueWith call will be invoked once the task completes and can therefore be used to dispose each WebClient instance.

However, to get the code to execute concurrent requests to a single host you need to configure the service point. Read this question: Trying to run multiple HTTP requests in parallel, but being limited by Windows (registry)

edited May 23 '17 at 11:50

Community

1
1

answered Feb 16 '16 at 20:02

jgauffin

99,844
45
235
372

It doesn't, quite run, that is correct, it freezes when it tries. – FailedUnitTest Feb 16 '16 at 20:03
Wasnt me, but I am going to try your solution and +1 if works – FailedUnitTest Feb 16 '16 at 20:05
-1 for a comment posing as an answer, but even now, this is not the problem, and that's not good code in your suggestion. WebClient can and should be reused for multiple requests. – weston Feb 16 '16 at 20:06
@weston, webclient does not support concurrent requests, I already thought of this approach – FailedUnitTest Feb 16 '16 at 20:06
Maybe you can dispose clients on Task.ContinueWith instead of holding a separate list, right? – Mehrzad Chehraz Feb 16 '16 at 20:08
@weston: My answer is about async vs dispose in general. Haven't looked into WebClient – jgauffin Feb 16 '16 at 20:08
WebClient dispose does nothing, like a MemoryStream dispose. So this is not the issue. http://codereview.stackexchange.com/a/9726/12729 – weston Feb 16 '16 at 20:09
In that case, taking WebClient out of using should fix it. – FailedUnitTest Feb 16 '16 at 20:11
Except the WebClient is getting disposed before it can possibly have a chance to complete. So yeah, it probably is the issue. – Dave Feb 16 '16 at 20:11
weston: Not true, check reference source; http://referencesource.microsoft.com/#System/net/System/Net/webclient.cs – jgauffin Feb 16 '16 at 20:12
Weston: The dispose is the issue and you can invoke multiple webclients just like in the code. – jgauffin Feb 16 '16 at 20:13
weston: So again, what is the -1 for? – jgauffin Feb 16 '16 at 20:16
A non useful answer. – weston Feb 16 '16 at 20:17
1. WebClients can start multiple tasks – weston Feb 16 '16 at 20:17
2. Tasks WebClients start do not share a life span with the WebClient that started them – weston Feb 16 '16 at 20:18
WebClients do not support concurrent requests, which is the point of using asynchronous calls here. Therefore you cannot reuse instances of Webclient – FailedUnitTest Feb 16 '16 at 20:18
3. The dispose method you pointed me to is probably related to the many synchronous methods on WebClient. – weston Feb 16 '16 at 20:19
@weston Please atleast try before -1 for god sake! This is the error for multiple requests (System.NotSupportedException): WebClient does not support concurrent I/O operations. – Mehrzad Chehraz Feb 16 '16 at 20:21
@MehrzadChehraz When I downvoted it was just a comment. OP will try, then we will see. In meantime don't downvote my answer just because you don't agree with my comments on here. – weston Feb 16 '16 at 20:25
@weston I did test it and it does not support that, I didn't downvote for your comments, your answer is not relevent to the question. I'm gonna upvote Dave's answer since it is a cleaner approach. – Mehrzad Chehraz Feb 16 '16 at 20:28
It would appear I am wrong on concurrent requests and reuse of webclient. However it gives a very clear exception, not the deadlock they are apparently experiencing. – weston Feb 17 '16 at 11:36

Task.WaitAll gets stuck

2 Answers2