Sendgrid-csharp: Make ConfigureAwait(false) configurable

Created on 14 May 2017  路  11Comments  路  Source: sendgrid/sendgrid-csharp

The default will be false.

Update examples (README, USE_CASE, USAGE, Example Projects) to demonstrate usage.

help wanted community enhancement

Most helpful comment

This change would be a bit surprising. Library code should almost almost be calling ConfigureAwait(false), otherwise the consumers of the library might face deadlocks if they call your asynchronous methods in a blocking fashion (e.g Task.Result or Task.Wait()).

As long the user code doesn't specify ConfigureAwait(false) then the SynchronizationContext will flow correctly in those methods (and if they do use ConfigureAwait(false) there's nothing the library can do about).

All 11 comments

The main reason to call ConfigureAwait(true) instead would be for ASP.NET web applications where the SynchronizationContext carries important data like System.Web.HttpContext.Current, which will become null after an await with ConfigureAwait(false). This leads to very hard to debug null reference exceptions..

Thanks @Korijn!

This change would be a bit surprising. Library code should almost almost be calling ConfigureAwait(false), otherwise the consumers of the library might face deadlocks if they call your asynchronous methods in a blocking fashion (e.g Task.Result or Task.Wait()).

As long the user code doesn't specify ConfigureAwait(false) then the SynchronizationContext will flow correctly in those methods (and if they do use ConfigureAwait(false) there's nothing the library can do about).

Of everything I've read it always says to use .ConfigureAwait(false) when in library code, there's never been mention of using true or not calling .ConfigureAwait() at all. All of the projects I've seen move to async have had to add this in order to fix these types of bug reports.

I've looked through the code a few times and it's really basic, there's one spot that's not using async which should be (it's not being called in these reports though), but other than that it doesn't seem like these issues would be coming from this library but rather from the way it's being used.

I'd expect more reports of not being able to send email if this was the real cause of the deadlocks. In my projects I'm using it with NancyFX as OWIN middleware with the SystemWeb host and there's been no issues at all. I also have a similar library I wrote that uses HttpClient with .ConfigureAwait(false) on all the async calls and there's been no issues with that either.

All I know is I could trigger System.Web.HttpContext.Current becoming null by stepping over the SendMail (which indirectly calls ConfigureAwait(false)) call in the SendGrid client package. I had to roll my own client to resolve the issue. I don't call ConfigureAwait anywhere else (at all, true or false) in my ASP.NET application and use tons of other libraries.

@Korijn can you paste a bit of code where you're calling the problematic SendMail please.

dynamic response = await Client.client.mail.send.post(requestBody: data);

Can you make sure you have httpRuntime.targetFramework set to 4.5 (or higher) or that you have <add key="aspnet:UseTaskFriendlySynchronizationContext" value="true" /> in your appSettings (if you're not doing either of these, using async/await has undefined behaviour)

There's more infomation https://blogs.msdn.microsoft.com/webdev/2012/11/19/all-about-httpruntime-targetframework/ (and some context http://stackoverflow.com/a/18384253/1367)

Yeah, I got that set (and I've read the blog entries):

<httpRuntime maxRequestLength="52428800" executionTimeout="300" relaxedUrlToFileSystemMapping="true" targetFramework="4.5.2" enableVersionHeader="false" />

Hi @Korijn,

Based on this code dynamic response = await Client.client.mail.send.post(requestBody: data); it appears you are using a old version of this SDK. Can you update to the latest version?

@WilkaH, @xt0rted,

Thanks for being awesome and offering some help.

Also, @xt0rted, thanks for the heads up on that call that is missing the ConfigureAwait. I've opened up a bug for that here.

Was this page helpful?
0 / 5 - 0 ratings