Alex Rousskov wrote:
On Mon, 2008-04-21 at 15:48 +1200, Amos Jeffries wrote:
comm_close(fd) API:
1) No I/O callbacks will be dialed after comm_close is called (including
the time when comm_close is running).
Sounds good.
2) All close callbacks registered before comm_close was called will be
called asynchronously, sometime after comm_close exits.
Sound good.
3) The code can ask Comm whether a FD associated with a close callback
has been closed. The answer will be "yes" after comm_close is called and
"no" before that. This interface needs to be added. Direct access to
fd_table[fd].flags will not work because the same FD could have been
assigned to another connection already. The code will have to use its
close callback to determine FD status.
Sounds good, BUT, direct access to fd_table pieces may need to be blocked
entirely (private:) so code is forced to go through the Comm API properly.
Yes, except if we want to avoid modifying old code that can still access
those flags directly because it gets immediate close callbacks (more on
that below).
(2) states that the higher-level close callbacks may be run at any time.
ie after the callback (3) refers to is run. This leaves a window open for
disaster, unless the closing callbacks are made immediate, and back we go
to recursion...
Those are the same close callbacks! There are no low-level and
high-level close callbacks here. The external code can use its stored
callback pointer to get FD status even after that close callback has
been called. There is no problem with that. The callback will not look
at fd_table in that case, it will just say "yes, the fd is closed as far
as you should be concerned".
And, per recent suggestions, old code will get immediate close callbacks
so it does not need to be modified to use the close callback pointer to
ask about FD status.
Alright. I got to this branch of the thread before the other which makes
things clearer. Same for the below.
4) Registering any callback after comm_close has been called is wrong.
Comm will assert if it can detect such late registrations. Not all late
registrations will be detected because the same FD can be already
assigned to a different (new) connection[*].
That non-detection seems to me to be a worry. The same problem in (3)
occurs here. (4) can guarantee that the closing callbacks don't play nasty
re-registrations. But only if they are called immediate instead of
scheduled.
Sorry, I do not understand. The "late" registration of a close callback
can come from anywhere. The old code relies on fd only. There is no way
for comm to distinguish whether the caller is using a FD from a closed
connection or a new one. This problem exists in the old code as well so
there is no new danger here! This problem can only be solved when we
have comm handlers of one sort or another.
The above comm_close API is easy to implement without massive code
changes. Do you think it is the right API? If not, what should be
changed?
Apart from the worry with immediate vs delayed closing callbacks.
To reduce that worry somewhat I think, the callbacks which actually use
ERR_COMM_CLOSING for anything other than immediate abort will need to be
replaced with two; a normal callback that checks the FD is open, and a
simple closing callback.
I am confused by the "normal callback that checks the FD is open" part.
What is that for? Are you talking about some internal comm calls to
close the FD? I believe that handler code that currently does not ignore
ERR_COMM_CLOSING notifications will need to be moved into the close
handler code (because only the close handler will be called if we
eliminate ERR_COMM_CLOSING).
Nevermind. I was saying it wrong, but meaning what you have re-stated.
Amos
--
Please use Squid 2.6.STABLE19 or 3.0.STABLE4
