C# Thread seems to start multiple times

Question

This Program should rename .txt Files to .txtok. In my Test-Directory i created ~10 Textfiles.

At runtime, a FileNotFoundException was thrown. The missing File was a File which was already renamed in a previous Thread.

It seems that multiple Threads have started in one Loop-Iteration!?

static void Main(string[] args)
    {
        foreach (String s in Directory.EnumerateFiles(@"C:\Test", "*.txt", SearchOption.TopDirectoryOnly))
        {
            new Thread(() =>
            {
                File.Move(s, s + "ok");
            }).Start();                
        }
        Console.ReadKey();
    }

Does anybody had a Problem simmilar to this?

Thanks

A `new` Thread is created *each loop* (but only once *per* loop) .. surely that is to be expected from the posted code? :D However, the "issues" is **there is only one variables called `s`**. Lemme try to find some duplicates .. — , Feb 08 '13 at 23:08
http://stackoverflow.com/questions/512166/c-sharp-the-foreach-identifier-and-closures , http://stackoverflow.com/questions/4341031/different-behaviors-with-a-for-loop-and-a-foreach-loop-with-closures (follow the link in the accepted answer) , http://stackoverflow.com/questions/7133816/how-do-closures-differ-between-foreach-and-list-foreach , http://stackoverflow.com/questions/11542731/can-someone-explain-access-to-modified-closure-in-c-sharp-in-simple-terms (At least ReSharper will warn about this access, not sure if Visual Studio will by default) — , Feb 08 '13 at 23:09

Eric Lippert · Answer 1 · 2013-09-11T16:26:47.517

You are experiencing the pain of an "access to modified closure" bug. This is one of the most common problems reported on StackOverflow. Search for "access to modified closure" for more details, or read my article on the subject:

http://ericlippert.com/2009/11/12/closing-over-the-loop-variable-considered-harmful-part-one/

You can fix it by either upgrading to C# 5, or by doing:

    foreach (String s in Directory.EnumerateFiles(@"C:\Test", "*.txt", SearchOption.TopDirectoryOnly))
    {
        string s1 = s;
        new Thread(() =>
        {
            File.Move(s1, s1 + "ok");
        }).Start();                
    }

That said, this code is not good code; don't create so many threads like that. Threads are heavyweight. Treat creating a thread like you'd treat hiring a new employee; you wouldn't hire an employee to rename a file and then fire them; it's too expensive. You'd hire one employee to rename all the files.

Not sure I agree that it is a bug, but I do agree that the whole thing should be on 1 background thread, not a new thread for each file. — PeteGO, Feb 08 '13 at 23:15
Thanks for your answer! I know the code makes no sense. This bug happened in a bigger application where it makes sense. — fr_0_g, Feb 08 '13 at 23:19

Chris Barrett · Answer 2 · 2013-02-08T23:32:28.010

The issue is caused by the interaction between foreach loops and lambda capture.

The variable s is overwritten in each iteration of the foreach loop. This means that the reference to s captured by the lambda in new Thread has changed by the time the new thread is executed.

The first thread will execute successfully, but the remainder of the threads are also pointing to the same value of s and will fail.

The solution is to create a temporary variable:

foreach (String s in Directory.EnumerateFiles(@"C:\Test", "*.txt", SearchOption.TopDirectoryOnly))
{
  var temp = s;
  new Thread(() => File.Move(temp, temp + "ok")).Start();                
}

As per @Eric Lippert's suggestion, consider using PLINQ or the TPL to manage the threading for you:

// assume `using System.Linq`

Directory.EnumerateFiles(@"C:\Test", "*.txt", SearchOption.TopDirectoryOnly)
         .AsParallel()
         .Select(f => File.move(f, f + "ok"))
         .ToList();

This neatly avoids the issues with foreach and gives the runtime control over the threading behaviour.

C# Thread seems to start multiple times

2 Answers2