Why I am getting wrong value in threading

Question

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 29-Nov-20 6:47am

Mohamed Hamed

Updated 29-Nov-20 22:00pm

Add a Solution

3 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

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

OriginalGriff · Answer 1 · 2020-11-29T08:59:00

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!

RickZeeland · Answer 2 · 2020-11-29T19:26:00

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#[^]

George Swan · Answer 3 · 2020-11-29T22:00:00

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();
 }

Why I am getting wrong value in threading

3 solutions

Solution 1

Solution 2

Solution 3

Add your solution here

Preview 0