Null Reference while handling a List in multiple threads

Question

Basically, i have a collection of objects, i am chopping it into small collections, and doing some work on a thread over each small collection simultaneously.

int totalCount =  SomeDictionary.Values.ToList().Count;
int singleThreadCount = (int)Math.Round((decimal)(totalCount / 10));
int lastThreadCount = totalCount - (singleThreadCount * 9);

Stopwatch sw = new Stopwatch();

Dictionary<int,Thread> allThreads = new Dictionary<int,Thread>();
List<rCode> results = new List<rCode>();

for (int i = 0; i < 10; i++)
{
    int count = i;

    if (i != 9)
    {
        Thread someThread = new Thread(() =>
        {
            List<rBase> objects =  SomeDictionary.Values
                                          .Skip(count * singleThreadCount)
                                          .Take(singleThreadCount).ToList();

            List<rCode> result = objects.Where(r => r.ZBox != null)
            .SelectMany(r => r.EffectiveCBox, (r, CBox) => new rCode
                                {
                                    RBox = r,
                                    // A Zbox may refer an object that can be 
                                    // shared by many 
                                    // rCode objects even on different threads
                                    ZBox = r.ZBox,
                                    CBox = CBox
                                }).ToList();

            results.AddRange(result);
        });

        allThreads.Add(i, someThread);
        someThread.Start();
    }
    else 
    {
        Thread someThread = new Thread(() =>
        {
            List<rBase> objects =  SomeDictionary.Values
                                           .Skip(count * singleThreadCount)
                                           .Take(lastThreadCount).ToList();

            List<rCode> result = objects.Where(r => r.ZBox != null)
            .SelectMany(r => r.EffectiveCBox, (r, CBox) => new rCode
                        {
                            RBox = r,
                            // A Zbox may refer an object that 
                            // can be shared by many 
                            // rCode objects even on different threads
                            ZBox = r.ZBox, 
                            CBox = CBox
                        }).ToList();

            results.AddRange(result);
        });

        allThreads.Add(i, someThread);
        someThread.Start();
    }
}

sw.Start();
while (allThreads.Values.Any(th => th.IsAlive))
{ 
    if (sw.ElapsedMilliseconds >= 60000) 
    { 
        results = null; 
        allThreads.Values.ToList().ForEach(t => t.Abort()); 
        sw.Stop(); 
        break; 
    } 
}

return  results != null ? results.OrderBy(r => r.ZBox.Name).ToList():null;

so, My issue is that SOMETIMES, i get a null reference exception while performing the OrderBy operation before returning the results, and i couldn't determine where is the exception exactly, i press back, click the same button that does this operation on the same data again, and it works !! .. If someone can help me identify this issue i would be more than gratefull. NOTE :A Zbox may refer an object that can be shared by many rCode objects even on different threads, can this be the issue ? as i can't determine this upon testing, because the error happening is not deterministic.

Are the different threads able to alter the value of r.ZBox at any given time? — Tony The Lion
– Tony The Lion, Commented Sep 14, 2012 at 9:44
you are modifying the results List<> from all threads at the same time. This is a bad idea, as List<T> is not thread safe. You will never know what you will end up with when you manipulate your list like this. — Dirk Trilsbeek
– Dirk Trilsbeek, Commented Sep 14, 2012 at 9:44
Seems like you could just replace the whole thing with Parallel.ForEach. — Tudor
– Tudor, Commented Sep 14, 2012 at 9:46
for instance, yes. Whenever you use multiple threads, make sure that all objects accessed by more than a single thread at one time are thread safe - or create a thread-safe access for them using a lock() — Dirk Trilsbeek
– Dirk Trilsbeek, Commented Sep 14, 2012 at 9:49

IvoTops · Accepted Answer · 2012-09-14 14:15:04Z

2

The bug is correctly found in the chosen answer although I do not agree with the answer. You should switch to using a concurrent collection. In your case a ConcurrentBag or ConcurrentQueue. Some of which are (partially) lockfree for better performance. And they provide more readable and less code since you do not need manual locking.

Your code would also more than halve in size and double in readability if you keep from manually created threads and manual paritioning;

Parallel.ForEach(objects, MyObjectProcessor);

public void MyObjectProcessor(Object o)
{
  // Create result and add to results
}

Use a ParallelOptions object if you want to limit the number of threads with Parallel.ForEach............

answered Sep 14, 2012 at 14:15

IvoTops

3,54120 silver badges18 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Tudor · Accepted Answer · 2012-09-14 10:11:30Z

1

Well, one obvious problem is here:

results.AddRange(result);

where you're updating a list from multiple threads. Try using a lock:

object resultsLock = new object(); // globally visible
...
lock(resultsLock) 
{
    results.AddRange(result);
}

answered Sep 14, 2012 at 10:11

Tudor

62.6k13 gold badges105 silver badges147 bronze badges

Comments

KittenLS · Accepted Answer · 2012-09-14 09:45:23Z

0

I suppose the problem in results = null

while (allThreads.Values.Any(th => th.IsAlive))
    { if (sw.ElapsedMilliseconds >= 60000) { results = null;  allThreads.Values.ToList().ForEach(t => t.Abort());

if the threads not finised faster than 60000 ms you results become equal null and you can't call results.OrderBy(r => r.ZBox.Name).ToList(); it's throws exception

you should add something like that

if (results != null) 
  return  results.OrderBy(r => r.ZBox.Name).ToList();
else
  return null;

answered Sep 14, 2012 at 9:45

KittenLS

811 silver badge3 bronze badges

2 Comments

sm_ Over a year ago

Am sorry i should've mentioned that i modified the code due to code safety reasons before i posted it, this condition is already take into consideration. Thanks for the note i'll update this in the question.

sm_ Over a year ago

@TonyTheLion i'll look around for the WaitAll method ;)

Collectives™ on Stack Overflow

Null Reference while handling a List in multiple threads

3 Answers 3

Comments

Comments

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related