Parallel.ForEach with adding to list

Question

I'm trying to run multiple functions that connect to a remote site (by network) and return a generic list. But I want to run them simultaneously.

For example:

public static List<SearchResult> Search(string title)
{
    //Initialize a new temp list to hold all search results
    List<SearchResult> results = new List<SearchResult>();

    //Loop all providers simultaneously
    Parallel.ForEach(Providers, currentProvider =>
    {
        List<SearchResult> tmpResults = currentProvider.SearchTitle((title));

        //Add results from current provider
        results.AddRange(tmpResults);
    });

    //Return all combined results
    return results;
}

As I see it, multiple insertions to 'results' may happend at the same time... Which may crash my application.

How can I avoid this?

Solution 1

//In the class scope:
Object lockMe = new Object();    

//In the function
lock (lockMe)
{    
     results.AddRange(tmpResults);
}

Basically a lock means that only one thread can have access to that critical section at the same time.

Solution 2

You can use a concurrent collection.

The System.Collections.Concurrent namespace provides several thread-safe collection classes that should be used in place of the corresponding types in the System.Collections and System.Collections.Generic namespaces whenever multiple threads are accessing the collection concurrently.

You could for example use ConcurrentBag since you have no guarantee which order the items will be added.

Represents a thread-safe, unordered collection of objects.

Solution 3

For those who prefer code:

public static ConcurrentBag<SearchResult> Search(string title)
{
    var results = new ConcurrentBag<SearchResult>();
    Parallel.ForEach(Providers, currentProvider =>
    {
        results.Add(currentProvider.SearchTitle((title)));
    });

    return results;
}

Solution 4

The Concurrent Collections are new for .Net 4; they are designed to work with the new parallel functionality.

See Concurrent Collections in the .NET Framework 4:

Before .NET 4, you had to provide your own synchronization mechanisms if multiple threads might be accessing a single shared collection. You had to lock the collection ...

... the [new] classes and interfaces in System.Collections.Concurrent [added in .NET 4] provide a consistent implementation for [...] multi-threaded programming problems involving shared data across threads.

Solution 5

This could be expressed concisely using PLINQ's AsParallel and SelectMany:

public static List<SearchResult> Search(string title)
{
    return Providers.AsParallel()
                    .SelectMany(p => p.SearchTitle(title))
                    .ToList();
}

View more solutions

Comments

[shaharmor]

But what will happend if WHILE those results are being added the results from another provider try to add to? will they FAIL or WAIT until possible?

[Haedrian]

When there's a lock, the thread will wait until it can get the lock.

[shaharmor]

So basically its like saying: Wait untill !results.isLocked, and when its free lock it and write?

[Haedrian]

Locks work on pieces of code not on the variables. You can put much more code there. Its important to realise that. The lock won't protect the variable outside of the lock. Basically the thread will wait at the lock, if the lock is locked, it will wait. If its not, it'll lock it for itself, and enter that section. Then when its done it releases the lock and other threads can enter. If you need more flexibility there are other kinds of locks but this will work for you.

[Henk Holterman]

A minor point: this is not the safest choice for the lock object. Better to use a special private object: lock(resultsLock) .

[Piotr Kula]

locks can slow down the overall execution time though.. concurrent collections seem better to avoid that

[lkg]

Yes, this is the actual answer. You'll get better performance (generally) with concurrent collections.

[Frank Q.]

Why do we need a lock at class level if the object modified is declared locally ? If we lock(results) and then add to it what is wrong ? Lock at class level means, if multiple requests are calling Search then you have degraded the performance

[Kugan Kumar]

linq selectMany is great, sadly linq is slower than normal foreach. :(

[Douglas]

Don't micro-optimize. The OP implied that SearchTitle connects to a remote site. Its latency will be several orders of magnitude slower than the difference between LINQ and foreach.

[Anthony McGrath]

Have to use a loop: foreach (var item in currentProvider.SearchTitle((title))) results.Add(item);

[Theodor Zoulias]

A ConcurrentQueue<T> might be preferable to a ConcurrentBag<T>, because the former preserves the insertion order. The ConcurrentBag<T> is an extremely specialized collection intended for mixed producer consumer scenarios, and adding items in parallel (without taking items from the collection) is not such a scenario.