ERR_COMM_CLOSING and comm API changes

On Sat, 2008-04-19 at 11:52 +0200, Henrik Nordstrom wrote:

> I also think the COMM_ERR_CLOSING callback from comm
> should be dropped. Nearly all handlers looking for COMM_ERR_CLOSING do
> nothing else than return immediately to abort execution.. and the few
> exceptions there is can be dealt with by a close callback if at all
> needed:

FWIW, I have always wanted to get rid of COMM_ERR_CLOSING in Squid3 :-).
A close callback is sufficient to let the FD user know that there will
be no more callbacks for that FD. With some effort, we may be able to
pragmatically check that every FD user registers such a callback
(because they may otherwise get stuck waiting for I/O).

As far as I can see, we currently support multiple close callbacks in
comm_add_close_handler[*]. I assume we do need that support, but please
correct me if I am wrong. If somebody can give an example or two of
where support for multiple close callbacks is a good idea (and not just
a side effect of some poorly written code), I would appreciate it.

A perhaps bigger and more important question is whether we should:
  - Drop ERR_COMM_CLOSING but minimize comm API changes; or
  - Significantly improve comm API (uncluding droping ERR_COMM_CLOSING).

The former is a rather small change, with rather small benefits, but
with fewer short-term stability implications. The latter is the Right
Thing To Do, but I am not sure v3.1 is the right time for that.
Opinions?

Thank you,

Alex.
P.S.(*) The setNext() call in the current comm_add_close_handler is a
gross violation of AsyncCall API. I must have overlooked that piece and
will need to make sure it is fixed, possibly as the result of this
discussion.

> errorSendComplete, invalidates the err object
>
> ftpStateData::dataRead, sets a boolean that the read is done
>
> HttpStateData::handleRequestBodyProducerAborted injects a
> COMM_ERR_CLOSING
>
> HttpStateData::readReply sets a boolean that the read is done
>
> ServerStateData::sentRequestBody clears requestSender
>
>
> the other 20 or so users of comm callbacks just returns immediately on
> COMM_ERR_CLOSING, "aborting" the call..
Received on Tue Apr 22 2008 - 12:31:05 MDT