Parallel.ForEach loop is not working "it skips some and double do others"-CodePudding

I have 2 methods that can do the work for me, one is serial and the other one is parallel. The reason for parallelization is because there are lots of iteration(about 100,000 or so) For some reason, the parallel one do skip or double doing some iterations, and I don't have any clue how to debug it.

The serial method

for(int i = somenum; i >= 0; i-- ){

    foreach (var nue in nuelist)
    {
        foreach (var path in nue.pathlist)
        {
            foreach (var conn in nue.connlist)
            {
                Func(conn,path); 
            }
        }
    }
}

The parallel method

for(int i = somenum; i >= 0; i-- ){

    Parallel.ForEach(nuelist,nue =>
    {
        Parallel.ForEach(nue.pathlist,path=>
        {
            Parallel.ForEach(nue.connlist, conn=>
            {
                Func(conn,path);
            });
        });
    });
}

Inside Path class

Nue firstnue;
public void Func(Conn conn,Path path)
{
    List<Conn> list = new(){conn};
    list.AddRange(path.list);
    _ = new Path(list); 
}
public Path(List<Conn>)
{
   //other things
   firstnue.pathlist.Add(this);
   /*
   firstnue is another nue that will be 
   in the next iteration of for loop
   */
}

They are both the same method except, of course, foreach and Parallel.ForEach loop.

the code is for the code in here (GitHub page)

CodePudding user response：

List<T>, which I assume you use with firstnue.pathlist, isn't thread-safe. That means, when you add/remove items from the same List<T> from multiple threads at the same time, your data will get corrupt. In order to avoid that problem, the simplest solution is to use a lock, so multiple threads doesn't try to modify list at once.

However, a lock essentially serializes the list operations, and if the only thing you do in Func is to change a list, you may not gain much by parallelizing the code. But, if you still want to give it a try, you just need to change this:

firstnue.pathlist.Add(this);

to this:

lock (firstnue.pathlist)
{
  firstnue.pathlist.Add(this);
}

CodePudding user response：

Thanks to sedat-kapanoglu, I found the problem is really about thread safety. The solution was to change every List<T> to ConcurrentBag<T>.

For everyone, like me, The solution of "parallel not working with collections" is to change from System.Collections.Generic to System.Collections.Concurrent