Repository navigation
HttpWebRequest.GetRequestStreamAsync doesn't open the connection to the server - we always buffer the content #18632
Description
The corefx implementation of HttpWebRequest.GetRequestStreamAsync effectively just returns an in-memory stream. The connection to the server isn't made and the data isn't uploaded until Stream.Dispose is called on the that Stream. This is problematic for several reasons, including that the actual asynchronous operation ends up happening in the synchronous Dispose call on the stream, and deadlocks can easily happen if code assumes that the server will respond once the supplied data has been written (but before Dispose is called).
Activity
davidsh commentedon Sep 19, 2016
This is actually the same design as .NET Framework (Desktop). The only difference is that in Desktop, there is a property on HttpWebRequest (.AllowWriteStreamBuffering) which can toggle this behavior.
Eventually, when we add all the API surface back, we will have to solve it then. The difficulty at this point, though, is that the HttpWebRequest API surface is built on top of HttpClient and it has a different data model for handling when data actually starts going to a server.
davidsh commentedon Sep 19, 2016
I am removing the 'bug' label because it isn't a bug, per se. It is the proper behavior if the .AllowWriteStreamBuffering option is TRUE, which it always is right now in the current implementation.
stephentoub commentedon Sep 19, 2016
Ok. Thanks, David.
Im5tu commentedon Sep 19, 2016
Will you actually come around to fixing these sorts of things though considering how it could break applications? I'm guessing I'm saying why not fix it now?
karelz commentedon Mar 2, 2017
We should evaluate it for 2.0 based on usage data of
AllowWriteStreamBuffering.karelz commentedon Mar 3, 2017
@weshaggard can you please help us evaluate usage of
AllowWriteStreamBuffering?karelz commentedon Mar 3, 2017
Usage data is on apisof.net (thanks @stephentoub for pointing it out): https://apisof.net/catalog/System.Net.HttpWebRequest.AllowWriteStreamBuffering
That seems above the threshold we went after in .NET Standard 2.0 - @danmosemsft, @weshaggard what do you think?
karelz commentedon Mar 3, 2017
Our threshold was around 0.1%, so this is clearly in from that point of view.
@davidsh @CIPop - can you guys please cost it roughly? (you said it is complex/costly), we will have to roll it up to our management chains for 2.0
weshaggard commentedon Mar 3, 2017
@karelz I'm not sure I understand the question. AllowWriteStreamBuffering is part of .NET Standard 2.0 so the API has been added back, is the question whether or not we should fix the implementation to do the buffering correctly again? If so I would yes we probably should in order to be compatible as that flag is set in the default set of flags we use (https://github.com/dotnet/corefx/blob/ec2a6190efa743ab600317f44d757433e44e859b/src/System.Net.Requests/src/System/Net/HttpWebRequest.cs#L84).
karelz commentedon Mar 3, 2017
@weshaggard the buffering is hidden under
AllowWriteStreamBuffering = true. It isnot(EDITED) the default.We decided to leave it in 2.0 for now and figure out the cost.
karelz commentedon Mar 3, 2017
I appologize (@weshaggard corrected me) - the flag is set in Default on .NET Core.
@davidsh @CIPop does Desktop have the same Default value with the flag set?
When the flag set, what does the buffering mean? Is the in-memory stream considered the buffer? (on Desktop and on .NET Core)
davidsh commentedon Mar 4, 2017
The default value is TRUE (which works). And yes, "buffering" means that the data is held in a MemoryStream. The dev gets the request stream and writes to it. But those writes are not sent on the wire but fill up a MemoryStream. Later, when the request is sent to the HTTP server, the MemoryStream is replayed (usually by grabbing the stored buffer) and sent on the wire.
The Desktop and .NET Core both use a MemoryStream for the buffering.
karelz commentedon Mar 4, 2017
Thanks!
@weshaggard I noticed that you looked at (private) enum value instead of on the property with the same name
AllowWriteStreamBufferingI think our usage analysis above is valid. Assuming people will (mostly) use the property only to set it to non-default value, which is
false1 remaining item
vitek-karas commentedon Apr 20, 2017
Per the code at https://github.com/dotnet/corefx/blob/master/src/System.Net.Requests/src/System/Net/HttpWebRequest.cs#L137
the value of the AllowReadStream property is now always false... not sure if that is intentional, but the above discussion says otherwise (that we always buffer and thus the property should always be true).
davidsh commentedon Apr 20, 2017
Actually, I should not have linked these issues. The new issue you entered dotnet/corefx#18668 has nothing to do with this issue. Let's continue discussion on that issue instead.
karelz commentedon Apr 26, 2017
Triage discussion: While important, this is P2 at this moment and won't make it into 2.0, unless we get additional feedback from customers.
jokker87 commentedon Mar 7, 2019
Do i get this right, even when I set AllowWriteStreamBuffering to false, in .net core, when i call GetRequestStream, the stream always gets buffered in a MemoryStream till the stream is closed? So we have a memory issue when uploading big files to an endpoint?
davidsh commentedon Mar 7, 2019
Yes. We recommend that you use HttpClient APIs and not the legacy HttpWebRequest APIs. HttpClient has APIs that can support the similar scenario where uploading the request body from a large source (i.e. filestream) will not be completely buffered into memory first. Instead, it will read directly from the filestream and write directly to the network.
karelz commentedon Oct 2, 2019
Triage: Related to @alnikola's effort in HttpWebRequest. This may be non-trivial.
Preparation for migration back to .Net Core/.Net 5 ->Replaced HttpWeb…
liveans commentedon Apr 8, 2024
Fixed by #95001