Re: client_side and comm_close

On Mon, 2008-04-21 at 16:02 +1200, Amos Jeffries wrote:
> > On Sun, 2008-04-20 at 22:01 +0300, Tsantilas Christos wrote:
> >> Maybe it can be easier:
> >
> > The ease of implementation is a separate question. We still have to
> > agree what we are implementing, and the items below attempt to define
> > that. Ambiguity and lack of clear APIs is quite dangerous here (as the
> > related bugs illustrate!).
> >
> >> Just keeping the comm_close as is and in AsyncCallsQueue just
> >> cancels/not execute asyncCall's for which the fd_table[fd].flags.closing
> >> is set.
> >
> > Implementing #1 below can indeed be as simple as you sketch above. We
> > need to make sure that the final call that closes the FD and makes fd
> > value available to other connections is placed last (that may already be
> > true and is easy to fix if not).
>
> Not true.
>
> IMO, The proposed API _very_ first two lines of code in comm_close are to
> register a special Comm callback to perform the fclose() call, and then to
> immediately set fd_table flag closed for the rest of the comm_close
> process.

Agreed on the flag, disagreed on the call. The special/internal Comm
call (to self) should be scheduled last (after all close callbacks) and
not first because the close handler might need access to some FD-related
info. That info should be preserved until all close handlers have been
called.

> With that condition at the start we can guarantee that any registrations
> made during the close sequence are either non-FD-relevant or caught.

Yes, the flag is sufficient for that. The internal "close for good" call
can still be last.

> The special Comm callback is only special in that it is not required to
> check flags open before fclose()'ing the system-level FD, which will allow
> new connections to open on the FD.

It is special because it is calling an internal comm method not some
external close handler. Its profile and parameters are different. I
would not call it a callback because of that, but the label is not
important. It is not a close callback in terms of
comm_add_close_handler.

> Between the initial comm_close() call and the special Comm callback, we
> don't need to care if callbacks write their shutdown statements to the fd
> (its still technically open) but the closed flag prevents comm accepting
> any new delayed event registrations or reads.

Exactly. Our only problem is with code that calls comm with a stale fd,
after that fd has been really closed and a new connection was opened
with the same fd. That's not a new problem and we will solve it in v3.2
using comm handlers.

I hope the above puts us on the same page about the implementation
sketch for the comm_close API, but please yell if something still seems
broken.

Thank you,

Alex.
Received on Tue Apr 22 2008 - 13:48:10 MDT