Implementation Async/Await

Question

I have a simple ASP.NET Web API project. Now I want to make all basic methods as asynchronous. I tried to do something, and it's working but I'm not sure that it was implemented properly.

Please check whether I properly implemented async/await in my project.

I had implemented methods like below in my repository:

        public T GetById(object id)
        {
            return this.Entities.Find(id);
        }
        public void Insert(T entity)
        {
            try
            {
                if (entity == null)
                {
                    throw new ArgumentNullException("entity");
                }
                this.Entities.Add(entity);
                this.context.SaveChanges();
            }
            catch (DbEntityValidationException dbEx)
            {
                foreach (var validationErrors in dbEx.EntityValidationErrors)
                {
                    foreach (var validationError in validationErrors.ValidationErrors)
                    {
                        errorMessage += string.Format("Property: {0} Error: {1}",
                        validationError.PropertyName, validationError.ErrorMessage) + Environment.NewLine;
                    }
                }
                throw new Exception(errorMessage, dbEx);
            }
        }

I tried to create async methods:

        public Task InsertAsync(T entity)
        {
            return Task.Run(() => Insert(entity));
        }
        public Task<T> GetByIdAsync(object id)
        {
            return Task.Run(() => GetById(id));
        }

And now I'm using it as:

      public async virtual Task<HttpResponseMessage> Post([FromBody]T entity)
      {
                try
                {
                    await repository.InsertAsync(entity);
                    return Request.CreateResponse(HttpStatusCode.Created, entity);             
                }
                catch (Exception ex)
                {
                    return ErrorMsg(HttpStatusCode.InternalServerError, ex.Message);
                }

       }
       public async virtual Task<HttpResponseMessage> GetById(int id)
       {
            var entity = await repository.GetByIdAsync(id);

            if (entity == null)
            {                
                var message = string.Format("No {0} with ID = {1}", GenericTypeName, id);
                return ErrorMsg(HttpStatusCode.NotFound, message);
            }
            return Request.CreateResponse(HttpStatusCode.OK, entity);
       }

Answer 1

Your implementation is technically correct, but...

There is no benefit in using Task.Run in an web application.

All you're doing is deferring work from one thread to another. So, in fact, not only is there no benefit, you're actually adding overhead.

I'll quote Stephen Cleary, because I could nevet put it better:

There are (at least) four efficiency problems introduced as soon as you use await with Task.Run in ASP.NET:

• Extra (unnecessary) thread switching to the Task.Run thread pool thread. Similarly, when that thread finishes the request, it has to enter the request context (which is not an actual thread switch but does have overhead).
• Extra (unnecessary) garbage is created. Asynchronous programming is a tradeoff: you get increased responsiveness at the expense of higher memory usage. In this case, you end up creating more garbage for the asynchronous operations that is totally unnecessary.
• The ASP.NET thread pool heuristics are thrown off by Task.Run “unexpectedly” borrowing a thread pool thread. I don’t have a lot of experience here, but my gut instinct tells me that the heuristics should recover well if the unexpected task is really short and would not handle it as elegantly if the unexpected task lasts more than two seconds.
• ASP.NET is not able to terminate the request early, i.e., if the client disconnects or the request times out. In the synchronous case, ASP.NET knew the request thread and could abort it. In the asynchronous case, ASP.NET is not aware that the secondary thread pool thread is “for” that request. It is possible to fix this by using cancellation tokens, but that’s outside the scope of this blog post.

So, unless you're doing true async I/O, do it synchronously.

Alternatively, if you want to expose an async interface, where the implementation may or may not perform asynchronous I/O, synchronous implementation should use Task.FromResult instead of Task.Run. Example:

interface IRepository<T>
{
     public Task<T> GetByIdAsync(object id)
}

// implementation that uses synchronous I/O
class SyncRepository<T> : IRepository<T>
{
    public Task<T> GetByIdAsync(object id)
    {
        T t = this.Entities.Find(id);
        return Task.FromResult(t);
    }
}

// implementation that uses async I/O
class AsyncRepository<T> : IRepository<T>
{
    public Task<T> GetByIdAsync(object id)
    {
        return this.Entities.FindAsync(id);
    }
}

Read also:

• Stephen Toub's Should I expose synchronous wrappers for asynchronous methods?.
• Stephen Cleary's Task.Run Etiquette Examples: Don't Use Task.Run in the Implementation