Should I use Task.Run or Task.FromResult?-CodePudding

I have a controller which calls a service method, this method calls synchronous a stored procedure

Service:

public class ServiceSomething : IServiceSomething, IService {
   public int ProcessSomethingInDatabase(List<SqlParameter> sqlParameters){
      IConnection connection = ConnectionFactory.GetConnection();
      //This makes a synchronous call
      DataTable dataTable = Connection.DalDataTable("sp_process_something", sqlParameters);
      int result = GetResultFromDataTable(dataTable);
      return result;
   }  
}

Controller:

public class SomethingController : Controller {
   private readonly IServiceSomething _serviceSomething;

   [HttpPost]
   public async Task<CustomResult> ProcessSomethingInDatabase(Criteria criteria){
      List<SqlParameter> sqlParameters = CriteriaToSqlParams(criteria);
      int result = await Task.Run(() => _serviceSomething.ProcessSomethingInDatabase(sqlParameters));
      return new CustomResult(result);
   }
}

That process could take a long time (from 30 seconds to 1 hour on some occasions).

The problem is that the synchronous call freezes the application, and because of that we use Task.Run. It has been requested to me that the thread don't be initialized in the controller.

Now I would like to know what is the best implementation, so much as not to freeze the application and to handle a process that can take hours to finish.

Should I really create a thread for this?

public async Task<int> ProcessSomethingInDatabaseAsync(List<SqlParameter> sqlParameters){
       IConnection connection = ConnectionFactory.GetConnection();
       return await Task.Run(() => {
          DataTable dataTable = Connection.DalDataTable("sp_process_something", sqlParameters);
          int result = GetResultFromDataTable(dataTable);
          return result;
       });
}

And the controller be

[HttpPost]
public async Task<CustomResult> ProcessSomethingInDatabase(Criteria criteria){
   List<SqlParameter> sqlParameters = CriteriaToSqlParams(criteria);
   int result = await _serviceSomething.ProcessSomethingInDatabaseAsync(sqlParameters);
   return new CustomResult(result);
}

or should I use Task.FromResult?

public Task<int> ProcessSomethingInDatabaseAsync(List<SqlParameter> sqlParameters){
       IConnection connection = ConnectionFactory.GetConnection();
       DataTable dataTable = Connection.DalDataTable("sp_process_something", sqlParameters);
       int result = GetResultFromDataTable(dataTable);
       return Task.FromResult(result);
}

Note: The service is hosted on a Windows Service and it is communicated through WCF

CodePudding user response：

The short answer to your question is none of them, let's see why.

There are at least a couple of issues in the way you are trying to design your solution.

First of all you claimed that the operation you are trying to implement could take up until 1 hour of processing. This means that you must not execute that operation in the context of an HTTP request. HTTP requests are meant to be quick, any operation that can take a time greater than a few seconds should not be implemented via HTTP. Web clients, web servers and web infrastructure are all designed for quick processing of HTTP requests and there are timeouts everywhere which won't allow you to perform your operation inside of an HTTP request.

You can use an HTTP request to ask your backend to perform a long running operation. Your web stack will process the request and will decide whether or not the task that you are requesting can be started (based on your business rules), but that's it: the actual execution of the task must be delegated to backend services (for instance by using a queue).

This is a large topic, but I hope you get the idea: you can't use an action method to perform a long running operation; your action method should only validate the request to execute the operation and delegate the actual execution to someone else. You can read this blog post to get more information about this approach.

The second point which needs attention is the usage of Task.Run. The only valid reason to use Task.Runis calling CPU-bound workload from a UI thread (for instance calling a long running CPU-bound workload from an event handler of a windows form application). That's it. If you are in a scenario other than this one you should avoid the usage of Task.Run. If you have an operation which is asynchronous in nature, such as a database query, you should use an asynchronous api to execute that operation and await the result inside of an async method. In modern .NET code there is full support for asynchronous apis, so you can always use them to perform IO operations. If you are using a data access library which doesn't offer asynchronous apis, then change your data access library.

The usage of Task.Runis particularly dangerous in the context of ASP.NET applications. In ASP.NET applications you have a pool of threads which are used by the runtime to handle incoming HTTP requests. Each time you call Task.Run you are borrowing one of this threads to execute a workload (represented by the delegate that you pass to Task.Run). You are not expected to do that, because threads are an important resource in a web server and they should be used to serve incoming HTTP requests and handled by the ASP.NET runtime. By borrowing one threads for your Task.Run execution you are interfering with the ASP.NET thread pool management and you should not do that.

To summarize, if you are writing ASP.NET code:

use asynchronous apis each time you need to perform a workload which is truly asynchronous, such as a database query or an HTTP request to a web service. await the result of the asynchronous operation inside of an async method and never block on asynchronous operations using apis such as Task.Wait() and Task.Result
if you need to perform a CPU-bound synchronous workload inside of an action method, simply do that and do not fake asynchrony by wrapping your method call by using Task.Run. Generally speaking never use Task.Run in ASP.NET code: the ony place where it makes sense is UI client applications, such as windows forms or WPF applications. Use Task.Run each time you need to call long running cpu-bound workload from a UI thread, so that you do not freeze the application UI.
never execute operations which can last more than a few seconds inside of HTTP requests. HTTP requests are meant to be processed quickly. Use a queue mechanism to delegate to backend services the execution of long running tasks.

Consider reading this blog post for more information about the usage of Task.Run.

A final note on Task.FromResult<T>, that you mentioned in your question. This method is meant to create a Task instance representing a successfully completed asynchronous operation, having a specified result. That's it. This is not a way to fake asynchrony (generally speaking you should avoid faking asynchrony), it's just a way to solve the problem of creating a Task instance for an already completed operation producing a certain result.