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

Deadlock in async code #52

Closed
atorresGit opened this issue Oct 7, 2016 · 5 comments
Closed

Deadlock in async code #52

atorresGit opened this issue Oct 7, 2016 · 5 comments
Assignees

Comments

@atorresGit
Copy link

In BaseRequest class, SendAsync method, around line 132, when the response its being read, should be calling configureAwait(false).
We are using the library in a website with legacy sync code and we are having a deadlock because of this. Please can you put a tag in the code, signaling version 1.0.1, so we can fork that code and fix this in our solution. We can't use the latest version, because we need to keep using json.net 6.0.2.
I tried to create a pull request, but I don't have permissions.
Thanks!

@MIchaelMainer
Copy link
Contributor

Thank you for bringing this to our attention. I'll take a look at this. By the way, you should be able to update to the latest Nuget package. Nuget's default behavior is to use the earliest version of dependencies. Since you already have 6.0.2 installed, I expect that Nuget will keep that dependency if you update. Let us know if it behaves any differently.

@MIchaelMainer MIchaelMainer self-assigned this Oct 12, 2016
@MIchaelMainer
Copy link
Contributor

Under what circumstances did you encounter the deadlock? What call were you making?

@atorresGit
Copy link
Author

This is not the real code, but will give you an idea on how its being used:
`public void ProcessProfile()
{
...
var user = GetCurrentUser().Result; // change ProcessProfile() to async will imply a big refactor
DoThingsWithUser(user);
...
}

public async Task GetCurrentUser()
{
...
var me = await graphServiceClient.Me
.Request()
.GetAsync()
.ConfigureAwait(false); // here we have the deadlock...
...
return me;
}`

All this is used in a controller of a MVC3 website (ASP.NET synchronizationContext)

@MIchaelMainer
Copy link
Contributor

MIchaelMainer commented Oct 13, 2016

@atorresGit What exactly is the change that you'd like to get in BaseRequest.cs? I see the missing ConfigureAwait, but what else do you want to add in there?

I believe that commit d41633daf8710e51bb320c3dd74e34b80dc9de06 correlates to version 1.0.1. Does this make sense to you as a watermark to tag?

MIchaelMainer added a commit that referenced this issue Oct 17, 2016
@MIchaelMainer
Copy link
Contributor

Fixed with 837cd1f

@ghost ghost locked as resolved and limited conversation to collaborators Feb 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants