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?

OR

b) lookup the DNS at some far earlier point and store the IPA in ServiceRep for some sort of load balancing selection of just one by getConnection()?

Amos
--
Please be using
  Current Stable Squid 2.7.STABLE9 or 3.1.12
  Beta testers wanted for 3.2.0.8 and 3.1.12.2

Reply via email to