-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
#1724 Reverting back HttpClient full buffering #1853
#1724 Reverting back HttpClient full buffering #1853
Conversation
… be addressed in the new message invoker pool
@raman-m please, asap ;-) |
were not being disposed! |
I cannot get how does the property influence to disposing internal resources? |
I'm not going to approve till Raynald's report if he provides some tests in prod. @RaynaldM FYI We need your report and approve here. |
As soon as you decide to handle the stream yourself, then you are responsible of releasing it. It's logical no? |
freed willy |
I need Ray's input here... There is no problem to revert. The release is still active. This fix is a part of the Nov'23 milestone. |
I won't be able to validate these last changes in prod until January 3 or 4. |
@RaynaldM 🆗 |
Follows up #1724
After testing the new http message invoker pool on a test environment, we noticed that some resources were not being released. After further checks, we realised that some streams were not being closed correctly.
This doesn't appear in the application with the standard http client, but I made a mistake by adding the
HttpCompletionOption.ResponseHeadersRead
property when calling the SendAsync method, which could potentially cause the same problems as with the new message invoker pool.A revert should therefore be performed for the time being, and this problem will be addressed in the new message invoker pool.
🙈 🙉 🙊
Proposed Changes
HttpClientWrapper
keeping the full buffering for now. Usage of the new option is out of scope and hazardous!!!