HttpWebRequest.ReadWriteTimeout is ignored - reading never times out

[loop-evgeny]

.NET Core 2.1.7 on Windows 7 x64

var httpWebRequest = (HttpWebRequest)WebRequest.Create("http://mirror.zetup.net/ubuntu-cd/18.04.1/ubuntu-18.04.1-desktop-amd64.iso");
httpWebRequest.Timeout = 2000;
httpWebRequest.ReadWriteTimeout = 1;

var webResponse = httpWebRequest.GetResponse();
var responseStream = webResponse.GetResponseStream();

int totalBytes = 0, readBytes;
do
{
    var buffer = new byte[16384];
    readBytes = responseStream.Read(buffer, 0, buffer.Length);
    totalBytes += readBytes;
} while (readBytes != 0);

Based on the docs I would expect the responseStream.Read() call to time out, but it never does. responseStream.CanTimeout returns false - so at least it's honest!

In this case the entire file (1953349632 bytes) is eventually downloaded, but in a production case we have the server has not closed the connection for nearly a month now and the client (.NET Core 2.0) is still trying to read from it!

[davidkaya]

In https://github.com/dotnet/corefx/issues/20043 it was decided to make the property noop, so I think this issue is related to docs. I think the underlying streams have CanTimeout==false, so there would be required changes to those streams.

[davidsh]

In #21845 it was decided to make the property noop, so I think this issue is related to docs.

We currently document these variances between .NET Framework and .NET Core here:
https://github.com/dotnet/corefx/wiki/ApiCompat

We have a section for HttpWebRequest. But it looks like we need to update that to include a description for the ReadWriteTimeout property and that the property isn't implemented.

[loop-evgeny]

OK, I read the discussion in the linked issue:

Being silent (noop) might then lead to getting bugs raised since devs will think it should work

Err... yeah! It's hard for me to see how you reached the conclusion that a library silently ignoring what the user code explicitly tells it to do is better than refusing to do it (by throwing).

Anyway, regardless of how it's handled, the lack of any read timeout is a pretty severe limitation, to say the least. I can't see how a severe bug like this, that allows code to just get "stuck" forever, made it out of beta. I really hope to see this fixed properly and I fear that, if ReadWriteTimeout is documented to be a no-op, that will reduce the chances of it ever happening.

Is there a reasonable workaround for this? Is there an HTTP client class in .NET Core that does support timeouts, properly?

[karelz]

HttpWebRequest is obsolete APIs, we are slowly moving away from it. We recommend developers to use HttpClient. That should satisfy all your needs for timeouts. If not, please let us know.

[loop-evgeny]

Thanks, @karelz but it's unclear to me how the HttpClient.Timeout property applies to reading from the response stream and the documentation for that property doesn't mention it. Porting my code to HttpClient:

var httpClient = new HttpClient {Timeout = TimeSpan.FromMilliseconds(2000)};

var httpResponse = httpClient.GetAsync(url, HttpCompletionOption.ResponseHeadersRead).GetAwaiter().GetResult();
var responseStream = httpResponse.Content.ReadAsStreamAsync().GetAwaiter().GetResult();

int totalBytes = 0, readBytes;
do
{
    var buffer = new byte[16384];
    readBytes = responseStream.Read(buffer, 0, buffer.Length);
    totalBytes += readBytes;
} while (readBytes != 0);

If the server stops sending data for 2 seconds will responseStream.Read() throw an exception now? responseStream.CanTimeout still returns false.

[loop-evgeny]

Just tested it using a Kestrel server, configured like this:

            app.MapWhen(c => c.Request.Method == HttpMethods.Get && c.Request.Path == "/evgeny-test", ab =>
            {
                ab.Run(async context =>
                {
                    context.Response.ContentType = "text/plain";
                    await context.Response.WriteAsync("line1");
                    Thread.Sleep(60000);
                    await context.Response.WriteAsync("line2");
                });
            });

Reading from HttpClient's response stream does not time out. So this does not solve the original problem.

[karelz]

It seems thbat particular timeout is not available on HttpClient. Can you use the other timeouts instead?
What is your scenario where you need to set the ReadWriteTimeout - is it just about more fine grained control over detection of "dead" connections?

[hazzik]

Can you use the other timeouts instead?

What are the "other" timeouts? There are none.

[hazzik]

What is your scenario where you need to set the ReadWriteTimeout - is it just about more fine grained control over detection of "dead" connections?

