- From: Richard Atterer <richard@list03.atterer.net>
- Date: Wed, 19 Feb 2003 22:48:08 +0100
- To: www-lib@w3.org
Hi, in my continuing attempts to get libwww to resume FTP and HTTP downloads properly, I've come across another problem - libwww's behaviour if the connection to a HTTP 1.1 pipelining server suddenly drops. If the connection drops with FTP or HTTP 1.0 or non-pipelined HTTP 1.1, then the application gets back an error, which is the right thing. If the connection drops with HTTP 1.1 and a pipeline in place, libwww will "recover the pipeline" _exactly_once_ (#define MAX_HOST_RECOVER 1), by re-connecting to the server and re-adding all requests which were registered just before the connection dropped. This is stupid, for a number of reasons: - It is not consistent with the FTP/HTTP1.0/HTTP1.1-non-pipelined case. Chances are the application will not expect it. - Rather than making life easier for the app, it makes it more difficult, because the app *still* has to cater for the case that the connection drops again after the retry, in which case it needs to re-schedule the other requests of that pipeline etc. - There's no way to customize the number of retries. Why exactly once? There's also no way to prevent libwww from doing this. - Most importantly: The request that was active at the time the connection dropped is *restarted* (not resumed at the right byte offset!), and its data is sent to the output stream without further notice, i.e. the output stream will think that nothing has happened and that the bytes it has just been given are for one continuous download. The attached patch simply disables the "pipeline recovery" feature. A cleaner way would be to remove the recovery code altogether, since IMHO libwww is just trying to be too clever here - it should leave the error situation (dropped connection) for the application to deal with. [The alternative approach to fixing this would be still *more* intelligence: Make it resume the hitherto active request from the right byte offset. But I don't like this, because 1) the app should be notified of such an error, 2) the app will need to have code dealing with dropped connections anyway, so why reproduce it in libwww, 3) libwww has no way of knowing whether resuming is the right action; for example, the app may actually want to resume the download using a different, hopefully better mirror.] Apart from setting MAX_HOST_RECOVER=0, the patch also fixes an off-by-one error (>= instead of >), and it prevents that pipelining is switched off altogether with MAX_HOST_RECOVER==0 - with the current code, pipelining is switched off for the MAX_HOST_RECOVERth retry. Cheers, Richard -- __ _ |_) /| Richard Atterer | CS student at the Technische | GnuPG key: | \/¯| http://atterer.net | Universität München, Germany | 0x888354F7 ¯ '` ¯ --- HTHost.c.orig 2003-02-17 23:43:12.000000000 +0100 +++ HTHost.c 2003-02-18 00:58:12.000000000 +0100 @@ -31,7 +31,7 @@ #define TCP_IDLE_ACTIVE 60000L /* Active TTL in ms on an idle connection */ #define MAX_PIPES 50 /* maximum number of pipelined requests */ -#define MAX_HOST_RECOVER 1 /* Max number of auto recovery */ +#define MAX_HOST_RECOVER 0 /* Max number of auto recovery */ #define DEFAULT_DELAY 30 /* Default write flush delay in ms */ struct _HTInputStream { @@ -824,7 +824,7 @@ /* ** First check that we haven't already recovered more than we want */ - if (host->recovered > MAX_HOST_RECOVER) { + if (host->recovered >= MAX_HOST_RECOVER) { HTTRACE(CORE_TRACE, "Host recover %p already %d times - not doing it anymore\n" _ host _ host->recovered); return NO; } @@ -958,7 +958,7 @@ case HT_TP_SINGLE: return count <= 0; case HT_TP_PIPELINE: - return (host->recovered < MAX_HOST_RECOVER) ? + return (host->recovered < MAX_HOST_RECOVER || MAX_HOST_RECOVER==0) ? (count < MaxPipelinedRequests) : (count <= 0); case HT_TP_INTERLEAVE: return YES;
Received on Wednesday, 19 February 2003 16:54:06 UTC