C# 2.0 Threading Question (anonymous methods)

Question

I have a simple application with the following code:

   FileInfo[] files = (new DirectoryInfo(initialDirectory)).GetFiles();
   List<Thread> threads = new List<Thread>(files.Length);

   foreach (FileInfo f in files)
   {
       Thread t = new Thread(delegate()
       {
            Console.WriteLine(f.FullName);
       });
       threads.Add(t);
   }

   foreach (Thread t in threads)
       t.Start();

Lets say in 'I=initialDirectory' directory I have 3 files. This application should then create 3 threads, with each thread printing off one of the file names; however, instead each thread will print off the name of the last file in the 'files' array.

Why is this? Why is the current file 'f' variable not getting setup in the anonymous method correctly?

Answer 1

The anonymous method keeps a reference to the variable in the enclosing block -- not the actual value of the variable.

By the time the methods are actually executed (when you start the threads) f has been assigned to point to the last value in the collection, so all 3 threads print that last value.

Answer 2

It's because f.FullName is a reference to a variable, and not a value (which is how you tried to use it). By the time you actually start the threads f.FullName was incremented all the way to the end of the array.

Anyway, why iterate through things here twice?

foreach (FileInfo f in files)
{
   Thread t = new Thread(delegate()
   {
        Console.WriteLine(f.FullName);
   });
   threads.Add(t);
   t.Start();
}

However, this is still wrong, and perhaps even worse since you now have a race condition to see which thread goes faster: writing the console item or iterating to the next FileInfo.

Answer 3

I don't have the rep to comment, so I'll just reply:

Thanks for the help; for some reason in my mind it would magically create a new method for each loop iteration with the reference to the current FileInfo, which obviously wasn't the case... I guess it makes sense that they were all referencing the same variable 'f', and not what 'f' was referencing.

I fixed the issue by moving the worker code to a new method, and using the thread with ParameterizedThreadStart.

Thanks!

*edit: Oops, didn't realize you could comment on your own questions

Answer 4

Here are some nice articles about anonymous methods in C# and the code that will be generated by compiler:

http://blogs.msdn.com/oldnewthing/archive/2006/08/02/686456.aspx
http://blogs.msdn.com/oldnewthing/archive/2006/08/03/687529.aspx
http://blogs.msdn.com/oldnewthing/archive/2006/08/04/688527.aspx

I think if you did:

   foreach (FileInfo f in files)
   {
       FileInfo f2 = f; //variable declared inside the loop
       Thread t = new Thread(delegate()
       {
            Console.WriteLine(f2.FullName);
       });
       threads.Add(t);
   }

it would would work the way you wanted it to.

Answer 5

This is an example of a "closure" in C#, right?

Answer 6

It's because the underlying code for iterator (foreach) has already 'iterated' through all the values in the List before the threads start... So when they start, the value 'pointed' at by the iterator is the last one in the list...

Start the thread inside the iteration instead....

foreach (FileInfo f in files)
 {   
     string filName = f.FullName;
     Thread t = new Thread(delegate()   
                 { Console.WriteLine(filName); });   
     t.Start();
 }

I don't believe a race is possible here since there's no shared memory accessible from all threads.

Answer 7

The following would work as well.

    Thread t = new Thread(delegate()
    {
        string name = f.Name;
        Console.WriteLine(name);
    });