CODEWITHSUNDEEP
c#multithreading.net-2.0anonymous-methods
John
John

Reputation: 17501

C# 2.0 Threading Question (anonymous methods)

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?

Upvotes: 12

Views: 5773

Answers (5)

user22367
user22367

Reputation: 109

The following would work as well.

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

Upvotes: 0

Charles Bretana
Charles Bretana

Reputation: 146557

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.

Upvotes: 0

Michał Piaskowski
Michał Piaskowski

Reputation: 3850

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.

Upvotes: 6

Stewart Johnson
Stewart Johnson

Reputation: 14449

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.

Upvotes: 11

Joel Coehoorn
Joel Coehoorn

Reputation: 416121

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.

Upvotes: 1

Related Questions