CODEWITHSUNDEEP
c#asynchronoustask-parallel-libraryasync-await
jbhelicon
jbhelicon

Reputation: 133

Strange behavior of Task.Run

I've working on unravelling C# async programming, and the pattern below is clearly wrong, but in a way that I don't understand. What I set out to do was fire off n tasks, and then wait for them all to finish. When it became clear that i was baffled, I began to distill things down, and eventually created this:

    private async Task<bool> doLoop()
    {
        int loopCnt = 3;
        int[] sleeperReturns = new int[loopCnt + 1];
        for (int i = 0; i < loopCnt; i++)
        {
            System.Diagnostics.Debug.WriteLine("doLoop: i={0} calling Sleeper", i);
            Task.Run(async () => { sleeperReturns[i] = await sleep(i); });
            System.Diagnostics.Debug.WriteLine("doLoop: i={0} called  Sleeper", i);
        }
        return true;
    }

    private async Task<int> sleep(int i)
    {
        System.Diagnostics.Debug.WriteLine("Sleep: begins i={0}", i);
        await Task.Delay(5000 + i * 1000);
        System.Diagnostics.Debug.WriteLine("Sleep: ends   i={0}", i);
        return i;
    }

This produces the following output:

doLoop: i=0 calling Sleeper
doLoop: i=0 called  Sleeper
doLoop: i=1 calling Sleeper
doLoop: i=1 called  Sleeper
doLoop: i=2 calling Sleeper
doLoop: i=2 called  Sleeper
Sleep: begins i=3
Sleep: begins i=3
Sleep: ends   i=3
Sleep: ends   i=3
Sleep: ends   i=3

So, the mysteries to me are:

(1) Why don't any of the Sleep tasks begin until the doLoop is done?

(2) Why, when they are called and when they return, do they get the value of i from the end of the loop, rather than the value at the time at which they were called? The corollary to this is how would one pass local state to a new task correctly

Again, apologies because I know the code is wrong, but it sure seems to generate a peculiar behavior.

Upvotes: 0

Views: 516

Answers (1)

Servy
Servy

Reputation: 203812

  1. You're not calling await on the result of Task.Run, or otherwise doing anything with it. You're throwing that task onto the floor.

  2. You're closing over the loop variable. Make a copy of i inside of the loop and close over that copy.

Because you're not awaiting Task.Run you're firing and then forgetting all of the tasks, rather than performing them one at a time. Because you're closing over the loop variable, by the time any of the tasks actually runs i has been set to three.

You will get a warning when compiling this code on the call to Task.Run specifically indicating that you probably should have been awaiting it. You will also get a warning that doLoop is not awaiting anything, despite being async, this is another indication that something is wrong here.

If you want them to run sequentially then you can ignore the loop closure issue and just add in the await. If you want them to run in parallel then stick all of the tasks into a collection of tasks that you can call await Task.WhenAll(...) on.

On a side note, if you do want to parallelize all of the calls to sleep there's no need to use Task.Run at all. At least not as long as sleep has a sensible asynchronous implementation. If you simply call it N times and stick the resulting tasks into a collection they will all run asynchronously and in parallel, without the added overhead of asking the thread pool thread to start the asynchronous operation and put the result into an array. There's no need for that overhead. Also note that if you have a collection of Task<T> then WhenAll will give you a Task<T[]>, so there's no need to explicitly put all of the individual results into an array. However, your use of Task.Run is somewhat inefficient and needlessly complicates the code, it is not what is causing it to not work.

Upvotes: 6

Related Questions