2

Here is a minimum reproducible problem:

public class Example
{
    private readonly List<int> listToCollectResults = new();
    
    private async Task SleepFor250msAndAppendToListAsync(int i)
    {
        await Task.Delay(250);
        listToCollectResults.Add(i);
    }

    public void RunThis()
    {
        for (int i = 0; i < 100; i++)
        {
            SleepFor250msAndAppendToListAsync(i); // spin up N number of Tasks
        }
        
        Console.WriteLine(listToCollectResults.Count); // expect 0, since 250ms probably hasnt elapsed
        Thread.Sleep(1000); // wait for all tasks to complete & appending to list, should be complete after this step
        
        Console.WriteLine(listToCollectResults.Count); // missing some tasks?!
    }
}

I get the following output:

0 // expected this

88 // why is it not 100

Why is it that there are 'missing' tasks?

Guru Stron
  • 102,774
  • 10
  • 95
  • 132
pe pe
  • 31
  • 2
  • 1
    You should `await` `SleepFor250msAndAppendToListAsync(i)` tasks: when you call `SleepFor250msAndAppendToListAsync(i)` without `await` you fire and foget it; the task will be completed somewhen in the future, but you don't wait for the task completion; after 1 second you have 88 out of 100 completed (it can vary from workstation to workstation) – Dmitry Bychenko Mar 18 '23 at 11:48
  • 1
    Because you are not `await`ing the tasks you start and there is no guarantee whatsoever when a task will be finished or not ... So your output can be anything from 0 to 100 – derpirscher Mar 18 '23 at 12:06
  • @derpirscher is not it the goal? See the comment on the `Console.WriteLine(listToCollectResults.Count); // expect 0, since 250ms probably hasnt elapsed` line. – Guru Stron Mar 18 '23 at 12:08
  • 1
    It is a pretty classic threading race. Task uses the threadpool to execute the tasks, they don't all run at the same time. Increase the number by sleeping longer, but you can only be actually sure when you use Task.WaitAll(). Another bug you haven't found yet is when you run this code in a gui app, it will remind you that a task needs to be awaiting or its result retrieved. – Hans Passant Mar 18 '23 at 12:08
  • Hi all, thanks for the comments! When I posted this question I did not know that: 1) async-await tasks required thread-safety and subject to race conditions - I always thought that concurrent tasks were all run on the same thread. 2) I wrongly assumed that the no-await Tasks (Fire and Forget Policy) would always be run to completion at some point in time. I did not know that the Fire-and-Forget Policy meant that some tasks would possibly be 'forgotten' and never started. – pe pe Mar 20 '23 at 05:46
  • The tasks are not forgotten, but they can be slow to start. After 250ms, there are 100 tasks ready to run, but the system isn't going to start 100 threads to run them all at once. It'll create maybe 2 threads, and run the tasks sequentially on those threads. So some of the tasks might resume after 300ms or 350ms, depending on how quickly the other tasks ahead of them can finish. – Raymond Chen Mar 20 '23 at 13:56

2 Answers2

1

88 // why is it not 100

Because List<T> is not a thread safe collection, it is subjected to race conditions, including it's Add method (see the int size = _size; _size = size + 1; part - almost classic "candidate" for race conditions).

You can use a collection from System.Collections.Concurrent namespace to mitigate that (or for experimental/testing purposes you can also synchronize the access via one of framework provided synchronization primitives).

For example using the ConcurrentBag:

var listToCollectResults = new ConcurrentBag<int>();

Which will result in expected output of 100.

Read more:

P.S.

You can leverage Task.WhenAll in your RunThis method instead of blocking wait with some "random" timeout:

async Task RunThis()
{
    // spin up N number of Tasks
    var tasks = Enumerable.Range(0, 100)
        .Select(SleepFor250msAndAppendToListAsync) // equivalent of  i => SleepFor250msAndAppendToListAsync(i)
        .ToArray();
        
    Console.WriteLine(listToCollectResults.Count); // expect 0, since 250ms probably hasnt elapsed
    await Task.WhenAll(tasks);// wait for all tasks to complete & appending to list, should be complete after this step
        
    Console.WriteLine(listToCollectResults.Count); // missing some tasks?!
}
Guru Stron
  • 102,774
  • 10
  • 95
  • 132
  • *"For example using the `ConcurrentBag`:"* -- The `ConcurrentBag` is probably [the worst concurrent collection](https://stackoverflow.com/questions/15521584/what-is-the-correct-usage-of-concurrentbag/75343790#75343790) for the purpose of storing results of a parallel operation. If you don't have a **mixed** producer-consumer scenario, don't use the `ConcurrentBag`. – Theodor Zoulias Mar 18 '23 at 15:47
0

you can use folowing code for safe thread :

public class Example
{
    private readonly SemaphoreSlim SemaphoreSlim = new SemaphoreSlim(1);
    private readonly List<int> listToCollectResults = new();
    private async Task SleepFor250msAndAppendToListAsync(int i)
    {
        await Task.Delay(250);
        await semaphore.WaitAsync();
        listToCollectResults.Add(i);
        semaphore.Release();
    }
    public async Task RunThis()
    {
        var tasks=new List<Task>();
        for (int i = 0; i < 100; i++)
            tasks.Add(SleepFor250msAndAppendToListAsync(i)); 
        
        Console.WriteLine(listToCollectResults.Count);          
        await Task.WhenAll(tasks);
        Console.WriteLine(listToCollectResults.Count); 
    }
}
BQF
  • 33
  • 7
  • 2
    "when a task is created with a specific thread". There is nothing in the original code that created tasks on specific threads. "when a thread terminates (interrupts or completes or fails), all the tasks it spawned become unavailable." Not only is this not true for multithreaded apartments (which is what the original code appears to be using), there is no thread termination in the original code anyway. – Raymond Chen Mar 18 '23 at 14:20
  • Your program is runned in a thread , it's by default . It's not important that your program is a apartment thread or a multiple thread. – BQF Mar 18 '23 at 14:33
  • The `await` keyword resumes execution in the same apartment by default. (You can change this with `ConfigureAwait`). If you have a multithreaded apartment, then the resumption can occur in any thread in that apartment. The original thread can exit. Exiting a thread does not cause you to lose access to the tasks that it spawned. You can still access those tasks via the Task object. Also: " the task cannot complete because there is no other task" - this also doesn't make sense. Completing a task is not dependent on having another task. – Raymond Chen Mar 18 '23 at 14:57
  • 2
    *"When a thread terminates all the tasks it spawned become unavailable."* -- This is not true. *"The `await` keyword resumes execution in the same apartment by default."* -- This isn't true either. Talking about a wrong answer with wrong comments attached to it. – Theodor Zoulias Mar 18 '23 at 15:42
  • What I mean by "when a thread has exhausted all the tasks it created" is that no one is waiting to do things anymore, so things don't get done and are forgotten. This conclusion is purely empirical. – BQF Mar 19 '23 at 07:04
  • It is better to correct this answer and add to those situations related to errors (failed tasks) , but overall I think I have given the right answer – BQF Mar 19 '23 at 07:07
  • *"when a thread has exhausted all the tasks it created"* -- [You've quoted](https://stackoverflow.com/questions/75775645/missing-results-from-async-await/75776229#comment133678005_75776229) this phrase indicating that you've said it before, but you haven't. Your comments are not less confusing than your answer. – Theodor Zoulias Mar 19 '23 at 07:24