Strange Multi-Threading Index out of bound Issue

Question

No matter what I use: Threading Class or TPL task based pattern. There is always an Index out of bound on the data. From further research, I found the value of counter i can be 4, which should not be even possible. What I have Missed? I'm expecting your expert opinions!

Tested with Visual Studio 15.8(2017) 16.1(2019), project targeting .NET framework 4.72.

using System;
using System.Collections.Generic;
using System.Threading;
using System.Threading.Tasks;


namespace ConsoleApp1
{
    class Program
    {
        static void Main(string[] args)
        {
            // a multi-threading search demo, omit much code for simple and clear
            // generate 0-99, total 100 elements with ascending order
            List<int> testData = new List<int>();
            for (int i = 0; i < 100; i++)
            {
                testData.Add(i);
            }
            List<int> searchFor = new List<int>() {
                    67, 0, 99,
                    23, 24, 25,
                    -1, 106
                };

            const int threadsCount = 4;

            // Test switch
            bool useThreadInsteadOfTaskTPL = true;

            if (useThreadInsteadOfTaskTPL)
            {
                // search every piece of data
                for (int j = 0; j < searchFor.Count; j++)
                {
                    Thread[] threads = new Thread[threadsCount];
                    Console.WriteLine("Search for: {0}", searchFor[j]);
                    // trying to divide the data into 4 parts, and search in parallel
                    for (int i = 0; i < threadsCount; i++)
                    {
                        Thread thread = new Thread(() => {
                            // Capture the counters to make sure no lambda pitfall
                            int counterI = i;
                            int counterJ = j;
                            Console.WriteLine("i value: {0}", counterI);
                            Console.WriteLine("j value: {0}", counterJ);
                            // your code

                        });
                        threads[i] = thread;
                        threads[i].Start();
                    }
                    for (int i = 0; i < threads.Length; i++)
                    {
                        threads[i].Join();
                    }
                    Console.WriteLine();
                }
            }
            else
            {
                for (int j = 0; j < searchFor.Count; j++)
                {
                    Task[] tasks = new Task[threadsCount];
                    Console.WriteLine("Search for: {0}", searchFor[j]);
                    // trying to divide the data into 4 parts, and search in parallel
                    for (int i = 0; i < threadsCount; i++)
                    {
                        Task task = Task.Factory.StartNew(() => {
                            // Capture the counters to make sure no lambda pitfall
                            int counterI = i;
                            int counterJ = j;
                            Console.WriteLine("i value: {0}", counterI);
                            Console.WriteLine("j value: {0}", counterJ);
                            // your code

                        }, new CancellationTokenSource().Token,
                            TaskCreationOptions.None, TaskScheduler.Default);
                        tasks[i] = task;
                    }
                    Task.WaitAll(tasks);
                    Console.WriteLine();
                }
            }
            Console.ReadKey();
        }
    }
}

The expected value of i should go through 0...3, but the actual value of i may equals to 4 or keep unchanged between iterates.

Answer 1

You should reassign i and j on loop start (not inside lambda):

for (int i = 0; i < threadsCount; i++)
{
    // Capture the counters to make sure no lambda pitfall
    int counterI = i;
    int counterJ = j;

    Thread thread = new Thread(() =>
    {                                
        Console.WriteLine("i value: {0}", counterI);
        Console.WriteLine("j value: {0}", counterJ);

        // your code                    
    }
}

Your thread is scheduled for execution (it is not started immediately after Start() is called) and when it starts running the value of i (and j) can be already changed. (You can take a look at compiler generated code for this case and for yours).

And same for tasks - they are scheduled, not started immediately.

More details:

See this example (Action delegate is used instead of Thread) and generated code.

You can see difference (generated code creates instance of class which stores value to print and a method which actually prints):

• reassign inside delegate - for every iteration the same instance is used and value is incremented after calling the delegate. With Action it works as expected, because it executes immediately (calling method from generated class to print value), then value of generated class is incremented and new iteration is started.
• reassign outside delegate - instance of generated class is created for every iteration, so there is no increment. Every iteration has independent instance and next iteration can't change the value for previous one.

In case of threads the only difference is that thread is not started immediately, it is scheduled for execution and this takes some time. For first case - when method for printing value is called, the value can be already incremented (because of same instance for all iterations) and you get unexpected result.

You can check this by running application multiple times (for first case) - you will get not identical results when printing i variable - sometimes it is incremented when it is not expected (because it took some time from calling Start() and actual starting of thread execution after scheduling), sometimes values are correct (because thread was scheduled and started almost immediately after calling Start() before increment occurs).