Thread Safety of StringBuilder in Parallel.For

Question

I have created a struct, say AStruct, and override its ToString() method. Then I have written a parallel for to return some AStruct and put them in a list so I can use a StreamWriter to output them, like

StreamWriter sw = new StreamWriter(@"ABC.txt");
StringBuilder sb = new StringBuilder();
List<AStruct> AList = new List<AStruct>();

Parallel.For(0,10,i =>                          // generate 10 AStruct(s)
{
   AList.Add(DoSomethingThatReturnsAStruct);
});

for(int i =0; i< AList.Count();i++)             //put in a StringBuilder
{
   sb.AppendLine(AList[i].ToString());
}
sw.Write(sb.ToString());
sw.Close();

The problem is the output file only print 7/8 lines of AList, while AList actually got all the 10 elements. I wonder does this relate to thread-safety of StringBuilder. Could someone explain why not all lines are output?

It's not Stringbuilder that's the issue here, it's `List` that's being manipulated in parallel. — Ian Mercer, Jan 24 '16 at 16:59
@IanMercer no its not, read his question especially the last paragraph again. _while AList actually got all the 10 elements._ — CSharpie, Jan 24 '16 at 17:00
@kchpchan 1,10 in the Parallel.For generates 9 elements, not 10, either do 0,10 or 1,11 — CSharpie, Jan 24 '16 at 17:02
@CSharpie - probably while stepping through debugging. List is not thread safe for `Add`. Fix that first. — Ian Mercer, Jan 24 '16 at 17:06
Is that code surrounded by a try catch? Either catching a generic exception or an `ArgumentOutOfRange` exception? — Balah, Jan 24 '16 at 18:01

score 3 · Accepted Answer · 2016-01-24T17:53:53.460

In your code above the instance of StringBuilder is never accessed/modified by anything other than the main thread (or whatever thread is creating the sw) so thread-safety of StringBuilder is not relevant however you have a much bigger bug one that you should always remember when dealing with more than one thread.

Never ever modify a shared resource by multiple threads unless that resource is thread-safe

You are updating the AList from different threads so either use a lock to synchronize access or a thread-safe collection e.g. ConcurrentQueue (order guaranteed) or ConcurrentBag (order not guaranteed)

You are also adding 9 entries to the AList instead of 10.

Finally here's your modified code which yields the expected result.

var sw = new StreamWriter(@"ABC.txt");
try
{        
    var AList = new List<AStruct>();

    var locker = new object();
    Parallel.For(0, 10, i =>                          // generate 10 AStruct(s)
    {
        lock (locker) { AList.Add(new AStruct()); }
    });

    var sb = new StringBuilder();
    for (int i = 0; i < AList.Count; i++)             //put in a StringBuilder
    {
        sb.AppendLine(AList[i].ToString());
    }
    sw.Write(sb.ToString());
} finally
{
    sw.Close();
}

In here using `lock` would make the `Parallel.For` meaningless. Concurrent Collections actually works for me. Thanks! — kchpchan, Jan 24 '16 at 18:53

Thread Safety of StringBuilder in Parallel.For

1 Answers1