0

I'm fairly new to programming and decided to test out an idea I had involving asynchronous execution of array sorting tasks. I'm almost completely new to asynchronous programming and am running into an error that seems to only be explainable via some sort of strange asynchronous... stuff... occurring.

When I step through the following code to debug it, it works perfectly. When I allow it to run freely, however, I run into an Argument Out Of Range Exception: I becomes greater than allArrays.Count - 1, and the program fails. However, the if (i >= allArrays.Count) Console.WriteLine($"i is too high! i = {i}");

line is never executed before it fails. Can someone explain this to me, and help possibly propose a solution to this issue?

Thank you!

          //iterate while allArrays contains multiple arrays to be merged.
            while (allArrays.Count > 1)
            {
                if (allArrays.Count % 2 != 0)//if it's not even, we add one and make it even, so every array has a partner!
                {
                    allArrays.Add(new int[0]);
                }

                for (int i = 0; i < allArrays.Count - 1; i+=2)
                {
                    Console.WriteLine($"i is {i}");
                    if (i >= allArrays.Count) Console.WriteLine($"i is too high! i = {i}");
                    mergeTasks.Add(Task.Run(() => Merge(allArrays[i], allArrays[i + 1])));
                }
                await Task.WhenAll(mergeTasks);

                //empty the list of smaller arrays
                allArrays.Clear();
                //add results to all arrays then empty mergeTasks
                mergeTasks.ForEach(r => allArrays.Add(r.Result));
                mergeTasks.Clear();
            }
SpicyMike
  • 67
  • 1
  • 5
  • 2
    Can you add `Merge` method? – Guru Stron Jul 01 '20 at 21:25
  • 1
    Whenever something doesn't happen in debug but does in full speed it can be some kind of timing issue. Here I wonder if your loop is executing so fast that your Task doesn't set up in time to grab the current value of i before it's incremented, so ends upcalling `allArrays[i]` or `i+1` when i is already bigger than the end of the array. Maybe jiggle things around so you pass the `allArrays` and `i` to Merge separately and see what value of `i` is being given when the code is full speed (put your "too high" check into Merge) – Caius Jard Jul 01 '20 at 21:26

1 Answers1

3

Add temporary variable to store i and use it in lambda for task:

for (int i = 0; i < allArrays.Count - 1; i+=2)
{
    var tmp = i; // create copy of current i
    Console.WriteLine($"i is {i}");
    if (i >= allArrays.Count) Console.WriteLine($"i is too high! i = {i}");
    // use tmp here instead of i: 
    mergeTasks.Add(Task.Run(() => Merge(allArrays[tmp], allArrays[tmp  + 1]))); 
}

Task.Run accepts lambda. To use a variable from outside scope it will create so called closure. For the for loop the same closure instance is used, so the captured value can change, so it is possible for it to change to the last value(which is i > allArrays.Count - 1) before your last task calls Merge(allArrays[i], allArrays[i + 1]) resulting in the exception in the question.

You can try to "validate" this behavior for example with:

 mergeTasks.Add(Task.Run(() => {
        if (i >= allArrays.Count) Console.WriteLine($"i is too high! i = {i}");
        return Merge(allArrays[tmp], allArrays[tmp  + 1]);
    }));

You can dive deeper with this article or this question.

Guru Stron
  • 102,774
  • 10
  • 95
  • 132