Click here to Skip to main content
15,569,840 members
Please Sign up or sign in to vote.
1.00/5 (1 vote)
See more:
Why i am getting wrong value of
_counter 


What I have tried:

CODE
using System;
using System.Threading;

namespace ConsoleApp1
{
	class Program
	{
		private static int _counter = 0;
		private static void DoCount()
		{
			for (int i = 0; i < 100000; i++)
			{
				_counter++;
			}
		}

		static void Main(string[] args)
		{
			int threadsCount = 100;
			Thread[] threads = new Thread[threadsCount];

			//Allocate threads
			for (int i = 0; i < threadsCount; i++)
				threads[i] = new Thread(DoCount);

			//Start threads
			for (int i = 0; i < threadsCount; i++)
				threads[i].Start();

			//Wait for threads to finish
			for (int i = 0; i < threadsCount; i++)
				threads[i].Join();

			//Print counter
			Console.WriteLine("Counter is: {0}", _counter);
			Console.ReadLine();
		}
	}
}
Posted
Updated 29-Nov-20 23:00pm

Because count++ is not an atomic operation, it is syntactic sugar for:
C#
counter = counter + 1;
And because you have multiple threads accessing the same memory location at the same time, sometimes one thread will get in while another has loaded the value from memory, but not incremented it yet, while a third had incremented the value but not stored it yet, and a fourth has just stored it and is about to load it again. With the silly number of thread you are creating and limited number of cores to run them on, it's pretty much guaranteed that tis is going to happen a lot, and that means that two runs are very unlikely to produce the same result.

Basically, your code is described as "not thread safe" and unpredicable results are a feature of a lack of thread safety.

If you want reliable results, you would have to institute either locking or a mutex, and that'll effectively slow your app down to below the point at which a single thread would have done the same thing!

And remember: creating more threads than you have available cores is likely t slow your processing down, rather than speed it up: threading is not a "magic bullet" for performance improvement, it's something which has to be thought about very carefully if it's not going to both complicate your code and slow it down!
 
Share this answer
 
Griff is correct about locking, here is an example:
private static int _counter = 0;
private static object threadLock = new object();

private static void DoCount()
{
    lock (threadLock)
    {
        for (int i = 0; i < 100000; i++)
        {
            _counter++;
        }
    }
}

To speed things up:
private static void DoCount()
{
    int tempCounter = 0;

    for (int i = 0; i < 100000; i++)
    {
        tempCounter++;
    }

    lock (threadLock)
    {
        _counter += tempCounter;
    }
}


Note: instead of using Thread it is now advised to use: Task Class (System.Threading.Tasks) | Microsoft Docs[^]
Also see CodeProject article: The Basics of Task Parallelism via C#[^]
 
Share this answer
 
v4

As other solutions have stated, your problem arises because the methods you employ are not thread safe and the problem can be resolved by selectively locking the threads. But locking works by blocking the threads so that you have threads queuing up to do their work. This is inefficient and results in threads being employed for longer than is necessary. My suggestion would be to restructure your methods to allow all worker threads to run to completion without any blocking. Something like this.


C#
private static int DoCount(int max)
 {
     int counter = 0;
     for (int i = 0; i < max; i++)
     {
         ++counter;
     }
     return counter;
 }

private static async Task Main(string[] args)
 {
     int workerCount = 100;
     int unitsOfWork = 10000;
     List<Task<int>> tasks = new List<Task<int>>();
     for (int i = 0; i < workerCount; i++)
     {
         //start all the worker tasks
         tasks.Add(Task.Run(() => DoCount(unitsOfWork)));
     }
     //asynchronously wait for all tasks to complete
     //and collect the results
     var results = await Task.WhenAll(tasks);
     //sum all the results
     int totalCount = results.Sum();
     Console.WriteLine(totalCount);
     Console.ReadLine();
 }
 
Share this answer
 

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)



CodeProject, 20 Bay Street, 11th Floor Toronto, Ontario, Canada M5J 2N8 +1 (416) 849-8900