deruitda
deruitda

Reputation: 580

Can this "Synchronous" Code Cause Deadlocks?

My company has a Nuget Package they wrote that can do various common tasks easily for you. One of which is making HTTP requests. Normally I always make my HTTP requests asynchronous, however in this Nuget package is the following code:

    protected T GetRequest<T>(string requestUri)
    {
        // Call the async method within a task to run it synchronously
        return Task.Run(() => GetRequestAsync<T>(requestUri)).Result;
    }

Which calls this function:

    protected async Task<T> GetRequestAsync<T>(string requestUri)
    {
        // Set up the uri and the client
        string uri = ParseUri(requestUri);
        var client = ConfigureClient();

        // Call the web api
        var response = await client.GetAsync(uri);

        // Process the response
        return await ProcessResponse<T>(response);
    }

My question is, is this code really running synchronously by just wrapping the GetRequestAsync(requestUri) inside a Task.Run and calling .Result on the returned task? This seems like a deadlock waiting to happen, and we are seeing issues in areas of our app that utilize this function when running at higher loads.

Upvotes: 1

Views: 589

Answers (3)

Dogu Arslan
Dogu Arslan

Reputation: 3384

The reason that will not cause a deadlock is because Task.Run will push the delegate to be executed in a threadpool thread. Threadpool threads has no SynchronizationContext therefore no deadlock happens as there is no sync context to lock on between the async method GetRequestAsync and the caller. Same as you could have called .Result directly on the actual async method as well within Task.Run() block and that would not have caused a deadlock either.

Very inefficient though as you freeze 1 thread ie. 1 Core in the CPU do nothing but wait for the async method and I/O calls within it to complete. That s probably why you see a freeze in high load scenarios.

If you have a sync/async deadlock issue due to capturing sync context and blocking on async call, the deadlock will happen irrespective of load on a single call..

Upvotes: 2

Gleb
Gleb

Reputation: 1761

This won't cause a deadlock. But it's surely a resources wasting as one of the threads may be blocked.

The deadlock though may be possible if GetRequest looked like this:

protected T GetRequest<T>(string requestUri)
{
    var task = GetRequestAsync<T>(requestUri);
    return task.Result;

    // or
    // return GetRequestAsync<T>(requestUri).Result;
}

In example above you can see that I call GetRequestAsync within the current thread. Let's give the thread a number 0. Consider this line from the GetRequestAsync - await client.GetAsync(uri). .GetAsync is executed by a thread 1. After .GetAsync is done, default task scheduler is trying to return execution flow to the thread that executed the line - to the thread 0. But the thread that executed the line (0) is blocked right now as after we executed GetRequestAsync(), we are blocking it (thread 0) with task.Result. Hence our thread 0 remains blocked as it cannot proceed with execution of GetRequestAsync after await client.GetAsync(uri) is done nor it can give us the Result.

It's a pretty common mistake and I suppose you meant this one when asked about the deadlock. Your code is not causing it because you are executing GetRequestAsync from within another thread.

Upvotes: 1

Johnathan Barclay
Johnathan Barclay

Reputation: 20373

Accessing Task.Result will block the current thread until the Task is complete, so it is not asynchronous.

As for deadlocks, this shouldn't happen, as Task.Run uses another thread for GetRequestAsync, which is not being blocked by the call to Result.

Upvotes: 3

Related Questions