runs false if statement when creating new threads

Question

I'm writing a program that will email Cat Facts to anybody on a mailing list (not spamming, this is more of a learning experience than anything else). I have 2 arrays, 1 for facts (no problems) and one for emails. The one for emails will always throw an exception (even when in a try/catch block) and all of my if statements are completely ignored when this runs.

public void Broadcast() {
    fact++;
    if (fact ==factCount) {
        fact = 0;
    }
    String[] mailArray = mailingList.ToArray();
    String[] factArray = factList.ToArray();
    int i = mailArray.Length-1;
    String sendFact = factArray[fact];
    while (i>=0) {
        String s = mailArray[i];
        var t = new Thread(() => Mail(mailArray[i], sendFact));
        t.IsBackground = true;
        t.Start();
        i--;
        if (i==-1) {
            break;
        }
    }

The problem is here:

var t = new Thread(() => Mail(mailArray[i], sendFact));

Even when 'i' is -1, the while statement still runs and the break gets ignored. Originally I ran 'i' in reverse, starting at 0 then counting to mailArray.Length-1. It still gave the same issue, ignoring the while and break when I had them set to run in reverse.

Is this a problem with creating new threads?

Edit:

I moved:

    var t = new Thread(() => Mail(mailArray[i], sendFact));
    t.IsBackground = true;
    t.Start();

To a new method and called that instead of the code above - that works perfectly fine.

The lambda captures the `i` *variable*, not *value*. So by the time the threads start, `i` is `-1`. — Cameron, Jul 27 '14 at 20:31
Related: http://stackoverflow.com/questions/451779/how-to-tell-a-lambda-function-to-capture-a-copy-instead-of-a-reference-in-c — Douglas, Jul 27 '14 at 20:32
General advice: Use a foreach loop and don't use threads as a beginner (very hard to get right). — usr, Jul 27 '14 at 20:33

score 5 · Answer 1 · answered Jul 27 '14 at 20:32

Is this a problem with creating new threads?

Sort of. It's a problem with capturing i in a lambda expression, and then changing it. It means that when the thread executes your lambda expression, it takes the current value of i, not the value of i in the loop iteration that you created the thread in.

The simplest solution is to declare a new local variable inside the loop, copy i into that, and then use that variable in the lambda expression:

while (i >= 0)
{
    var copy = i;
    String s = mailArray[i];
    var t = new Thread(() => Mail(mailArray[copy], sendFact));
    t.IsBackground = true;
    t.Start();
    i--;
}

Or just use s, which you've already initialized outside the lambda expression:

while (i >= 0)
{
    String s = mailArray[i];
    var t = new Thread(() => Mail(s, sendFact));
    t.IsBackground = true;
    t.Start();
    i--;
}

Or, rather more readably, IMO, just use a foreach loop:

foreach (var item in mailArray)
{
    Thread t = new Thread(() => Mail(item, sendFact)) { IsBackground = true };
    t.Start();
}

This will have the same problem under C# 3 and 4, but under C# 5 the behaviour of capturing the iterator variable in a foreach loop was fixed.

(You should consider using Task instead of Thread, and potentially using something like Parallel.ForEach instead, btw.)

score 1 · Accepted Answer · answered Jul 27 '14 at 20:32

1

You are creating a closure over the loop variable - this is a well known problem.

Using s directly, as you already have assigned to it the current value, would fix it:

while (i>=0)
{
  string s = mailArray[i];
  var t = new Thread(() => Mail(s, sendFact));
  //..
}

answered Jul 27 '14 at 20:32

BrokenGlass

158,293
28
286
335

Yep, that works. I tried to make the string s to see if it would throw an exception, I didn't think of just using s to start the thread. – user3742977 Jul 27 '14 at 20:35

score 0 · Answer 3 · answered Jul 27 '14 at 20:32

This is a common problem, and it's related to the creation of this lambda expression:

() => Mail(mailArray[i], sendFact)

Basically, you're telling the thread class to start a new thread, and when it does, to execute that code.

Unfortunately, it will not make a snapshot of what the value of i is at that time, so let's say that due to timing, before the thread actually starts, you manage to increase i. The thread will call that lambda, using the current value of i, not the value i had when you created the lambda.

This is easy to fix, however.

Inside the while-loop, add a temporary variable to capture the current value of i, and use that variable in your lambda expression:

int temp = i;
var t = new Thread(() => Mail(mailArray(temp), sendFact));

For more information, check out the following link: C# in Depth: The Beauty of Closures.

runs false if statement when creating new threads

3 Answers3