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

On 24/06/11 21:18, Tsantilas Christos wrote:
> On 06/24/2011 08:35 AM, Amos Jeffries wrote:
>> 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.
>
> I didn't do anything on this, I am working on Alex's project.
> If you start working on this please inform me.
>

Okay. In the interests of getting it closed sooner I'll start on this
now. Will pass the results back here for review.

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