CODEWITHSUNDEEP
c#multithreading
Parsa
Parsa

Reputation: 11624

It seems ConcurrentBag is not Thread-Safe

I had written a program that a list-builder method returns IEnumerable of string including lots of strings (1milion items) and I stored it in List of string , then it append all the items in a StringBuilder instance in a Parallel.Foreach. Then I printed stringBuilderInstance.Length .

The problem was that it was fewer than 1000000 . after some googling, I realized thet List collection is not Thread-Safe , and that causes this problem. So 2 solution crosses my mind :

1) Use lock

2) Use ConcurrentBag

When I am using lock , it is OK and the length is 1 milion , BUT :

when I am using ConcurrentBag of string , the length is fewer than I expected !

What is the underlying reason of this problem?

List-Creator method :

public static List<string> CreateList()
{
    List<string> result = new List<string>();
    for (int i = 0; i < 1000000; i++)
    {
        result.Add(1.ToString());
    }
    return result;
}

Using ConcurrentBag :

public static void DoWithParallel_ThreadSafe()
{
    ConcurrentBag<string> listOfString = new ConcurrentBag<string>(CreateList());
    StringBuilder a = new StringBuilder();
    Action<string> appender = (number) =>
    {
        a.Append(number);
    };
    Parallel.ForEach(listOfString, appender);
    Console.WriteLine($"The string builder lenght : {a.Length}");
}

Using lock :

public static void DoWithParallel_UnsafeThread_Lock()
{
    List<string> listOfString = CreateList();
    StringBuilder a = new StringBuilder();
    Action<string> appender = (number) =>
    {
        lock (listOfString)
        {
            a.Append(number);
        }
    };
    Parallel.ForEach(listOfString, appender);
    Console.WriteLine($"The string builder lenght : {a.Length}");
}

Main :

static void Main(string[] args)
{
    DoWithParallel_UnsafeThread_Lock();
    DoWithParallel_ThreadSafe();
    Console.ReadKey();
}

Thank you in advance.

Upvotes: 1

Views: 2956

Answers (2)

Servy
Servy

Reputation: 203842

StringBuilder cannot be mutated from multiple threads, hence why the code doesn't work when you try to do that. Note that locking is pointless; just don't create multiple threads to do the work in the first place since the work can't be done from multiple threads. Since you're never accessing the ConcurrentBag from multiple threads, so it's pointless to be using that instead of a List.

Upvotes: 7

Aasmund Eldhuset
Aasmund Eldhuset

Reputation: 37950

StringBuilder is not thread-safe, which is why some Append() calls are "lost". So you still need the lock, even if your collection is thread-safe.

(Also, see Servy's answer for why you don't need the collection to be thread-safe at all.)

Upvotes: 5

Related Questions