List Index Out of Range exception when creating a

Question

The exact error:

Index was out of range. Must be non-negative and less than the size of the collection.

I've index arrays and lists countless times. I've used for loops with arrays and lists countless times. The data is there, it works. Except when I try to create a task for my function. Mind you, I successfully did this with a foreach loop for a similar function; this new one requires two arguments though, so I can't use a foreach loop properly. At least I don't think I can.

Here is the erroneous code:

if (addressList != null) {
    textBox1.Text += ("Address List Length: " + addressList.Count + Environment.NewLine);

    for (int i = 0; i < addressList.Count; i++) {
        textBox1.Text += ("Task for " + addressList[i] + ":" + portList[i] + " initiated." + Environment.NewLine);

        Task.Factory.StartNew(() => PingTaskAdapted(addressList[i], portList[i]));
    }                
}
else textBox1.Text = ("No IPs have been added.");

Assuming addressList[0] is google.com and portList[0] is 80, Output:

Address List Length: 1
Task for google.com:80 initiated.

then program break, with Visual Studio telling me that at PingTaskAdapted() I'm calling an index that is out of range, when it literally just printed the indexes in question, because they exist.

And just to be clear, if I call PingTaskAdapted(addressList[0], pingList[0]); it works with no issues.

Answer 1

You're the victim of accessing modified closure, as it's so succinctly called. Basically, since you're using a task - and a delegate to boot - the value of i is not guaranteed to be what it is you expect it to be. If you, however, copy i to a local variable, specific for the scope of one, single iteration, you should be fine.

for (int i = 0; i < addressList.Count; i++)
{
    textBox1.Text += ("Task for " + addressList[i] + ":" + portList[i] + " initiated." + Environment.NewLine);

    var iCopy = i;
    Task.Factory.StartNew(() => PingTaskAdapted(addressList[iCopy], portList[iCopy]));
}

However, as pointed out in this answer by nvoigt, it's far more clear when it comes to readability and maintainability if you copy the values which will be used rather than the iterator value.

Answer 2

Closures capture variables, not values.

Change the code to the following, and you'll see the issue go away:

for (int i = 0; i < addressList.Count; i++) {
    textBox1.Text += ("Task for " + addressList[i] + ":" + portList[i] + " initiated." + Environment.NewLine);

    var temp = i;
    Task.Factory.StartNew(() => PingTaskAdapted(addressList[temp], portList[temp]));
    }                

Answer 3

Your task will be accessing the list when the task runs. Not sequentially in the line of code you look at in the loop. To make sure that the correct values are captured in the closure (and the lists still exists and has the same values), make local copies outside of the task, that make sure the values are captured at that point in time the loop runs:

var localAddress = addressList[i];
var localPort = portList[i];
Task.Factory.StartNew(() => PingTaskAdapted(localAddress , localPort));