2

I'm getting an ArgumentOutOfRangeException when I'm really not sure why.

Task[] downloadTasks = new Task[music.Count];
for (int i = 0; i < music.Count; i++)
    downloadTasks[i] = Task.Factory.StartNew(() => DownloadAudio(music[i], lstQueue.Items[i]));
Task.Factory.ContinueWhenAll(downloadTasks, (tasks) =>
{
    MessageBox.Show("All the downloads have completed!",
        "Success",
        MessageBoxButtons.OK,
        MessageBoxIcon.Information);
});

The error occurs when the for loop runs when i = 1 and I'm not sure why it does this when I'm positive that music.Count = 1.

I always tried this approach as an alternative to the for loop and got the same exception:

int index = 0;
foreach (MusicFile song in music)
{
    downloadTasks[index] = Task.Factory.StartNew(() => DownloadAudio(song, lstQueue.Items[index]));
    index++;
}

Is there anything in the above code that might cause this?

I'm also not sure if this is relevant, but when I can accomplish the same thing using threads without any exception. It was only when I tried implementing tasks that this exception appeared.

Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321
Minato
  • 352
  • 3
  • 13

3 Answers3

4

This happens because you're passing StartNew a Lambda Expression, which implicitly captures your i variable. This effect is called Closure.

In order to get the proper behavior, you'll have to make a local copy of your index:

for (int i = 0; i < music.Count; i++)
{
    var currentIndex = i;
    downloadTasks[i] = Task.Factory.StartNew(() => 
                                             DownloadAudio(music[currentIndex],
                                             lstQueue.Items[currentIndex]));
}
Community
  • 1
  • 1
Yuval Itzchakov
  • 146,575
  • 32
  • 257
  • 321
  • While this does solve the problem above, now I'm getting an `InvalidOperationException` for trying to access `lstQueue.Items`. – Minato Dec 28 '14 at 07:33
  • That's because `lstQueue` is probably a UI object, which you're trying to access from a background thread (and is not permitted). What does it contain? – Yuval Itzchakov Dec 28 '14 at 07:38
  • It's a `ListView` that hold the progress percentage for a download. I'm trying to update the progress of each download simultaneously. But I'll figure out how to do what I'm trying to. Thank you! – Minato Dec 28 '14 at 07:41
  • 3
    @Minato Then look into [`Progress`](http://msdn.microsoft.com/en-us/library/hh193692%28v=vs.110%29.aspx). Does exactly what you need. – Yuval Itzchakov Dec 28 '14 at 07:42
1

In both instances, you are closing over the loop variable i in the first example, or your manually assigned index in the second.

What is happening is that the final value of i / index is used after the loop completion, which is when i++ has incremented beyond the size of the iterated array. (See also here)

Either capture the value of i inside the loop with an additional variable as per @Yuval, or alternatively, look at ways of coupling the two collections together, such that you do not need to iterate music and lstQueue independently, e.g. here we pre-combine the two collections into a new anonymous class:

var musicQueueTuples = music.Zip(lstQueue, (m, q) => new {Music = m, QueueItem = q})
    .ToList();

// Which now allows us to use LINQ to project the tasks:
var downloadTasks = musicQueueTuples.Select(
       mqt => Task.Factory.StartNew(
         () => DownloadAudio(mqt.Music, mqt.QueueItem))).ToArray();

Task.Factory.ContinueWhenAll(downloadTasks, (tasks) => ...
Community
  • 1
  • 1
StuartLC
  • 104,537
  • 17
  • 209
  • 285
0

Closures is your problem, where the variable i is being referenced by the lambada expression, thus it has access to i and always reads its value directly from the memory.

You can create a factory function that create task handlers. You can follow the following idea to solve the problem.

private Action CreateTaskHandler(int arg1)
{
    return () => DownloadAudio(music[arg1], lstQueue.Items[arg1])
}

Task[] downloadTasks = new Task[music.Count];
for (int i = 0; i < music.Count; i++)
    downloadTasks[i] = Task.Factory.StartNew(CreateTaskHandler(i));
}
Oday Fraiwan
  • 1,147
  • 1
  • 9
  • 21