On Wed, 24 Sep 2008, Dan Fandrich wrote:

>> It seems that something very similar happened before this patch, but for
>> one critical read the error was supressed. In that case, curl did detect
>> the situation immediately afterward and retry the request, saying
>> "Connection died, trying a fresh connect".
>
> This is going to be tricky to solve. How can you tell the difference between
> a race condition (between sending a new request and the remote server
> closing the connection) and the server deliberately or accidentally (but
> illegally) closing the connection AFTER and BECAUSE OF the request? In the
> latter case, retrying the request will fail again and potentially result in
> an infinite loop.

I don't think it is possible, and it wasn't really possible before either so
this isn't truly a new problem. The only sane approach I can think of would be
to only retry the request a limited number of times, quite possibly only once.

But I think this mostly proves that the existing "retry request on a
presumably dead connection" code was a band-aid fix needed because we didn't
do proper error checking of the recv() call before and I think that we should
primarily assume that the new way of actually respecting the errors is a
better way forward.

> The following patch makes all my tests pass again, but it's only marginally
> better than my last patch. It still has the potential of missing a genuine
> error and retrying when it shouldn't. Maybe Curl_retry_request should be
> changed to only attempt to retry a request once to avoid a possible infinite
> loop.

It always had that risk, but with such a limit I think the harm of retrying
will be kept to a minimum.

-- 
  / daniel.haxx.se