Multiple thread for same method

Question

I am trying to get 100000 string output and trying to achieve with multiple threads but when checking final result string, it only has 10000 line.

Here =>

  string result = "";

   private void Testing()
            {  
 var threadA = new Thread(() => { result += A()+Environment.NewLine; });
                var threadB = new Thread(() => { result += A() + Environment.NewLine; });
                var threadC = new Thread(() => { result += A() + Environment.NewLine; });
                var threadD = new Thread(() => { result += A() + Environment.NewLine; });
                var threadE = new Thread(() => { result += A() + Environment.NewLine; });
                var threadF = new Thread(() => { result += A() + Environment.NewLine; });
                var threadG = new Thread(() => { result += A() + Environment.NewLine; });
                var threadH = new Thread(() => { result += A() + Environment.NewLine; });
                var threadI = new Thread(() => { result += A() + Environment.NewLine; });
                var threadJ = new Thread(() => { result += A()+Environment.NewLine; });

                threadA.Start();
                threadB.Start();
                threadC.Start();
                threadD.Start();
                threadE.Start();
                threadF.Start();
                threadG.Start();
                threadH.Start();
                threadI.Start();
                threadJ.Start();

                threadA.Join();
                threadB.Join();
                threadC.Join();
                threadD.Join();
                threadE.Join();
                threadF.Join();
                threadG.Join();
                threadH.Join();
                threadI.Join();
                threadJ.Join();
    }

private string A()
        {
            for (int i = 0; i <= 10000; i++)
            {
                result += "select * from testing" + Environment.NewLine;
            }
            return result;
        }

But i dont get 100000,I just get 10000.Please let me known why?

A is not thread-safe. Multiple threads try to change `result` at the same time. One wins. You should build a string within a local variable in A, synchronise the context (by using `lock`, for example), and append the string to the big string. — ProgrammingLlama, Apr 23 '18 at 08:35
@BugFinder, I though 10 thread with 10k looping will get 100k line. — John, Apr 23 '18 at 08:37
it may even work sometimes, that is an unpredicatable behaviour, you need to lock your string or to use some concurrent collections and then aggregate them — Boo, Apr 23 '18 at 08:38
Stop using Thread anyway and get with the current era, use tasks, or Parralel.For or something, make sure you also look up how to make code thread safe — TheGeneral, Apr 23 '18 at 08:38
@john, Even though thread can change result value, I have no initialized result within A.So I though it will automatically append to result. — John, Apr 23 '18 at 08:40
Alternative John, you are trying to write to `result` from many places at once without concern for other people doing the same thing. If you visualise `result += value;` as `result = result + value;` then you can see that in the time between getting result, appending value, and writing it back, another thread could have read the old value of `result`. This is where your problem comes from. — ProgrammingLlama, Apr 23 '18 at 08:45

score 2 · Answer 1 · answered Apr 23 '18 at 09:07

Another way you can do this is to forget about creating a Thread it has a lot of overhead and there are many better solutions. Why not just use a Parallel.For. It uses the threadpool you can set how much parallelism you like.

Also if you you are dealing with threads you need to know how to write thread safe code, there are many sorts of locking mechanisms, or there are structures built with thread safety in mind, Thread-Safe Collections . If ordering doesn't matter you could easily use ConcurrentBag<T>

Which can shorten your code to

static ConcurrentBag<string> results  = new  ConcurrentBag<string>();

...

private static void myTest(int count )
{
   for (var i = 0; i < 1000; i++)
   {
      results.Add("select * from testing " + i * count);
   }
}

Usage

Parallel.For(0, 10, myTest);
var result = string.Join(Environment.NewLine, results);

Anyway, this wasn't intended to be a panacea to your problems or the worlds best line writing threaded masterpiece, its just to show you there is lots of resources for threading and many ways to do what you want.

Actually, want to +1 for Parallel.For, Anyway, Thank you. – John Apr 23 '18 at 09:18 — John, Apr 23 '18 at 09:18

ProgrammingLlama · Accepted Answer · 2018-04-23T09:06:15.370

As I've explained in the comments, A() is not thread-safe.

If you visualise result += value; as result = result+ value;, you can see that between a single thread getting the result, and writing it back, it's possible for another thread to get the (now) old value.

You should build each thread's contribution in a local variable (I've changed this to StringBuilder since it's more efficient than string concatenation) and then synchronise the context, and update the result object:

private readonly object _resultLock = new object();    
private void A()
{
    var lines = new StringBuilder();
    for (int i = 0; i <= 10000; i++)
    {
        lines.AppendLine("select * from testing");
    }
    lock (_resultLock)
    {
        result += lines.ToString();
    }
}

Since you already have a variable called "result" in the class scope, I've changed A() to a void.

It's best to lock as little as possible, since threads will have to wait to acquire the lock. We use _resultLock so that we know that the lock is for. You can read more about lock in the docs and on this question.

You might also want to look into tasks: docs, Task vs Thread question.

@FakeCaleb Thanks :) I meant to change the method to a void, but forgot. — ProgrammingLlama, Apr 23 '18 at 09:06

Multiple thread for same method

2 Answers2