Unexpected results when multi threading [duplicate]

Question

I have a function that is to be called by a thread initiated with a start and end parameter. The function works fine when run on single, main thread. However, when I try multi threading on it, the code breaks.

The function is as below:

static void processThread(long startLimit, long endLimit)
    {
        long rangeLimit = startLimit;
        while (startLimit < endLimit) {
            rangeLimit = rangeLimit + 100;
            startLimit++;
            Console.WriteLine("Processed for " + startLimit + ", " + rangeLimit); 
            startLimit = rangeLimit;
        }
    }

I am calling it from main as::

int threadCount = 4;
long[] startPoints = new long[threadCount];
long[] endPoints = new long[threadCount];
if ((endLimit / 100) % threadCount == 0)
{
    for (int i = 0; i < threadCount; i++)
    {
        endPoints[i] = endLimit * (i + 1) / threadCount;
    }
    startPoints[0] = 0;
    for (int i = 1; i < threadCount; i++)
    {
        startPoints[i] = endPoints[i - 1];
    }
}
Thread[] threads = new Thread[threadCount];
for (int i = 0; i < threadCount; i++)
{
    threads[i] = new Thread(() => processThread(startPoints[i], endPoints[i]));
    threads[i].Start();
    Console.WriteLine("Started for " + startPoints[i] + ", " + endPoints[i]); 
}

The expected result is something like

Processed for 1, 100
Processed for 101, 200
Processed for 201, 300
Processed for 301, 400
Processed for 401, 500
Processed for 501, 600
Processed for 601, 700
Processed for 701, 800
.....

And so on... But what I am getting is:

Started for 0, 2500
Started for 2500, 5000
Processed for 5001, 5100
Processed for 5101, 5200
Processed for 5201, 5300
Processed for 5301, 5400
Processed for 5401, 5500
Processed for 5501, 5600
Processed for 5601, 5700
Processed for 5001, 5100
Started for 5000, 7500
Processed for 5001, 5100
Processed for 5101, 5200
Processed for 5201, 5300
Processed for 5301, 5400
Processed for 5401, 5500
Processed for 5501, 5600
Processed for 5601, 5700
Processed for 5701, 5800
Processed for 5801, 5900
Processed for 5901, 6000
Processed for 6001, 6100
Processed for 6101, 6200
Started for 7500, 10000
Processed for 6201, 6300
Processed for 6301, 6400
Processed for 6401, 6500
Processed for 6501, 6600
Processed for 5701, 5800

Which has many repeated values and has none from the range 0-2500. I also tried with Task.Factory, and got the same result.

Any help on this would be greatly appreciated.

Answer 1

I ran your code and actually got an index out of bounds exception when starting the threads. This however worked for me:

Thread[] threads = new Thread[threadCount];
for (int i = 0; i < threadCount; i++) {
    var start = startPoints[i];
    var end = endPoints[i];
    threads[i] = new Thread( () => processThread( start, end ) );
    threads[i].Start();
    Console.WriteLine( "Started for " + startPoints[i] + ", " + endPoints[i] );
}

The issue might be that when the thread accesses startPoints[i] in your code, the i already changed in the main thread.

Answer 2

I would not comment on the correct solution for your problem, just to fix your output, you are missing this at the end in order to block main thread before all child ones are finished:

                Console.WriteLine("Started for " + startPoints[i] + ", " + endPoints[i]);
        }
        foreach (var thread in threads)
        {
            thread.Join();
        }

        Console.ReadKey();
    }

One more thing I have fixed in the code:

            int count = i;
            threads[i] = new Thread(() =>
            {
                processThread(startPoints[count], endPoints[count]);
            });

To prevent potential problems where i is invalid when evaluated from child threads.

I would also say that unless you really need to use "old" .net Threads, Parallel library or Tasks should be used for this as suggested by other commenters.

The same just rewritten with Task approach (I have kept it as is just replaced the Thread with Task for better illustration).

var factory = new TaskFactory();
        var tasks = new List<Task>();
        for (int i = 0; i < threadCount; i++)
        {
            int count = i;
            Task task = factory.StartNew(() =>
            {
                processThread(startPoints[count], endPoints[count]);
            });
            tasks.Add(task);
            Console.WriteLine("Started for " + startPoints[i] + ", " + endPoints[i]);
        }

        Task.WaitAll(tasks.ToArray());