-
Notifications
You must be signed in to change notification settings - Fork 5.4k
HttpClient Response Stream hangs #74568
Description
There have already been two similar requests #36822 and #11594.
In #36822 there is even an excellent repro #36822 (comment).
I discovered the issue in PowerShell:
- Start download large file
Invoke-WebRequest https://github.com/PowerShell/PowerShell/releases/download/v7.3.0-preview.7/PowerShell-7.3.0-preview.7-win-x64.zip -OutFile C:\temp\PowerShell-7.3.0-preview.7-win-x64.zip- Emulate network disaster by means of two options:
2.1 Close Wi-Fi connection on notebook
2.2 Disconnect Wi-Fi router from Internet - PowerShell cmdlet hangs on CopyToAsync()
- for option 2.1 until Wi-Fi recovered (if network was locked over ~1 min download is interrupted)
- for option 2.2 infinitly even after the Internet connection is restored
It is not a problem for interactive scenario since user can press Ctrl-C but it is an unpleasant problem for script and (PowerShell SDK custom) host scenarios.
Since it does not leave a trace in the logs and is eliminated by rebooting, the technical support concludes that it is a hardware problem, which is a false path.
PowerShell user also does not get a message that the file is not fully downloaded.
Note that it is the web client that we are considering. Why is it important to have a timeout?
The infinite timeout of sockets is a fundamental constant and this will never be changed. This is useful for low-level connections which can re-establish the connection without any extra effort.
But how do web services behave? In fact they close the connection if the client doesn't send requests for some time so as not to waste resources. This timeout is quite short.
But if any web server closes a session by a short timeout, then it doesn't make sense for any web client to have an infinite timeout on socket level - reconnection on the socket level doesn't work. On the contrary, the client must receive a timeout signal to either end the session on his side or try to resume it at a higher level.
This makes us think that any web client should have a default timeout greater than the typical timeout of any web server. If there is any doubt that there should be a default timeout, then at least the developer should be able to set it.
In which APIs is it better to add this timeout? I don't have the exact answer.
Obviously the timeout must be in SocketsHttpHandler.
But this is not enough since this class is sealed. Thus PowerShell historically uses HttpClientHandler which is a SocketsHttpHandler wrapper. We could migrate PowerShell to SocketsHttpHandler but HttpClientHandler uses some internal helper classes to map to SocketsHttpHandler. So perhaps it makes sense to add ReadWriteSocketTimeout to HttpClientHandler too (or make the helper classes public).
Note that the WebRequest API supports this timeout (although it doesn't seem to be public either).
runtime/src/libraries/System.Net.Requests/src/System/Net/HttpWebRequest.cs
Lines 1666 to 1702 in e71a958
| // Set up a ConnectCallback so that we can control Socket-specific settings, like ReadWriteTimeout => socket.Send/ReceiveTimeout. | |
| handler.ConnectCallback = async (context, cancellationToken) => | |
| { | |
| var socket = new Socket(SocketType.Stream, ProtocolType.Tcp); | |
| try | |
| { | |
| socket.NoDelay = true; | |
| if (parameters.Async) | |
| { | |
| await socket.ConnectAsync(context.DnsEndPoint, cancellationToken).ConfigureAwait(false); | |
| } | |
| else | |
| { | |
| using (cancellationToken.UnsafeRegister(s => ((Socket)s!).Dispose(), socket)) | |
| { | |
| socket.Connect(context.DnsEndPoint); | |
| } | |
| // Throw in case cancellation caused the socket to be disposed after the Connect completed | |
| cancellationToken.ThrowIfCancellationRequested(); | |
| } | |
| if (parameters.ReadWriteTimeout > 0) // default is 5 minutes, so this is generally going to be true | |
| { | |
| socket.SendTimeout = socket.ReceiveTimeout = parameters.ReadWriteTimeout; | |
| } | |
| } | |
| catch | |
| { | |
| socket.Dispose(); | |
| throw; | |
| } | |
| return new NetworkStream(socket, ownsSocket: true); | |
| }; |
This uses a callback, which is not convenient. In addition, it duplicates the standard code from the SocketsHttpHandler, which can be changed in the future but will not be automatically inherited by the application.