4

My application sometimes stop in the below code, not always but sometimes.

All the 3 methods CalcQuarterlyFigures, CalcWeeklyFigures & CalcMonthlyFigures return Task<List<MyClass>>.

Note, this runs inside a foreach loop.

List<Task> TaskList = new List<Task>();

if(i.DoCalculateAllHistory) {

    var quarterly = CalcQuarterlyFigures(QuarterlyPrices, i.SeriesID);
    TaskList.Add(quarterly);
    var weekly = CalcWeeklyFigures(WeeklyPrices, i.SeriesID);
    TaskList.Add(weekly);
    var monthly = CalcMonthlyFigures(MonthlyPrice, i.SeriesID);
    TaskList.Add(monthly);

    Task.WaitAll(TaskList.ToArray());

    if(monthly.Result.Count > 0)
        monthlyPerfFig.AddRange(monthly.Result);

    if(weekly.Result.Count > 0)
        weeklyPerfFig.AddRange(weekly.Result);

    if(quarterly.Result.Count > 0)
        quartPerfFig.AddRange(quarterly.Result);
} else {

    monthlyPerfFig.AddRange(await CalcMonthlyFigures(MonthlyPrice, i.SeriesID));

}

Am I missing anything here that leads to dead lock ?

Pரதீப்
  • 91,748
  • 19
  • 131
  • 172
  • 1
    What is the environment? Full .NET Framework or .NET Core? `Task.WaitAll(TaskList.ToArray());` will possibly cause a deadlock in full .NET framework. Since you already using `await` in `else` condition - use `.WhenAll`: `await Task.WhenAll(TaskList.ToArray());` – Fabio Jan 02 '19 at 05:44
  • @Fabio my application is using `.NET Framework 4.6.1`. – Pரதீப் Jan 02 '19 at 05:46
  • @Fabio Thanks, let me update the code and check. Application takes almost 1 full day to complete the full run. So it might take 24 hours to confirm if its working. – Pரதீப் Jan 02 '19 at 05:47
  • @Fabio - I have a doubt. Is `await` in else condition is the problem or `Task.WaitAll(TaskList.ToArray())` is the problem ? – Pரதீப் Jan 02 '19 at 05:50
  • If methods `CalcQuarterlyFigures`, `CalcWeeklyFigures` and `CalcMonthlyFigures` are asynchronous then `Task.WaitAll` is a problem. When you are using asynchronous methods `await` is correct approach. – Fabio Jan 02 '19 at 05:53
  • @Fabio - Yeah all the 3 methods are asynchronous. So what you are suggesting is either use individual `await` in all the 3 calls or use `await Task.WhenAll(TaskList.ToArray());` – Pரதீப் Jan 02 '19 at 05:55

1 Answers1

7

In provided context (sample code of .NET 4.6.1) Task.WaitAll(TaskList.ToArray()) will cause a deadlock.
Definitely useful source: Don't Block on Async Code

You should make you code block fully asynchronous

if (i.DoCalculateAllHistory) 
{
    var quarterlyTask = CalcQuarterlyFigures(QuarterlyPrices, i.SeriesID);
    var weeklyTask = CalcWeeklyFigures(WeeklyPrices, i.SeriesID);
    var monthlyTask = CalcMonthlyFigures(MonthlyPrice, i.SeriesID);

    // Task.WhenAll accepts "params" array
    await Task.WhenAll(quarterlyTask, weeklyTask, monthlyTask);

    // You don't need to check for .Count
    // nothing will be added when empty list given to .AddRange  
    quartPerfFig.AddRange(await quarterlyTask);
    weeklyPerfFig.AddRange(await weeklyTask);
    monthlyPerfFig.AddRange(await monthlyTask);
} 
else 
{
    var monthly = await CalcMonthlyFigures(MonthlyPrice, i.SeriesID);
    monthlyPerfFig.AddRange(monthly);
}
Fabio
  • 31,528
  • 4
  • 33
  • 72
  • Thanks mate, instead of `quarterly.Result` can we use `await quarterly` ? similarly for weekly and monthly results ? like this [answer](https://stackoverflow.com/a/32119616/3349551) uses `await` instead of `Result` – Pரதீப் Jan 02 '19 at 06:11
  • You can use `await`. – Fabio Jan 02 '19 at 06:26
  • Thanks for your patience and help. I'll update you if the full run is completed successfully. – Pரதீப் Jan 02 '19 at 06:30
  • 1
    @Pரதீப் `.Result` vs. `await` after `.WhenAll` is tricky... If you use `.Result` you show that you understand how all that works but on other hand people who don't will disagree and complain till you put `await` back... Overall `await` is safe if you are concerned that someone will change code by removing `.WhenAll` as not necessary. – Alexei Levenkov Jan 02 '19 at 06:56