Re: ERR_COMM_CLOSING and comm API changes

lör 2008-04-19 klockan 22:14 -0600 skrev Alex Rousskov:

> 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.

Yes.

> 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).

Should be quite easy to audit the code for, so not sure the code as such
needs to enforce it.

> As far as I can see, we currently support multiple close callbacks in
> comm_add_close_handler[*].

Yes. And it's used quite heavily, but quite likely not for the right
reasons.. One example is that both FwdState and the protocols register a
close handler to handle aborts & retrials.

It SHOULD be sufficient with one, based on the assumption that there
should only be exactly one concurrent user of the same fd...

There is also another user and it's pinned connections where client_side
keeps a close callback on the outgoing filedescriptor.

> 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?

Dropping ERR_COMM_CLOSING is a start. It looks like it may not be needed
in the current code (if it ever really was needed, but maybe it was once
in a time due to buffer management badness...)

Regards
Henrik
Received on Tue Apr 22 2008 - 13:03:08 MDT