1

I have the folowing code writing to a textbox:

for (int ii = 0; ii < 5; ii++)
{
    textBox1.Text += String.Format("Writing Line {0}{1}", ii + 1, System.Environment.NewLine);
}

I get the expected output off:

Writing Line 1
Writing Line 2
Writing Line 3
Writing Line 4
Writing Line 5

However when I update asyncornusly or from another thread as found here: How to update the GUI from another thread in C#?

for (int ii = 0; ii < 5; ii++)
{
    textBox1.BeginInvoke(new Action(() => textBox1.Text += String.Format("Writing Line {0}{1}", ii + 1 , System.Environment.NewLine)));
}

I get

Writing Line 5
Writing Line 5
Writing Line 5
Writing Line 5
Writing Line 5

The last line is printed 5 times where I expect the output to match the single-thread synchronous output?

What do I need to change to match the first output from the second thread?

Community
  • 1
  • 1
Michael Elkin
  • 221
  • 2
  • 4
  • 15

2 Answers2

2

This is because you are referencing the loop variable ii inside an asynchronous action. By the time the action is called, the loop has already finished, and the value of ii is 4. That is why you get all fives (i.e. 4+1) in the asynchronous version.

Making a temporary for ii would fix the problem:

for (int ii = 0; ii < 5; ii++) {
    var iii = ii;
    textBox1.BeginInvoke(new Action(() => textBox1.Text += String.Format("Writing Line {0}{1}", iii + 1 , System.Environment.NewLine)));
}

This error is caused by access to modified closure. See this Q&A for more information on it.

Community
  • 1
  • 1
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523
  • Thank you, that was it, I tried doing something like that, but I used a variable outside the loop instead of new variable for every loop iteration. I feel like such an idiot. Thank you again. – Michael Elkin Oct 21 '14 at 15:28
  • @MichaelElkin In all fairness, this is an extremely tricky issue to spot when you see it for the first time. In fact, I am surprised that C# compiler did not fix this automatically from the start. The issue is so prevalent that Microsoft has finally decided to fix it in C# 5.0. – Sergey Kalinichenko Oct 21 '14 at 15:33
0

The variable ii is treated as begin defined outside the loop. It's compiled as if it looks like this:

int ii;
for (ii = 0; ii < 5; ii++)
{
    textBox1.Text += String.Format("Writing Line {0}{1}", ii + 1, System.Environment.NewLine);
}

Because of this your lambda is capturing the shared variable, not the unique instance of the value. You need to make a copy within the loop:

for (int ii = 0; ii < 5; ii++)
{
    int localCopy = ii;

    textBox1.Text += String.Format("Writing Line {0}{1}", localCopy  + 1, System.Environment.NewLine);
}

This use to be a problem with the foreach statement, too. In C# 5 they changed the behaviour of foreach so that the variable is treated as being declared within the body of the foreach, so the capture will work correctly. Because of this you could write:

foreach(int ii in Enumerable.Range(0,5))
{
    textBox1.BeginInvoke(new Action(() => textBox1.Text += String.Format("Writing Line {0}{1}", ii + 1 , System.Environment.NewLine)));
}

The important point here is that for and foreach behave differently in how they capture. With for the variable is deemed to be outside the loop, with foreach it is deemed to be within (and therefore safe to capture).

Sean
  • 60,939
  • 11
  • 97
  • 136