CODEWITHSUNDEEP
c#multithreadingthread-safety
sm_
sm_

Reputation: 2602

Null Reference while handling a List in multiple threads

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.

Upvotes: 2

Views: 2210

Answers (3)

IvoTops
IvoTops

Reputation: 3531

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............

Upvotes: 2

Tudor
Tudor

Reputation: 62439

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);
}

Upvotes: 1

KittenLS
KittenLS

Reputation: 81

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;

Upvotes: 0

Related Questions