I have this piece of code (in synchronous method):

// ...
var processingTasks = Enumerable.Range(0, count)
    .Select(x => Task.Run(async () =>    
      await CallApiAndUpdateSthAsync(accounts[x], cancellationToken)))
    .ToList();

var processingTasksAggregate = Task.WhenAll(processingTasks);

Task.WaitAny(globalStatusMonitoringTask, processingTasksAggregate);

I am especially interested in this part:

Task.Run(async () => await CallApiAndUpdateSthAsync(accounts[x], cancellationToken))

Would it make a difference (from a possible deadlock perspective) if I removed the Task.Run? I.e. changed it to:

CallApiAndUpdateSthAsync(accounts[x], cancellationToken)

I wanted to remove it, because queueing the task on a thread pool (via Task.Run) seemed redundant, but my colleague advised me no to, because of possible deadlocks

The whole method is run as background job in sth similar to Hangfire:

[Edit 1]: It's a .NET Framework (not Core) application.

CodePudding user response：

Would it make a difference (from a possible deadlock perspective) if I removed the Task.Run?

Possibly. There are two parts to the common deadlock (link to my blog):

1. Blocking on asynchronous code. Your code is doing this with Task.WaitAny.
2. A context that only allows one thread at a time.

Since your code clearly has (1), then you can conclude it will deadlock if it has (2). Common contexts that can cause deadlocks include ASP.NET pre-Core and GUI thread contexts. If your app doesn't have a one-thread-at-a-time context, then there is no chance of deadlock.

I wanted to remove it, because queueing the task on a thread pool (via Task.Run) seemed redundant

Not really. "Asynchronous" does not mean "run on a different thread", so the two aren't redundant. The question is whether removing the Task.Run would work. It might, or it might not. If this code is run in a single-threaded context, then removing the Task.Run would cause a deadlock. If CallApiAndUpdateSthAsync may run synchronously (i.e., due to cache hits), then removing the Task.Run would cause the invocations to be run serially rather than in parallel, and that may not be desirable, either.

CodePudding user response：

If it's an asp.net core application (not just old school asp.net) there is no problem with task deadlock. SynchronizationContext (which can cause deadlock without ConfigureAwait(false)) is null in asp.net core. There isn’t a SynchronizationContext in ASP.NET Core.

Also without Task.Run - CallApiAndUpdateSthAsync will be executed on your current thread until the first await inside. With Task.Run - entire CallApiAndUpdateSthAsync will be executed out of your main execution. You can fix it with adding Task.Yield in the beginning of CallApiAndUpdateSthAsync.