Re: client_side and comm_close

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.

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

Thanks,

Alex.
Received on Tue Apr 22 2008 - 13:23:42 MDT