Re: /bzr/squid3/trunk/ r9220: Added Comm close handler for the data channel of FtpStateData

> ------------------------------------------------------------
> revno: 9220
> committer: Alex Rousskov <rousskov_at_measurement-factory.com>
> branch nick: trunk
> timestamp: Tue 2008-09-23 08:49:50 -0600
> message:
> Added Comm close handler for the data channel of FtpStateData
> transaction in preparation for officially dropping connect callbacks for
> closing descriptors.
>
> The data channel can be opened and closed a few times and the descriptor
> must be kept in sync with the close handler. I factored out the
> open/closing code into a simple FtpChannel class. That class is now used
> for both FTP control and data channels.
>
> The changes resolve one XXX discussion regarding FTP not having a close
> handler for the data channel. On the other hand, adding a second close
> handler attached to the same transaction is not a trivial change as the
> side-effects of Squid cleanup code are often illusive.
>
> For example, I suspect that FTP cleanup code does not close or even
> check the control channel. I added a DBG_IMPORTANT statement to test
> whether the control channel remains open. Or should that be an assert()?
>
> I think that only one out of the two callbacks can be dialed because the
> close handler executed first will invalidate the transaction object.

FTP data channel can open close any time. It's close handler needs to only
handle the fd, with no implications on the other FTP state.

The FTP general close handler should close the control channel with a
"QUIT\0" or "ABOR\0" (per RFC), and then close the data channel
(discarding anything in-transit on ABOR, reading to end of current in
buffer and closing on QUIT).

So the shutdown procedure for FTP should be calling the control channel
close handler and letting it handle the data channel cleanup. However,
this would not play nicely on shutdown right now. Just on regular
connection closes.

FTP still needs a re-work cleanup at some later date which can do this
sequence checking and fixup. Also to get rid of many global functions and
do translations of generated pages properly from templates.

Amos
Received on Wed Sep 24 2008 - 01:38:01 MDT