[Bug]: Deadlock when blocking on async methods
Closed this issue · 4 comments
Software Version
v5.3.0
Language Version
.NET Framework 4.8, Version 10.0.22621 Build 22621
Operating System
Microsoft Windows 11 Pro
What happened?
Hi,
I have been upgrading our system from using version 2.8.2 of the EasyPost NuGet Package to the most recent version 5.3.0 so that we can have access to some newer EasyPost features. One of the major changes between those versions is that all of the methods that make API calls are now async. Our calling code is primarily synchronous. Usually when an SDK is async-focused and I need it to work synchronously I just access the Result property on the Task that is returned from the async method (or similarly, call GetAwaiter().GetResult()). Unfortunately, when I do that with the new EasyPost SDK, it deadlocks and never completes.
I understand the preferred approach is to fully embrace the async/await pattern, and I see related issues in this repo where using await is the solution to the issue. In my situation, I can't just use the await keyword because that would mean I have to make my encapsulating method async as well, which would just propagate the problem outward and affect parts of our system that aren't related to EasyPost at all. I suspect this isn't a problem specific to me, especially considering that this SDK was fully synchronous up until version 3 which is only 16 months old, so many existing customers are already using it synchronously.
I see usages ConfigureAwait(false) in some of the await calls in your source code, but also many await calls that do not have it. I suspect using ConfigureAwait(false) universally for await calls could prevent the deadlocking.
What was expected?
Calling .GetAwaiter().GetResult() or .Result on any async method in the SDK to block main synchronous control flow only until the async action completes.
Sample Code
public class TestController : System.Web.Mvc.Controller
{
// this action completes successfully
public async Task<ActionResult> TestEasyPostGetOrderAsync()
{
var order = await Client.Order.Retrieve("order_***");
var timestampUpdated = order.UpdatedAt.ToString();
return Content(timestampUpdated);
}
// this action deadlocks
public ActionResult TestEasyPostGetOrder()
{
var retrieveOrderTask = Client.Order.Retrieve("order_***");
// this line stalls indefinitely
var order = retrieveOrderTask.Result;
var timestampUpdated = order.UpdatedAt.ToString();
return Content(timestampUpdated);
}
// this action deadlocks
public ActionResult TestEasyPostGetOrder2()
{
var retrieveOrderTask = Client.Order.Retrieve("order_***");
// this line stalls indefinitely
var order = retrieveOrderTask.GetAwaiter().GetResult();
var timestampUpdated = order.UpdatedAt.ToString();
return Content(timestampUpdated);
}
}Relevant logs
No response
Thank you for getting in touch and reporting your findings. We will see what we can do in terms of making our asynchronous code work in a synchronous environment. We'll keep you updated!
@johnnyh-tactics Hey Johnny, I was trying to recreate your tests, but wasn't running into any issues in terms of sync/async incompatibilities.
I ran the following unit tests in all the .NET versions we support, including .NET Framework 4.6.2 (which I would imagine wouldn't have any breaking changes between it and the 4.8 you're targeting), and all of them passed.
Granted, I used "create/retrieve parcel" instead of "create/retrieve order", but that shouldn't make a difference in terms of the ConfigureAwait(false) you pointed out (that's downstream of both API calls).
Would you mind trying to run this and confirm? It might be an environmental thing rather than a code thing, which will help us narrow down where we need to look to potentially patch this.
Thanks in advance!
using EasyPost.Models.API;
using EasyPost.Parameters.Parcel;
using Xunit;
namespace EasyPost.Integration;
public class Synchronous
{
/// <summary>
/// Test that an end-user can run asynchronous code asynchronously
/// </summary>
[Fact]
public async void TestUserCanRunAsyncCodeAsynchronously()
{
// create a client
string? key = Environment.GetEnvironmentVariable("EASYPOST_TEST_API_KEY");
Assert.NotNull(key);
var client = new Client(new ClientConfiguration(key));
// create a parcel asynchronously
var parcel = await client.Parcel.Create(new Create
{
Height = 1,
Length = 1,
Width = 1,
Weight = 1,
});
Assert.NotNull(parcel);
Assert.IsType<Parcel>(parcel);
string parcelId = parcel.Id!;
// retrieve a parcel asynchronously
var retrievedParcel = await client.Parcel.Retrieve(parcelId);
Assert.NotNull(retrievedParcel);
Assert.IsType<Parcel>(retrievedParcel);
}
/// <summary>
/// Test that an end-user can run asynchronous code synchronously via .Result
/// </summary>
[Fact]
public void TestUserCanRunAsyncCodeSynchronouslyViaResult()
{
// create a client
string? key = Environment.GetEnvironmentVariable("EASYPOST_TEST_API_KEY");
Assert.NotNull(key);
var client = new Client(new ClientConfiguration(key));
// create a parcel via .Result
var parcel = client.Parcel.Create(new Create
{
Height = 1,
Length = 1,
Width = 1,
Weight = 1,
}).Result;
Assert.NotNull(parcel);
Assert.IsType<Parcel>(parcel);
string parcelId = parcel.Id!;
// retrieve a parcel via .Result
var retrievedParcel = client.Parcel.Retrieve(parcelId).Result;
Assert.NotNull(retrievedParcel);
Assert.IsType<Parcel>(retrievedParcel);
}
/// <summary>
/// Test that an end-user can run asynchronous code synchronously via .GetAwaiter().GetResult()
/// </summary>
[Fact]
public void TestUserCanRunAsyncCodeSynchronouslyViaGetAwaiter()
{
// create a client
string? key = Environment.GetEnvironmentVariable("EASYPOST_TEST_API_KEY");
Assert.NotNull(key);
var client = new Client(new ClientConfiguration(key));
// create a parcel via .Result
var parcel = client.Parcel.Create(new Create
{
Height = 1,
Length = 1,
Width = 1,
Weight = 1,
}).GetAwaiter().GetResult();
Assert.NotNull(parcel);
Assert.IsType<Parcel>(parcel);
string parcelId = parcel.Id!;
// retrieve a parcel via .Result
var retrievedParcel = client.Parcel.Retrieve(parcelId).GetAwaiter().GetResult();
Assert.NotNull(retrievedParcel);
Assert.IsType<Parcel>(retrievedParcel);
}
}@nwithan8 Thanks for the quick response! This is interesting, your tests do indeed work fine for me. I also tried the Order.Retrieve method in a xunit test and that also works fine. So the difference must be that the original examples I provided are running within a System.Web.Mvc.Controller action.
With that in mind, I did some more research, and it appears that even very simple versions of blocking on async methods from a Mvc.Controller action causes this same issue, as illustrated by this example I found https://gist.github.com/leonardochaia/98ce57bcee39c18d88682424a6ffe305. Also in that example is a function that runs the resulting Task synchronously. It's not elegant, but that solution works for me for running the EasyPost async methods synchronously in a Mvc.Controller action.
I think the difference between the EasyPost SDK and the other SDKs we use that work fine with .Result is that, even though those other SDKs return Task<>, they aren't declared as async - which I think means they aren't implemented with async/await under the hood. Appologies for my misunderstanding there, and for assuming that that meant the EasyPost SDK was buggy.
No problem, thanks for reporting this in the first place! And thank you for researching about async/await in Mvc.Controller; we'll point people to that gist you linked if anyone has the same issue in the future.
Glad it sounds like you were able to get the library working for your application again. Please let us know if there's anything else we can help with!