C# - Index was outside the bounds of the array using same list sizes in for loop

Question

I am creating a file downloader in .NET which downloads an array of files from a server using Asynchronous tasks. However, even though I create the Task[] and returned string[] with the same length.

Here is my method:

    public static string[] DownloadList(string[] urlArray, string[] toPathArray, string login = "", string pass = "", bool getExt = false)
    {
        Console.WriteLine("DownloadList({0}, {1}, {2}, {3}, {4})", urlArray, toPathArray, login, pass, getExt);
        try {
            returnedArray = new string[urlArray.Length];
            Task[] taskArray = new Task[urlArray.Length];
            for (int i = 0; i < urlArray.Length; i++)
            {
                Thread.Sleep(1000);
                Console.WriteLine("i = {0}", i);
                Task task = new Task(() => { returnedArray[i] = Download(urlArray[i], toPathArray[i], login, pass, getExt, true); });
                task.Start();
                taskArray[i] = task;
            }
            Task.WaitAll(taskArray);
            Thread.Sleep(1000);
            Console.WriteLine();
            Console.WriteLine("Done! Press Enter to close.");
            Console.ReadLine();
            return returnedArray;
        }
        catch(Exception e)
        {
            Console.WriteLine();
            Console.WriteLine(e.Message);
            Console.ReadLine();
            return null;
        }
    }

and the exception:

which points to this line:

I know it will be something daft that I missed, but I'm rattling my brain trying to figure it out. Thanks for the help in advance!

on the last iteration when `i` is 9 you run task `task.Start();` and it's very high chance to work after iteration ends and `i++` works and in that case your `i` will become `10` — Samvel Petrosov, May 25 '17 at 15:27
Make a local copy of `i` to use in the lambda. Your lambda doesn't execute until after the loop is complete, when i = 10 -- for all ten copies of the lambda. — 15ee8f99-57ff-4f92-890c-b56153, May 25 '17 at 15:27
@jdweng `i < a.Length` is correct. If `a.Length == 10`, `i` will never exceed 9. You're thinking `<=`. — 15ee8f99-57ff-4f92-890c-b56153, May 25 '17 at 15:28
@S.Petrosov It's quite possible for it to be on *all* iterations. The loop will be done in a jiffy... scheduling the tasks into the ThreadPool (or whereever) will likely take longer. — spender, May 25 '17 at 15:29
@spender I agree, but on the last iteration it's more easy to imagine :) — Samvel Petrosov, May 25 '17 at 15:30
They do me too @FedericoDipuma, but unfortunately I have to, as the API I make the requests to rejects multiple requests within a certain time — Timmo, May 25 '17 at 15:37
Actually, I now somehow don't need the `Thread.Sleep` calls. No idea what happened but the api is ok with multiple requests at once now. Super quick too — Timmo, May 25 '17 at 16:04

score 8 · Accepted Answer · answered May 25 '17 at 15:28

8

It looks like Access to Modified Closure problem. Copy i to local variable:

for (int i = 0; i < urlArray.Length; i++)
{
    int index = i;
    Thread.Sleep(1000);
    Console.WriteLine("i = {0}", index );
    Task task = new Task(() => { returnedArray[index] = Download(urlArray[index], toPathArray[index], login, pass, getExt, true); });
    task.Start();
    taskArray[index] = task;
}

answered May 25 '17 at 15:28

Backs

24,430
5
58
85

Thank you. That was it. I just added a local int with the index and used it inside the loop instead – Timmo May 25 '17 at 15:36

score 2 · Answer 2 · answered May 25 '17 at 15:32

@Backs's answer is correct. Interestingly, in recent versions of .Net, if you use foreach, then the loop variable is closed over. So, you could:

foreach(var i in Enumerable.Range(0, urlArray.Length))
{
    ... new Task(() => { returnedArray[i] = ...
}

without needing to take a copy.

score -1 · Answer 3 · answered May 25 '17 at 15:31

Looks like you are overshootinhg the array by one, because you count the length of the array in the cycle, and that's 10, while the array goes from 0 to 9.

 for (int i = 0; i < urlArray.Length; i++)

Should be

 for (int i = 0; i < urlArray.Length - 1; i++)

C# - Index was outside the bounds of the array using same list sizes in for loop

3 Answers3