0

I got an error that should not arise:

 Task[] lestasks = new Task[5];
 
 int idplayer =0;
 int numtask = 0;

 while (numtask < 5 && idplayer < players.Count)
      {
      lestasks[numtask] = Task.Run(() => { Updateplayer(players[idplayer])); });

      numtask++;
      idplayer++;
      }

 Task.WaitAll(lestasks);

System.ArgumentOutOfRangeException, idplayer is out of range. But the while condition check if idplayer is not superior or egal to players.count.

So, how it's possible for players[idplayer] to be out of range if idplayer is inferior to players.count?

UserNam3
  • 655
  • 4
  • 11
  • 1
    Please try to create a [mre] to show us. Also, when you catch the exception in your [debugger](https://stackoverflow.com/questions/25385173/what-is-a-debugger-and-how-can-it-help-me-diagnose-problems), what is the actual value of `idplayer` and `numtask`? – Some programmer dude Jul 25 '23 at 10:56
  • 2
    The debugger will tell you. Did you tried debugging before asking? – Ralf Jul 25 '23 at 10:56
  • What is `players`? What `Updateplayer` does? – Guru Stron Jul 25 '23 at 10:56
  • 2
    Try debugging, when your code is actually run (you scheduled it with `Task.Run`), `idplayer` most likely has the value of `players.Count`. – Luke Vo Jul 25 '23 at 10:57
  • Sounds like an "access to modified captured variable" problem. Try making a copy of `idplayer` in the body of the while loop (e.g. `int idplayercopy = idplayer;`) use that in the call inside `Task.Run()` instead., i.e. `Updateplayer(players[idplayercopy])` – Matthew Watson Jul 25 '23 at 11:00
  • So what could be happening is that `idplayer++` is incremented before the task for the last iteration has run, so `idplayer` will actually be out of range at that point. Then the task is actually started and it gets a the current value of `idplayer` which will be out of range. If you make a copy before starting the task and use that copy, then that situation cannot arise. – Matthew Watson Jul 25 '23 at 11:02
  • I tried with the debugger and the error arised when idplayer = 8, and in my running setup, players had a length = 8. That's why I didn't understand how it's possible to get this error. – UserNam3 Jul 25 '23 at 11:17
  • 1
    @MatthewWatson is right, like the answer below and I confirm it after fix, it's because the task take longer to start than the loop to continue. idplayer is incremented before the task so idplayer got a too hight value. – UserNam3 Jul 25 '23 at 11:18
  • Also in this case `foreach(var player in players.Take(5))` seems to be a nicer option (`foreach` compared to `for` creates a local copy automatically) – Guru Stron Jul 25 '23 at 16:21
  • Or just `var lestasks = players.Take(5).Select(p => Task.Run(() => Updateplayer(p))).ToArray()` – Guru Stron Jul 25 '23 at 16:22

1 Answers1

3

The issue could be due to a race condition between the Task creation and the value of idplayer. The Task.Run method runs the delegate asynchronously on a thread pool thread. While you have the condition numtask < 5 && idplayer < players.Count in place, this does not guarantee that the value of idplayer won't become greater than or equal to players.Count before the task associated with lestasks[numtask] starts executing.

Here's a possible scenario of what might be happening:

  1. The while loop is evaluating the condition, and it is true since both numtask and idplayer are less than their respective limits.
  2. A task is created and associated with lestasks[numtask].
  3. Before the newly created task starts running, the value of idplayer is incremented due to idplayer++.
  4. The while loop iterates again, but now the value of idplayer is outside the range of players.Count, causing the "ArgumentOutOfRangeException."

To prevent this, you should create a local copy of idplayer before starting the task to ensure that each task has a separate value and does not modify the value of idplayer during its execution. You can do this by using a temporary variable inside the loop like this:

int numtask = 0;

while (numtask < 5 && idplayer < players.Count)
{
    int currentPlayerIndex = idplayer; // Create a local copy of idplayer
    lestasks[numtask] = Task.Run(() => { Updateplayer(players[currentPlayerIndex])); });

    numtask++;
    idplayer++;
}

Task.WaitAll(lestasks);

By using currentPlayerIndex, you ensure that each task uses its own copy of idplayer without any interference from other tasks or the loop, thus avoiding the "ArgumentOutOfRangeException" in this scenario.

Polarcode
  • 309
  • 6
  • 4
    You know that [Generative AI (e.g., ChatGPT) is banned](https://meta.stackoverflow.com/questions/421831/temporary-policy-generative-ai-e-g-chatgpt-is-banned?cb=1)? And ChatGPT generates suspiciously similar looking answer. – Guru Stron Jul 25 '23 at 11:00
  • 2
    _"The issue could be due to a race condition between the Task creation and the value of idplay"_ - issue is not the race condition but lambda closure which captures loop variable which results in all actions passed to `Task.Run` to referrer to the same data. – Guru Stron Jul 25 '23 at 11:02
  • You are absolutely right. By creating a simple temporary variable that retains the idplayer value, the problem no longer arises. So the loop continues faster than the task is created. Thanks a lot! – UserNam3 Jul 25 '23 at 11:15
  • @GuruStron I'm using a far more advanced and capable tool than any language model that will ever exist – it's called [brain](https://www.hopkinsmedicine.org/health/conditions-and-diseases/anatomy-of-the-brain#:~:text=How%20does%20the%20brain%20work,others%20make%20you%20feel%20pain.). – Polarcode Jul 25 '23 at 14:36
  • @GuruStron Also, I assumed you noticed my words: "The issue could be due." – Polarcode Jul 25 '23 at 14:41