Re: /bzr/squid3/trunk/ r11499: Upgrade comm layer Connection handling

On 19/06/11 14:19, Amos Jeffries wrote:
> On 18/06/11 23:13, Tsantilas Christos wrote:
>> Hi all,
>> After last commits the ICAP client can not be used, the squid crashes
>> when trying to open a connection to the ICAP server.
>>
>> Looking in the code I found that the problem exist inside the
>> Adaptation::Icap::ServiceRep::getConnection method. The new code is not
>> exactly equivalent with the old code.
>>
>> The old code return always a fd to the user. In the case no connection
>> found in idle connections, it uses the comm_open to reserve an fd.
>>
>> The current code, if I am correct, in the case no connection found in
>> idle list it just uses a
>> connection = new Comm::Connection;
>> which just creates a Comm::Connection object with fd=-1. This is causes
>> a crash later in Xaction::openConnection.
>>
>> I believe some code is missing here, but I am not sure if it should be
>> added in comm/Connection.* (maybe a second constructor which allocates
>> an fd) or in ServiceRep::getConnection function.
>
> Awe heck forgot this one. Sorry.
>
>
> Comm no longer contains implicit DNS resolution. It needs to be given
> the remote IP address before opening.
>
> Since ServiceRep::getConnection() is strictly synchronous and assumes
> that there is only one IP for the service.
>
> Xaction::openConnection() can become async so now contains:
> "TODO: find the IPs and attempt each one if this is a named service"
> marks the spot.
>
> For now it converts IP-addressed services using raw-IP text conversion
> "remote= s.cfg().host.termedBuf()". Which is synchronous and
> non-blocking but not resolving FQDN.
>
> When FQDN resolution is required do we ...
>
> a) split openConnection() into two async steps and only use one of the
> possibly multiple IPs returned?

Anyone done anything towards fixing this up already? if not I'll pick it
up and implement (a) tomorrow.

Amos

-- 
Please be using
   Current Stable Squid 2.7.STABLE9 or 3.1.12
   Beta testers wanted for 3.2.0.9 and 3.1.12.3