How to wait for all task includes task.factory

Question

private async Task StartRequesting()
{
   for (int i = 0; i < 50; i++)
   {
      await Task.Factory.StartNew(async () =>
      {
          string Result =  await GetAsync("www.google.com");
          Console.WriteLine(Result);
     });
   }
}

Console.WriteLine("Starting Request");

await StartRequesting();

Console.WriteLine("Requesting has been finished");

How can I now wait that all the Task has been finished? I tryed Task.WhenAll but the Result is then like that

Starting Request
(google result)
(google result)
(google result)
Requesting has been finished
(google result)
(google result)

Note: I dont want use it without Task.Factory.StartNew because it takes then too long to done all the request.

1. You should use `Task.Run` instead of `Task.Factory.StartNew`. 2. There should be no need for it, as GetAsync is already async. Why wrap an I/O-bound task inside a CPU-bound task and wasting resources? 3. How have you done `Task.WhenAll`? It should be: `var tasks = new List(); for (int i=0; i<50; i++) { tasks.Add(GetAsync("www.google.com").ContinueWith(task => Console.WriteLine(task.Result)); } await Task.WhenAll(tasks);`. — ckuri, Apr 30 '20 at 19:28
@ckuri testing rightt now will. giving you feedback some minutes — ressana, Apr 30 '20 at 19:46
@ckuri well that works. but I need to create a new task when calling `await StartRequesting()` to prevent deadlock — ressana, Apr 30 '20 at 20:00
okay weird I think I fixed that without using a new Task. I have another question. why should I use Task.Run instead of Task.Factory.StartNew? I saw on some websites that these are same ? — ressana, Apr 30 '20 at 20:13
This could be if your GetAsync method has lots of expensive non-async code before the first await in the method, as everything before the first await is executed on the calling thread. — ckuri, Apr 30 '20 at 20:18
[`StartNew` is a dangerous, low-level method](https://blog.stephencleary.com/2013/08/startnew-is-dangerous.html). `Task.Run` is a modern replacement for it. — Stephen Cleary, Apr 30 '20 at 23:04
@StephenCleary could you share your version how you would do it? — ressana, May 01 '20 at 00:10
I would do it [this way](https://stackoverflow.com/a/61532055/263693). — Stephen Cleary, May 01 '20 at 01:19
but the UI hangs try it out with a WinForm. anyway that one from @ckuri works great — ressana, May 01 '20 at 02:12

score 3 · Accepted Answer · answered Apr 30 '20 at 20:20

3

I always find the cleanest way to do this is via LINQ, assuming there isn't a limit on concurrent requests:

private async Task StartRequesting()
{
    var tasks = Enumerable.Range(1, 50)
        .Select(async _ =>
        {
            string Result = await GetAsync("www.google.com");
            Console.WriteLine(Result);
        });

    await Task.WhenAll(tasks);
}

No blocking occurs and no Tasks need to be manually created.

answered Apr 30 '20 at 20:20

Johnathan Barclay

18,599
1
22
35

ok well thats nice. how can that be done if there is a generic list instead of number? – ressana Apr 30 '20 at 20:28
I tested that but the UI is hanging when using this way – ressana Apr 30 '20 at 20:50
also adding a await Task.Delay() does nothing – ressana Apr 30 '20 at 20:51
its the same code from you. if you change the value from 50 to 250 you can see it ( I mean it dosn't matter if 50 or 5000 the ui hangs – ressana Apr 30 '20 at 21:00
Its the GetAsync function from this post https://stackoverflow.com/a/27108442/13436341 – ressana Apr 30 '20 at 21:35
Just realised your writing a console app which doesn't have a UI thread. `StartRequesting` will only return once all 50 HTTP requests have completed; this will take an amount of time, irrespective of whether the requests happen in serial or parallel. – Johnathan Barclay May 01 '20 at 07:16

Kevin B Burns · Answer 2 · 2020-04-30T21:10:52.793

-1

It would be easier, cleaner and more efficent to throw all the tasks into a list and get the results after everything is done.

 private async Task StartRequesting()
        {
            var taskList = new List<Task<string>>();

            for (var i = 0; i < 50; i++)
                taskList.Add(Task.Run(async () => await GetAsync("www.google.com")));

            var tWhenAll = await Task.WhenAll(taskList.ToArray());

            foreach(var t in taskList)
                Console.WriteLine(t.Result);
        }

edited Apr 30 '20 at 21:10

answered Apr 30 '20 at 20:18

Kevin B Burns

1,032
9
24

2

There are multiple issues with this code. 1. Don’t wrap I/O-bound tasks within CPU-bound tasks (if there is no good reason like heavy CPU-bound work). You are wasting 50 (!) threads here and threads are very expensive. 2. The `if` block is unnecessary and will never be called as `await` will automatically throw if an exception occurs. 3. Not an issue per se, but this code will print the results only when all tasks/requests have finished, whereas OP’s code prints the results as soon as they become available. – ckuri Apr 30 '20 at 20:21
@ckuri, Per op 'How can I now wait that all the Task has been finished?' Also, 50 threads that run at 2 seconds a pop, compared to running each task 50 times at 2 seconds a pop is worth the "expensiveness" of a thread. But I changed my code, since my code = await anyways, it would do the same thing either way. Just cleaner the way you suggested. – Kevin B Burns Apr 30 '20 at 21:12

How to wait for all task includes task.factory

2 Answers2