passing lists to threads

Question

I have a method that accepts a List<int> called DoWork. I have a huge List<int> Ids. I split the huge list into 4 sub lists:

List<List<int>> t = (List<List<int>>)SplitColumn<int>(Ids);

(SplitColumn is slightly modified from the answer to splitting a list into sub lists).

I paused the program and inspected t with the debugger and it is four lists divided exactly as I would expect.

Then, what I'm trying to do is spawn four threads (one for each sublist). The part I'm having trouble with is passing the four lists. I am getting out of bounds problems, I'm not sure what's going on here:

        List<Thread> threads = new List<Thread>();

        for(int i = 0; i < t.Count; i++) 
        {
            threads.Add(new Thread(() => DoWork(t[i])));
        }

        foreach (Thread thread in threads)
        {
            thread.Start();
        }


        foreach (Thread thread in threads)
        {
            thread.Join();
        }

You'd have better luck passing your threads the whole list and a starting index and length of the block they're supposed to process. Making sublists is much more time and space intensive. — Wug, Oct 12 '12 at 20:22
Since this is tagged as .NET 4, I wonder why are you not using TPL for this? — Brian Rasmussen, Oct 12 '12 at 20:23
Have you considering using the Task Parallel Library's Parallel.ForEach() method? this will handle everything but the splitting automatically for you (if you want 4 chunks, you should chunk it prior to the parallel loop) — Chris, Oct 12 '12 at 20:23
I'll look into TPL, thanks. This was just mostly an experiment to see if threading would even speed things up enough to justify it. — user17753, Oct 12 '12 at 20:27
Parallel.For will even do the splitting. Not exactly in 4 parts maybe but adapted to circumstance. Probably better. — H H, Oct 12 '12 at 20:28

score 8 · Accepted Answer · answered Oct 12 '12 at 20:23

8

This is a classic, called capturing the loop variable.

In this code, the same variable i is shared by all threads. By the time the threads are running the main thread will have made i == t.Count, hence the range exception.

    for(int i = 0; i < t.Count; i++) 
    {
        threads.Add(new Thread(() => DoWork(t[i])));
    }

To fix it:

    for(int i = 0; i < t.Count; i++) 
    {
        int copy = i;
        threads.Add(new Thread(() => DoWork(t[copy])));
    }

answered Oct 12 '12 at 20:23

H H

263,252
30
330
514

OK cool I get it now, similar (but not really) to closure issues in functional languages. The answer was faster than I can accept it... so I will when I'm able to, ha! – user17753 Oct 12 '12 at 20:30
Another just as easy solution is to start the thread in the first loop, that way the closure doesn't escape the loop and won't cause any problems. – Servy Oct 12 '12 at 20:46

score 3 · Answer 2 · answered Oct 12 '12 at 20:23

Captured variables is not your friend in this case. Try:

        for(int i = 0; i < t.Count; i++) 
        {
            int j=i;
            threads.Add(new Thread(() => DoWork(t[j])));
        }

What's happening is that when your original runs to completion, i==t.Count. And when DoWork executes, you are actually doing t[t.Count]. Not good!

score 0 · Answer 3 · answered Oct 12 '12 at 20:24

You can pass in an object when you start the thread. So create an overload of DoWork that take in a plain old object. Then do the modification below.

    for(int i = 0; i < t.Count; i++) 
    {
        threads.Add(new Thread(DoWork));
    }

    foreach (Thread thread in threads)
    {
        thread.Start([your list right there]);
    }

score 0 · Answer 4 · answered Oct 12 '12 at 20:41

0

As Henk Holterman, points out, you are closing over the loop variable. What I would do instead is this:

List<Thread> threads = t
    .Select(x => new Thread(() => DoWork(x)))
    .ToList();

See also Closing over the loop variable considered harmful.

answered Oct 12 '12 at 20:41

Thom Smith

13,916
6
45
91

score 0 · Answer 5 · answered Oct 12 '12 at 20:42

You can send complex type to thread

public string FileSource;
public string FileName;
public ThreadPriority Priority;

public ThreadInfo(string File, ThreadPriority priority)
{
   FileSource = File;
   FileName = Path.GetFileNameWithoutExtension(File);
   Priority = priority;
}

static void Main()
{
   ThreadInfo ti = null;
   try
   {
      ti = new ThreadInfo(FileName, ThreadPriority.Normal);
      ThreadPool.QueueUserWorkItem(ThreadImageProcess.doWork, ti);
   }
   catch (Exception error)
   {
      MyEventLog.WriteEntry("Error creating Thread Pool: "+ error.StackTrace, EventLogEntryType.Error, (int)ImageProcess.MyEventID.ProcessFile, (int)ImageProcess.MyErrorCategory.Information, null);
   }
   finally
   {
      if (ti != null)
          ti = null;
   }
}

public static void doWork(object sender)
{
  string FileName = "";
  try
  {
     Thread.CurrentThread.Priority = (sender as ThreadInfo).Priority;
     FileName = (sender as ThreadInfo).FileSource;
  }
  catch (Exception error)
  {
     //send some message
  }
}

passing lists to threads

5 Answers5