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

.. I read that as "the code is broken and needs to be fixed."

Ideally we wouldn't have this mess. I've never liked the close
handlers in the first place; the code shouldn't be using fd's
but should be using some opaque "connector" type which has
a bunch of methods, including send/receive/timeout/etc; and
really, how the heck are you calling HttpStateData::SendComplete()
with a file descriptor thats invalid?

The tunnel code could be fixed by doing something like:

close_server()
{
        server_fd = -1; /* We can't use it anymore */
        if (client_fd > -1)
                comm_close(client_fd);
}

close_client()
{
        client_fd = -1; /* we can't use it anymore */
        if (server_fd > -1)
                comm_close(server_fd);
}

(Not trying to poopoo the whole async calls idea, I'm just trying to point
out that the problems you're trying to solve shouldn't be problems in the
first place.)

Adrian

On Sun, Jan 06, 2008, Tsantilas Christos wrote:
> Hi Henrik,
> It is not a cbdata problem.
>
> An example is in the HttpStateData::sendComplete method which is going
> to execute an commSetTimeout(fd ...)
>
> Now there are cases in which this method scheduled for execution
> sometime in the future but before it is executed the squid code closes
> the fd using comm_close. In this case the fd is not valid any more and
> when the HttpStateData::sendComplete executes the commSetTimeout an
> assertion will crash squid.
> Moreover I believe that it is possible the squid reuse the fd and the
> commSetTimeout will be executed on a wrong fd.
>
> Also I had a similar problem in tunnel.cc with the tunnelClientClosed
> and tunnelServerClosed comm_close handlers, where the comm_close handler
> of server closes the client socket and opposite.
>
> 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.
>
> It is not look the best method to do not release/close immediately a not
> needed filedescriptor but I did not found a better way to handle these
> problems. Do you see any problem here?
>
> Regards,
> Christos
>
>
> Henrik Nordstrom wrote:
> > On l??r, 2008-01-05 at 21:25 +0000, chtsanti wrote:
> >
> >> When a socket is closed then it is possible that remains AsyncCalls to be
> >> executed which will try to perform operation on the closed socket.
> >> This patch does not close the socket when comm_close called but instead
> >> schedules the socket closing as an AsyncCall operation
> >
> > Smells like there may be a cbdata barrier missing somewhere...
> >
> > Regards
> > Henrik

-- 
- Xenion - http://www.xenion.com.au/ - VPS Hosting - Commercial Squid Support -
- $25/pm entry-level VPSes w/ capped bandwidth charges available in WA -