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?

Stewart Johnson · Accepted Answer · 2008-10-30T14:05:02.177

11

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.

edited Oct 30 '08 at 14:05

answered Oct 30 '08 at 13:57

Stewart Johnson

14,281
7
61
70

1

Note for future readers: this behaviour [will actually change](http://stackoverflow.com/a/8899347/137188) in C# 5.0. Each iteration will create a new, separate loop variable. With that change, the code in this question would behave as the asker originally expected. – tcovo Jan 24 '12 at 17:43

Michał Piaskowski · Answer 2 · 2008-10-30T17:03:30.507

6

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.

edited Oct 30 '08 at 17:03

answered Oct 30 '08 at 14:22

Michał Piaskowski

3,800
2
34
46

Yep, sure enough that works! Thanks! I was originally thinking the foreach loop was doing that automatically (new 'f' variable per iteration), but I guess that doesn't really make any sense for it to work like that – John Oct 30 '08 at 15:25

score 1 · Answer 3 · answered Oct 30 '08 at 14:01

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.

In this code the threads access f variable before the main thread has finished modifying it. — Michał Piaskowski, Oct 30 '08 at 15:38

score 0 · Answer 4 · answered Oct 30 '08 at 17:28

0

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.

answered Oct 30 '08 at 17:28

Charles Bretana

143,358
22
150
216

there is a race condition, f is shared. You can test it by making every thread sleep for a few milliseconds: Thread t = new Thread(delegate() { Thread.Sleep(300); Console.WriteLine(f.FullName); }); – Michał Piaskowski Oct 30 '08 at 21:05
f is shared, but string filName is not... it's a stack frame variable and will be specific to each thread... – Charles Bretana Oct 30 '08 at 22:52
for it to be a race there has to be a shared memory variable whose value is mutable, and wrong during some portion of the processing... string filename is initialied from f.FullNam before the threads are created, and it's value is stored on the thread-specific stack frame – Charles Bretana Oct 30 '08 at 22:56

score 0 · Answer 5 · answered Oct 30 '08 at 20:51

0

The following would work as well.

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

answered Oct 30 '08 at 20:51

user22367

109
1
5

I don't think this will work. f is still declared outside the loop and accessed inside anonymous delegate – Michał Piaskowski Oct 30 '08 at 21:08

C# 2.0 Threading Question (anonymous methods)

5 Answers5