1

Consider the following code:

attempt = 0;
for (int counter = 0; counter < 8; counter++)
{
    if (attempt < totalitems)
    {
        Tasklist<output>.Add(Task.Run(() =>
        {
            return someasynctask(inputList[attempt]);
        }));
    }
    else
    {
        break;
    }
    attempt++;
}
await Task.WhenAll(Tasklist).ConfigureAwait(false);

I want to have for example 8 concurrent tasks, each working on different inputs concurrently, and finally check the result, when all of them have finished. Because I'm not awaiting for completion of Task.Run() attempt is increased before starting of tasks, and when the task is started, there may be items in the inputList that are not processed or processed twice or more instead (because of uncertainty in attempt value.

How to do that?

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
Hesam
  • 27
  • 1
  • 4
  • You should provide a MCVE... anyway, as I understand what you are telling, your problem is the variable `attempt` doesn't have the expected value when you start the tasks, am I correct ? – gobes Oct 18 '17 at 07:41
  • PLINQ may help. – qxg Oct 18 '17 at 07:51
  • Yes, You are correct! attempt value is changed more sooner than starting the task @gobes – Hesam Oct 18 '17 at 09:35
  • Isn't it a problem related to closures? Take a look here: http://csharpindepth.com/Articles/Chapter5/Closures.aspx – FCin Oct 18 '17 at 09:44

2 Answers2

5

The problem lies within the use of a "lambda": when Task.Run(() => return someasynctask(inputList[attempt])); is reached during the execution, the variable attempt is captured, not its value (i.e. it is a "closure"). Consequently, when the lambda gets executed, the value of the variable at that specific moment will be used.

Just add a temporary copy of the variable before your lambda, and use that. E.g.

if (attempt < totalitems)
{
    int localAttempt = attempt;
    Tasklist<output>.Add(Task.Run(() =>
    {
        return someasynctask(inputList[localAttempt]);
    }));
}
Bernhard Hiller
  • 2,163
  • 2
  • 18
  • 33
  • Could you tell me how to do that? – Hesam Oct 18 '17 at 09:37
  • I created a copy inside lambada, and it works now, but still a question? Do this solution work always, or maybe sometimes may be some inconsistency? – Hesam Oct 18 '17 at 10:02
  • 1
    'localAttempt' should be declared in the lambda itself to force the copy of the value; in your code there is still a closure problem. – gobes Oct 18 '17 at 10:03
  • I don't know what is closure problem :( There's a lot to read in the csharpindepth.com/Articles/Chapter5/Closures.aspx. I want a more simple tutorial @gobes – Hesam Oct 18 '17 at 10:06
0

Thanks to @gobes for his answer:

Try this:

attempt = 0;
for (int counter = 0; counter < 8; counter++)
{
    if (attempt < totalitems)
    {
        Tasklist<output>.Add(Task.Run(() =>
        {
            int tmpAttempt = attempt;
            return someasynctask(inputList[tmpAttempt]);
        }));
    }
    else
    {
        break;
    }
    attempt++;
}
await Task.WhenAll(Tasklist).ConfigureAwait(false);

Actually, what the compiler is doing is extracting your lambda into a method, located in an automagically generated class, which is referencing the attempt variable. This is the important point: the generated code only reference the variable from another class; it doesn't copy it. So every change to attempt is seen by the method.

What happens during the execution is roughly this:

enter the loop with attempt = 0 add a call of the lambda-like-method to your tasklist increase attempt repeat After the loop, you have some method calls awaiting (no pun intended) to be executed, but each one is referencing THE SAME VARIABLE, therefore sharing its value - the last affected to it.

For more details, I really recommend reading C# in depth, or some book of the same kind - there are a lot of resources about closure in C# on the web :)

Hesam
  • 27
  • 1
  • 4
  • 4
    int tmpAttempt = attempt; should be above Task.Run, right? bad bug for someone who stumbles here, better scroll to next answer. – colin lamarre Dec 20 '19 at 09:41
  • 1
    ***THIS ANSWER IS WRONG***. See this [question](https://stackoverflow.com/q/271440/903600). – fgrieu May 09 '21 at 23:01