Re: client_side and comm_close

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

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

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.

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.

> We also need to make sure that the
> user-level code does not access fd_table based on a possibly stale fd
> value. That would require more work, but probably still nothing major.
>
>> We only need a way to identify comm_close handlers and the asyncCall
>> which will do the actual close of the socket.....
>
> I am not sure what you mean, but the general-purpose AsyncCallQueue will
> not do these comm-specific checks. We already have custom comm callbacks
> and a general "Should This Call be Dialed?" interface. The callbacks
> will check the flags and will not dial if they should not. Please
> clarify if I am talking about a different problem here.
>
> Thank you,
>
> Alex.
>
>> Alex Rousskov 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).
>> >
>> > 2) All close callbacks registered before comm_close was called will be
>> > called asynchronously, sometime after comm_close exits.
>> >
>> > 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.
>> >
>> > 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[*].
>> >
>> > 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?
>> >
>> > [*] We can reliably detect #4 violations by replacing "fd" integers
>> > outside of comm with a unique integer ID or, better, a handler object
>> > containing metadata. That kind of change may be too big for v3.1
>> though.
>
>
>
Received on Tue Apr 22 2008 - 14:04:09 MDT