List Index Out of Range exception when creating a task

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.

You should use `Enumerable.Zip`. – Alexander Jul 12 '17 at 18:55 — Alexander, Jul 12 '17 at 18:55

score 18 · Accepted Answer · answered Jul 12 '17 at 09:17

18

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));

answered Jul 12 '17 at 09:17

nvoigt

75,013
26
93
142

1

I'd prefer this way of doing it, despite my own answer (which I wrote the way I did for clarity about which variable was modified). This answer's code snippet makes it abundantly clear *which* value will be used in the task execution. – J. Steen Jul 12 '17 at 09:19
This is a very strange phenomenon. Interesting to learn about. My code does indeed work now, thank you. – soxroxr Jul 12 '17 at 09:48

J. Steen · Answer 2 · 2017-07-12T09:26:38.740

8

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.

edited Jul 12 '17 at 09:26

answered Jul 12 '17 at 09:16

J. Steen

15,470
15
56
63

Do you have any references about this? id like to read up on it. – thanatorr Jul 12 '17 at 09:18
Thank you for the additional information about accessing modified closure. – soxroxr Jul 12 '17 at 09:51
@thanatorr Jon Skeet has a good article about the subject: [The Beauty of Closures](https://csharpindepth.com/articles/Closures). Especially *Comparing capture strategies: complexity vs power* section is relevant. – Mustafa Özçetin Jun 21 '23 at 15:25

score 5 · Answer 3 · answered Jul 12 '17 at 09:17

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]));
    }

List Index Out of Range exception when creating a task

3 Answers3

Linked

Related