Re: async-calls squid3/src comm.cc,1.81.4.16,1.81.4.17

On Mon, 2008-01-07 at 00:51 +0100, Henrik Nordström wrote:
> sön 2008-01-06 klockan 12:27 +0200 skrev Tsantilas Christos:
> > It is not a cbdata problem.
>
> Are you sure?
>
> cbdata exists to help invalidate callbacks when they are no longer
> meaningful. A callback being made to something no longer meaningful is a
> clear sign of a missing cbdata barrier..
>
> > An example is in the HttpStateData::sendComplete method which is going
> > to execute an commSetTimeout(fd ...)
>
> The FD related state should no longer exist if the FD has been closed.
> That's what cbdata deals with.

Henrik,

    The problem Christos is addressing is somewhat different from a
non-meaningful callback problem: The FD-related state has
onclose-callbacks that must be called before the state is destroyed and
the FD is closed. If there were no onclose-callbacks, then we could
close FD immediately and cancel or invalidate all FD-related callbacks.
The onclose callbacks are like class destructors -- they must be called
before the object memory (i.e., the FD) is freed. We cannot just
invalidate or skip them (per comm API).

Whether onclose-callbacks are a bad idea is a separate discussion.
AsyncCall branch should not redesign them if at all possible, even if
there are reasons to redesign them. We just need to make them work in an
async-call environment.

> > With this patch squid now starts closing the fd , it mark it as
> > "closing" but actually schedules the real fd closing to a future, after
> > the comm_close handlers will be really executed.
>
> And why is this needed? The sendComplete callback should be invalidated
> if the HTTP connection state is no longer there.

The HTTP connections state is still there. Somebody told comm to close
the related FD. Comm must call onclose callbacks first. After that, comm
can close the FD. This is how the (old) comm close API is defined, I
think. What Christos is trying to do is to make that work with async
calls.

> Adding this reshedule adds a noticeable overhead an delay, plus makes
> tracing of the code flow more difficult.

We should be able to improve that by redesigning onclose API or comm
internals. I think that should be discussed after async calls are fully
working though. The fundamental AsyncCall design idea is that modules
like comm should not call other code directly. All such calls should be
async. I still think that design is the right one as it will help us to
make Squid3 robust and, eventually, SMP-scalable (with other lesser
advantages like better debugging and profiling).

Thank you,

Alex.
Received on Mon Jan 07 2008 - 10:54:19 MST