Re: [squid-users] Re: [PATCH] Revised fix for download corruption

On Monday 04 November 2002 23.57, Phil Oester wrote:
> I've seen it, and I don't believe it is right. The meat of it:
>
> - if (storeAufsSomethingPending(sio)) {
> + if (FILE_MODE(sio->mode) == O_WRONLY &&
> storeAufsSomethingPending(sio)) {
>
> Why go through the motions of opening/reading/closing when we know
> we don't need to? It's a waste of cpu/spindle time to retrieve the
> file once we know the client is no longer listening. Mark it
> closed as soon as possible, and move on to something useful.

Exacly, and is why I did my patch where I did. If we see a
storeClose() for a object beeing read then immediately close the file
and abort any pending I/O operations. Only if the file is open for
writing and there is pending write operations in the queue delay the
close until the writes have finished.

The two patches are similar, but addresses the read problem at
different places in time:

  * Your patch address the problem upon completetion of a I/O event
some time after a storeClose() has been requested

  * My match address it immediately when storeClose() is called, not
waiting for the pending I/O event to complete. This also completely
aborts the I/O event if it hasn't started yet.

As said in the discussion leading up to the patch it is only a
temporary bandaid. The real problem is that aioRead() cannot verify
the validity of the buffer before copying the content into it. This
problem occurs due to a unneeded indirection layer in the aufs
implementation breaking a cbdata boundary and should be addressed,
but until then the simple patch should suffice.

And no, the patch addresses reads only, not writes like Robert
claimed..

Regards
Henrik
Received on Mon Nov 04 2002 - 16:40:00 MST