Re: [PATCH] Move timeout handling from ConnOpener::connect to ConnOpener::timeout

On 01/17/2013 10:28 PM, Amos Jeffries wrote:
> On 18/01/2013 5:37 p.m., Alex Rousskov wrote:
>> On 01/17/2013 06:35 PM, Amos Jeffries wrote:
>>>> Amos, do you remember what "partially open FD" in the following comment
>>>> meant, exactly? Why cannot we just call comm_close() to cleanup?
>>> It means we have issued socket() and connect() OS calls. fd_table[]
>>> state probably exists. But we are still waiting on either TCP response,
>>> or the In-Progress event to run and the FD to be confirmed as open.
>>> This is signalled by (temporaryFd_ >= 0).
>> Why cannot we just call comm_close() to cleanup Comm state?
>
> Because comm_openex() and comm_close() are not "symmetrical" in what
> they do. comm_close() involves a lot of handlers cleanup and close code
> for state objects which simply do not exist yet.

Why non-existing handlers and objects are a problem?

We have a write handler and a close handler registered with Comm, right?
Why not call comm_close() to clean them up (from Comm point of view) and
close the descriptor? We do not need those handlers to be actually
called (we may even cancel them before calling comm_close). We just need
to clear Comm state and close the descriptor.

Will comm_close() break if we call it instead of reaching into Comm guts
and manipulating its tables? If yes, what is Comm missing? Perhaps we
can give that missing piece to Comm so that we can comm_close()?

> The FD has only had socket() and connect() done on it and a few specific
> things done to the fd_table. One of which is registering the
> calls_.earlyAbort_ handler as what gets run on comm_close() in case the
> Squid shutdown/restart sequence or TCP close happens during this Jobs
> operations. If we use comm_close() most of the required cleanup will
> never happen.

Please note that I am _not_ suggesting to call comm_close to clean up
our job state (like a lot of Squid code does) even though I believe it
would be possible if doneAll() is adjusted accordingly.

I am just trying to find a way to call comm_close() to clean up Comm
state without violating Comm APIs. We will clean the job's state in
addition to calling comm_close().

Thank you,

Alex.
Received on Fri Jan 18 2013 - 05:47:27 MST