Problem of having intermittent exception of ArgumentException when adding item in List<T> wrapped in Task.WhenAll

[eriawan]

Description

I am trying to test some codes/logics that basically testing the speed or perf of adding item to a List and then this code is wrapped inside a Task.Run, then the code in the Task.Run is awaited, added to a List, and then all of the tasks are awaited into a single Task.WhenAll.

Unfortunately I got exceptions intermittently thrown when adding item to a List.

Reproduction Steps

This is the code to reproduce on my gist at GitHub:
https://gist.github.com/eriawan/65d38acbdd3649f394d6eff0f1faa434

Just run the console project and the code files in that Gist.

This method is excerpted from the Gist, note the line that add the item to randomResult, this is the point that throws exception.

        public static async Task TaskWhenAll_ProcessIterateFor()
        {
            List<Task> tasklist = new List<Task>();
            List<Double> randomResult = new List<Double>();
            for (int i = 1; i <= 50; i++)
            {
                tasklist.Add(Task.Run(() =>
                {
                    var iterationResult = ProcessIterateFor(60);
                    iterationResult.ForEach((resultItem) => randomResult.Add(resultItem));
                }));
            }
            await Task.WhenAll(tasklist);
        }

Expected behavior

I expect that it should not be having/throwing error/exception at all.

Actual behavior

This exception is intermittently thrown:

• Type: System.ArgumentException
• Exceptio message: Source array was not long enough. Check the source index, length, and the array's lower bounds. (Parameter 'sourceArray')
• Stack trace: (with "Just my code" setting disabled)

   at System.Array.CopyImpl(Array sourceArray, Int32 sourceIndex, Array destinationArray, Int32 destinationIndex, Int32 length, Boolean reliable) in /_/src/coreclr/System.Private.CoreLib/src/System/Array.CoreCLR.cs:line 45
   at System.Collections.Generic.List`1.set_Capacity(Int32 value) in /_/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/List.cs:line 114
   at System.Collections.Generic.List`1.AddWithResize(T item) in /_/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/List.cs:line 218
   at DiskusiTestArrayExceptionDiTaskWhenAll.MiscelanousComputeProcessSimplified.<>c__DisplayClass1_0.<TaskWhenAll_ProcessIterateFor>b__1(Double resultItem) in D:\github-repo-large\belajar-dotnet-bahasa\source\DiskusiTestArrayExceptionDiTaskWhenAll\MiscelanousComputeProcessSimplified.cs:line 35
   at System.Collections.Generic.List`1.ForEach(Action`1 action) in /_/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/List.cs:line 616
   at DiskusiTestArrayExceptionDiTaskWhenAll.MiscelanousComputeProcessSimplified.<>c__DisplayClass1_0.<TaskWhenAll_ProcessIterateFor>b__0() in D:\github-repo-large\belajar-dotnet-bahasa\source\DiskusiTestArrayExceptionDiTaskWhenAll\MiscelanousComputeProcessSimplified.cs:line 35
   at System.Threading.ExecutionContext.RunFromThreadPoolDispatchLoop(Thread threadPoolThread, ExecutionContext executionContext, ContextCallback callback, Object state) in /_/src/libraries/System.Private.CoreLib/src/System/Threading/ExecutionContext.cs:line 264
--- End of stack trace from previous location ---

Screenshots:

Sample console output:

When exception caught:

and this is the debugged source code of that:

Regression?

Not that I can tell whether this is regression or not, but I can reproduce this on .NET Framework 4.8, .NET 8.0 and .NET 9.0, although the stack trace is for .NET 8.0 and .NET 9.0

Known Workarounds

If I use ConcurrentBag within the code that add the item, the problem seems almost gone, although the perf is somehow slower.
But is this the only way? Just for curiosity, I have tried to use lock, CMIIW. But using lock still doesn't solve the problem and the perf is worse than without lock.

Any correction/feedback appreciated, thanks in advance!

Configuration

.NET SDK I use:

1. .NET SDK 9.0.300, Runtime: 9.0.5
2. .NET SDK 8.0.410, Runtime 8.0.16
   Visual Studio used: VS 2022 17.14.4

Other information

If I use Parallel.ForAsync, the problem is gone. But I may have less scenario to test, as I only have 50 times of loop.

[eriawan]

FYI, if I run the code on top of .NET Framework 4.8, the problems seems worse: (almost every iteration throws exception)

[ovska]

List<T> is not thread-safe to modify from multiple threads. If you want to process the items in batches in parallel, you should do so for each batch, and only then combine the results afterwards into a list.

[eriawan]

List<T> is not thread-safe to modify from multiple threads. If you want to process the items in batches in parallel, you should do so for each batch, and only then combine the results afterwards into a list.

Yes, you are right. What I'm trying to understand is these two things:

1. If it is not thread safe, the official doc doesn't mention it explicitly, not like ConcurrentBag's documentation that says it is thread safe explicitly.
2. Since it is not every iteration throws exception, is there possible bug when comparing the uint with nuint as in the line that throws exception after comparing it.

[eriawan]

@dotnet/area-system-collections area owner, feel free to comment as well.

I would be happy to close this bug if this is not a bug. If it is possible, I would like to have simple explanation or comment if this is not a bug or by design. If it is by design, I would like to have the doc of List to be updated that it should clearly and explicitly state that it is not thread safe.

Apologize if my issue submission have some grammar mistakes. :)

[PranavSenthilnathan]

It's mentioned here: https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.list-1?view=net-9.0#thread-safety

[stephentoub]

It's mentioned here: https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.list-1?view=net-9.0#thread-safety

In general no instance members in .NET libraries are thread-safe unless otherwise documented, which is why it's documented for ConcurrentBag.

[eriawan]

It's mentioned here: https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.list-1?view=net-9.0#thread-safety

In general no instance members in .NET libraries are thread-safe unless otherwise documented, which is why it's documented for ConcurrentBag.

@stephentoub
Thank you for the reply! So do you think this issue is not a bug and it is by design?

[stephentoub]

So do you think this issue is not a bug and it is by design?

Behavior when using non-thread-safe members concurrently from multiple threads is undefined.

[eriawan]

@stephentoub many thanks!

Just a minor suggestion: in the official docs of LIst it suggests that we could use lock. I've tried that on my scenario and I still get the intermittent exception. I think I'm going to open an issue on the docs site.

I think (and I hope) this issue is not fully useless and it could bring some clarity to others as well 🙂