1

i have got this code in my project. it does some cpu processing so in order to speedup stuff am trying to use Parallel.ForEach.

For some reason parallel execution adds a null item which results in "Object reference not set to an instance of an object" exception at later stage of the program.

** It shouldnt been adding null

Working code

foreach (DataRow datarow in dataSet.Tables[0].Rows)
{    
   var item = new T();

   for (int i = 0; i < datarow.Table.Columns.Count; i++)
   {
      var columnName = datarow.Table.Columns[i].ColumnName;
      var columnValue = datarow[i];
      // set new object values
      // use reflection logic to grab values
   }

   finalList.add(item);
}

Error Code

List<DataRow> list = dataSet.Tables[0].AsEnumerable().ToList();

Parallel.ForEach<DataRow>(list, datarow =>
{    
   var item = new T();

   for (int i = 0; i < datarow.Table.Columns.Count; i++)
   {
      var columnName = datarow.Table.Columns[i].ColumnName;
      var columnValue = datarow[i];
      // set new object values
      // use reflection logic to grab values
   }

   finalList.add(item);
}
Adam
  • 1,018
  • 1
  • 9
  • 20
  • What is the type of `finalList`? If it's not one that can be modified concurrently by multiple threads you could see unexpected behaviour... – T2PS Sep 10 '18 at 07:20
  • var list = new List(); it feels like stuff are getting mixed up – Adam Sep 10 '18 at 07:21
  • Unless `finalList` is thread safe, you will need to wrap it with a synchronisation lock. – JayV Sep 10 '18 at 07:21
  • so add() is not thread safe? – Adam Sep 10 '18 at 07:23
  • we don't know, you did not yet tell what `finalList` is, so we can't tell if it's thread safe. if it's `List`, it's not thread safe, which may lead to unexpected behaviour. Where is the exception thrown? – René Vogt Sep 10 '18 at 07:24
  • yeah sorry finalList is a normal list. List(). this is the error. – Adam Sep 10 '18 at 07:30
  • 1
    i will use a ConcurrentBag instead – Adam Sep 10 '18 at 07:30
  • In the code that works you iterate over `dataSet.Tables[0].Rows`, however in your non-working code you iterate over `dataSet.Tables[0].AsEnumerable().ToList()`. You could be iterating over too much because there is no `.Rows` in the second example. – TheHvidsten Sep 10 '18 at 09:04

1 Answers1

0

Just to answer this, this code is using a non-threadsafe collection, List<>, , for FinalList, which is inaccessible by the parallel task code. They should use a threadsafe collection, like ConcurrentBag as the collection type instead.

A list of threadsafe collections can be found here.

dashnick
  • 2,020
  • 19
  • 34
  • 1
    The `ConcurrentBag` appears frequently in the examples in Microsoft's documentation, presumably because it has a method with the familiar name `Add`, but [it's not the best](https://stackoverflow.com/a/64823123/11178549 "When to use BlockingCollection and when ConcurrentBag instead of List?") collection for the job. A better alternative is the `ConcurrentQueue`, because it preserves the order of the enqueued items. – Theodor Zoulias Jul 06 '22 at 04:34