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

Adrian Chadd wrote:
> .. I read that as "the code is broken and needs to be fixed."

I think not so broken because there are solutions or workarounds off the
problems.
I think the last changes fix the problem (however I want to look more on
this to be sure there are not any other problems....).

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

I fully agree with you here. The operations should not be done directly
on file descriptors. But the subject of async calls project is different.

> really, how the heck are you calling HttpStateData::SendComplete()
> with a file descriptor thats invalid?
>

The problem was that when the HttpStateData::SendComplete scheduled the
descriptor was valid. But a comm_close on this file descriptor could
make it invalid before the HttpStateData::SendComplete really executed.

> 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);
> }

I first saw the bug in tunnel.cc and I implement something like that you
are sugesting here. But then I found that the problem is not appeared
only in tunnel.cc code, but also in http.cc code and maybe in other
places ....
Received on Sun Jan 06 2008 - 04:52:47 MST