We have an issue where we do a "chunked" connection (the code is similar to https://github.com/dotnet/corefx/issues/34550#issuecomment-454394111). When the network disconnects our HttpClient's hung on stream.Read and never leave the loop. We need a way to detect such a case, either with an exception or some property. Unfortunately the stream pretends to be alive an well when it is not.

[loop-evgeny]

What is your scenario where you need to set the ReadWriteTimeout - is it just about more fine grained control over detection of "dead" connections?

I'm not sure what you mean by "more-fine grained" control, since we currently seem to have no control at all.

The scenario is that we try to download a file from a server (which we do not control), the connection is established quickly enough, the server starts sending data, later stops sending data, but keeps the connection open. The Kestrel server code I posted demonstrates this nicely (just replace the Thread.Sleep with an infinite wait). We have such a case in production where the connection has been open since 13-Dec-2018. I would expect it to throw an exception after the specified timeout, so our application can catch that and retry later.

[Genbox]

Finally, we tackle this issue. I've been saying this for years as even .NET Core 1.1 had inconsistent timeout behavior between implementations of HTTP handlers (Curl / wininet), and I thought the new API along with SocketsHTTPHandler was going to be the end-all solution.

Anyway, what @loop-evgeny is saying is the same issue we face. Reading large chunks of data is a common operation and if there is a sudden disconnect (no RST/FIN packets in the TCP layer) then the read operation will hang.

The immediate solution would seem to be calling the SendAsync/GetAsync with a cancellation token, but unfortunately, that moves the handling of those timeouts to the consumer. To make matters worse, write operations are often NOT called by the producer but handled entirely by an underlying client. The Amazon AWS SDK S3 client is a good example where a connection will hang forever on a network disruption that does not result in a RST/FIN packet in the underlying network layer.

[davidsh]

Regarding the discussion here about streams having a "timeout" for read or write, the .NET Framework in general has a historical pattern for that where the abstract System.IO.Stream class has properties for ReadTimeout and WriteTimeout. It is necessary for derived Stream types to implement these properties.

However, these properties were designed in the days before async APIs. So, they were intended to be used by Stream.Read() and Stream.Write(). When async APIs arrived like Stream.ReadAsync() and Stream.WriteAsync(), I believe the intent was for "timeout" to be implemented against a CancellationToken that was passed into these methods and that these ReadTimeout and WriteTimeout properties no longer applied.

@stephentoub Can you comment on the expected design pattern here w.r.t. these properties and async APIs?

So, the comment here https://github.com/dotnet/corefx/issues/34550#issuecomment-454399130

Reading from HttpClient's response stream does not time out.

is not completely true. Reading from HttpClient's response stream can have a timeout implemented on it as long as the timer is attached to a cancellation token passed into the ReadAsync() method. When async pattern (await, Task etc.) was added to .NET, CancellationTokenSource, CancellationToken became the new pattern for "cancellation"/"timeout".

Example:

var client = new HttpClient();
Stream s = await client.GetStreamAsync("http://example.com/largefile");

var cts = new CancellationTokenSource();
cts.CancelAfter(1); // 1 millisecond timeout

int bytesRead;
var buffer = new byte[16384];
bytesRead = await s.ReadAsync(buffer, 0, buffer.Length, cts.Token);

[stephentoub]

Can you comment on the expected design pattern here w.r.t. these properties and async APIs?

For better or worse, these timeouts only ever applied to synchronous APIs, and are documented as such. For example, the Begin/EndRead/Write APIs on Stream that have been around forever ignored these (unless they were implemented in terms of the synchronous methods, in which case they would pick up the timeouts effectively by accident).

[hazzik]

We're using BinaryReader (because reasons) and so it's synchronous calls over the stream. So @davidsh your example does not really apply to us.

[davidsh]

Now tracking documentation updates with dotnet/dotnet-api-docs#3147

[coderb]

+1 this needs to be fixed.

Read()ing from a socket is something that can inherently hang indefinitely if the network decides to not send a RST/FIN packet which can and does happen.

PLEASE reconsider fixing this. It is NOT going to go away as an issue and will result in difficult to track down application bugs for who knows how many developers as they migrate from netfx to netcore.

As a start, why not just set the timeout on the underlying socket (which eventually is an ioctl() to the o/s). It's probably not a perfect solution, but far better than indefinite hangs!!!

"network timeouts are unsupported for synchronous networking apis" is not the proper response here.