CODEWITHSUNDEEP
c#.netmultithreadingasync-awaitazureservicebus
HuntressMain
HuntressMain

Reputation: 197

Potential deadlock being caused by this async/await code?

I have the following class:

public abstract class ServiceBusQueueService : IServiceBusQueueService
{
    private readonly string _sbConnect;

    protected ServiceBusQueueService(string sbConnect)
    {
        _sbConnect = sbConnect;
    }

    public async Task EnqueueMessage(IntegrationEvent message)
    {
        var topicClient = new TopicClient(_sbConnect, message.Topic, RetryPolicy.Default);
        await topicClient.SendAsync(message.ToServiceBusMessage());
    }
}

Which is being used like this:

public ulong CreateBooking()
{
     // Other code omitted for brevity 
     ulong bookingId = 12345; // Pretend this id is generated sequentially on each call

      _bookingServiceBusQueueService.EnqueueMessage(new BookingCreatedIntegrationEvent
      {
            BookingId = bookingId
      }).GetAwaiter().GetResult();

      return bookingId;
}

When the EnqueueMessage method is called from my CreateBooking method the program hangs and does not go any further after hitting the line await topicClient.SendAsync(message.ToServiceBusMessage());

Now the code works and my messages get successfully pushed to the Service bus when I change the call to my EnqueueMessage method as follows:

Task.Run(() => _bookingServiceBusQueueService.EnqueueMessage(new BookingCreatedIntegrationEvent
        {
            BookingId = bookingId
        })).Wait();

I am not very familiar with using async/await, after a little bit of research it sounds like it was causing a deadlock, is this correct? Why does changing the method call to be Task.Run(() => SomeAsyncMethod()).Wait(); cause it to stop hanging and work as intended?

Upvotes: 0

Views: 393

Answers (2)

Eric Lippert
Eric Lippert

Reputation: 660425

Suppose I gave you the following workflow:

  1. write a note that says "mow the lawn" and put it on the fridge.
  2. do nothing whatsoever until the task mentioned in the note is complete.
  3. Make a sandwich
  4. Do the tasks written on notes on the fridge.

If you followed that workflow, you would get to step (2), and then do nothing forever, because you're waiting for step (1)'s task to complete, which is not started until step (4).

You're encoding effectively that same workflow in software, so it should be no wonder that you're waiting forever.

So why does adding a Run "fix" it? Here's another workflow:

  1. write a note that says "mow the lawn", hire a worker, and give that note to the worker
  2. do absolutely nothing until the task mentioned in the note is complete
  3. Make a sandwich

Now you do not wait forever. You wait for the worker to mow your lawn, which is merely inefficient and wasteful. You could be doing other work while you're waiting, like making that sandwich. Or you could mow the lawn yourself and not take on the expense of hiring a worker.

That's why you never, ever synchronously wait on an asynchronous operation. There are only two possibilities: if you are doing the asynchronous operation in the future, you wait forever, which is obviously broken. If you are not, then you're wasting time sleeping when you could be doing work, which is obviously wasteful.

Use asynchrony as it was designed to be used: asynchronously.

Upvotes: 10

Stuart
Stuart

Reputation: 5506

You are correct, this is an asynchronous deadlock you are seeing. The best thing to do is always await Task returning functions, so that your code is async top-to-bottom.

You could also avoid the deadlock here by using .ConfigureAwait(false) after the await inside of EnqueueMessage.

The reason Task.Run fixes it here, is that it causes the delegate inside to be ran with no current SynchronizationContext, it's this that is captured by the await (when no .ConfigureAwait(false) is used) and leads to the deadlock when you are blocking on the result.

Upvotes: 1

Related Questions