Open DavidPoulsenHGS opened 4 days ago
Xtansia
commented
3 days ago Hey @DavidPoulsenHGS,
Thanks for reporting this, this is definitely something we'd want to address. I think the currently in-dev next major version would provide a good opportunity to handle this and add the additional target framework for .NET 6.0+.
Is this something you'd be potentially interested in looking into and contributing? No pressure if not!
DavidPoulsenHGS
commented
3 days ago Perhaps, depending on the complexity. I'd like to give it a shot, at least.
How would I go about doing this? Do I just fork the 1.x branch and submit a pull request?
Xtansia
commented
2 days ago @DavidPoulsenHGS All changes should generally go into the main branch (and then potentially backported to 1.x). So you'd fork the repository, branch off of main, make your changes and then raise a pull request.
Even partial solutions can be helpful, so if you do feel stuck you can always raise a draft pull request and I can help give feedback or try to guide things in the right direction.
What is the bug?
The HttpConnection class contains the following code in the Request method:
responseMessage = client.SendAsync(requestMessage, HttpCompletionOption.ResponseHeadersRead).GetAwaiter().GetResult();and a little further into the same method:responseStream = responseMessage.Content.ReadAsStreamAsync().GetAwaiter().GetResult();These two lines of code are prone to deadlocks due to calling GetAwaiter().GetResult(). This link explains why.
A surprisingly similar issue has been discussed to great length in an ElasticSearch.Net issue as well.
How can one reproduce the bug?
It's difficult to reproduce. Essentially requires continuously calling the Request method from multiple threads to provoke a deadlock. It can be quite common under load in a production environment, however.
What is the expected behavior?
The code should be free of potential deadlocks. I suggest changing the implementation to use the synchronous "Send" method of the System.Net.HttpClient instead.