Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ConfigureAwait calls to async calls that are awaited #3002

Merged
merged 1 commit into from
Oct 9, 2024

Conversation

jar-stripe
Copy link
Contributor

Why?

A user reported an issue with Stripe.net v46 where it would deadlock when executing a synchronous Get call on one of the services. After investigating, it looks like this call passes through several async calls that are missing ConfigureAsync(false) calls. ConfigureAsync(false) instructs the .NET task to not continue in the context captured when it is created; this is important because if that context is blocked (i.e. because we are synchronously waiting for a response), it will deadlock.

This problem was observed and subsequently reproduced on Windows using .NET Framework, and affects services created without using StripeClient (StripeClient accessed services work as expected).

What?

  • added ConfigureAwait(false) calls to all async/await methods that did not have them
  • enabled CA2007 warning (as an error) for src/Stripe.net which will alert to any async calls that do not call ConfigureAwait

Verified this fix by running the following in a ASP.NET Framework app, inside the Home page request handler, and confirming that Get does return and the program does proceed:

var svc = new LocationService();
svc.Get("tml_...", null, new Stripe.RequestOptions { ApiKey = "sk_test_..." });

… calls don't cause a deadlock

added to .editorconfig to error if this situation happens again in the Stripe.net code (not in the tests)
@jar-stripe jar-stripe merged commit 1abee33 into master Oct 9, 2024
4 checks passed
@jar-stripe jar-stripe deleted the jar/fix-sync-calls-dotnet-framework branch October 9, 2024 18:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants