1

The code works fine on my development environment, but in deployment with scallable architecture it appears to deadlock.

Objective here is to take a queue of API requests to send to SendGrid, batch them up and process each batch one at a time.

First call from ASHX handler 

public void ProcessRequest(HttpContext context)
{
    var result = Code.Helpers.Email.Sendgrid.Queue.Process().Result;
    if (result.Success)
    {

Queue.Process()

public static async Task<GenericMethodResult> Process()
{
    var queueItems = GetQueueItemsToProcess();
    var batches = BatchQueueItems(queueItems);

    foreach (var batch in batches)
    {
        var r = await batch.SendToSendGrid();
        if (r.StopBatch)
        {
            break;
        }
    }

    return new GenericMethodResult(true);
}

SendToSendGrid()

public async Task<SendGridAPIMethodResponse> SendToSendGrid()
{
    var r = new SendGridAPIMethodResponse();
    var json = API.Functions.CreateJSONData(this);
    var sg = new SendGridClient(Settings.Email.SendgridAPIKey);

    dynamic response;
    if (Action == Action.UpdateRecipient)
    {
        response = await sg.RequestAsync(SendGridClient.Method.PATCH, urlPath: "contactdb/recipients", requestBody: json);
    }

    string jsonResponse = response.Body.ReadAsStringAsync().Result;
    // Process response...

    return r;
}

I've stripped out as much of the code as I could.

Is anyone able to tell me why this code is timing out in production?

Tom Gullen
  • 61,249
  • 84
  • 283
  • 456
  • 1
    You appear to be sending an email directly from a handler. While this will work, it might be better to send the email on a [background job](https://www.hanselman.com/blog/HowToRunBackgroundTasksInASPNET.aspx). What if the email server (or web server in this case) is slow? Or what if it's temporarily down? Probably better to queue the emails up and have the ability to retry sending etc. – mason Apr 03 '17 at 13:47
  • @Mason it's not sending email, is sychronising contact data with their contact API. It has sensible rate limits in it so it only does a max of ~50 or so each minute. It is locked with a distributed lock, so if called again it will simply return without doing anything if busy. If it fails, it doesn't remove items from the queue and will try again later. It is setup as an Azure scheduled task, calling this URL each minute. It's hard to decouple this from the website itself due to caching. – Tom Gullen Apr 03 '17 at 13:56
  • It's still an outside service call. Up to you. I'd just hate for users of my application to have a slow/bad experience just because some other 3rd party service was slow/unavailable. – mason Apr 03 '17 at 13:58
  • @Mason how would it slow the experience down for users? It's a fairly simple queue processor. The delay introduced by the queue is invisible to the end user. – Tom Gullen Apr 03 '17 at 13:58
  • Ah, I see now that you've updated your comment that it's called by a scheduled task rather than via AJAX as a result of user interaction. Okay, so you're right it wouldn't slow a user down in this case. But then that brings up the question, why expose the functionality via a handler in an ASP.NET site? Why not have the scheduled task run the code directly or use something more suited to the job in Azure? – mason Apr 03 '17 at 14:00
  • @Mason would love to, but decoupling it from the webapp itself is technically tricky as is manipulates caching/db records. Long term yes I should move it into it's own app but for now it works and that's good enough :) – Tom Gullen Apr 03 '17 at 14:03

1 Answers1

2

This blocking call to .Result in SendToSendGrid() is causing a deadlock as you are mixing async and blocking calls.

string jsonResponse = response.Body.ReadAsStringAsync().Result;

Use async all the way through

var jsonResponse = await response.Body.ReadAsStringAsync();

and try to avoid mixing blocking calls in async methods.

You should also conside making your handler async as well by using HttpTaskAsyncHandler.

public class MyHandler : HttpTaskAsyncHandler {
    public override async Task ProcessRequestAsync(HttpContext context) {
        var result = await Code.Helpers.Email.Sendgrid.Queue.Process();
        if (result.Success) {
            //..other code
        }
    }
}
Nkosi
  • 235,767
  • 35
  • 427
  • 472
  • That's excellent and has helped a ton. I can see that `SendToSendGrid()` is finishing executing, but it appears there's still a deadlock somewhere. Is there anything else in the chain that could be causing this? Not sure if it's important, but the `SendToSendGrid()` function needs access to the current HTTPContext – Tom Gullen Apr 03 '17 at 13:03
  • Does that include the ASHX handler itself? I'm pretty confused why this is blocking, all I want the ASHX handler to do is run this function. If I could avoid async I would, but sendgrid api doesn't appear to have any synchronous functions. – Tom Gullen Apr 03 '17 at 13:16
  • Thanks for that, this all seems rather tricky to implement. Is there an easier way to do this? All I'm trying to do is simply send a request to SendGrid's API, and process the response. It's in a handler that is called as a job every 60 seconds. – Tom Gullen Apr 03 '17 at 13:25
  • Thanks, have updated answer to mention it's from ASHX – Tom Gullen Apr 03 '17 at 13:39
  • 1
    Check [Answer by Stephen Cleary](http://stackoverflow.com/a/13140963/1559611), why calls like `Wait`, `Result` leads to hang in a Async call, well documented behavior. Since ASP.Net entry point can be Async, so Await is the best option, its different for console applications though, where main entry point is not Async – Mrinal Kamboj Apr 03 '17 at 18:09