On 06.09.2012 02:30, Alex Rousskov wrote:
On 09/05/2012 03:32 AM, Alexander Komyagin wrote:
On Tue, 2012-09-04 at 09:16 -0600, Alex Rousskov wrote:
Again, I hope that this trick is not needed to solve your problem,
and I
am worried that it will cause more/different problems elsewhere. I
would
recommend fixing CommOpener instead. If _that_ is not sufficient,
we can
discuss appropriate low-level enhancements.
Both things shall be fixed, IMHO.
If double-connect is not needed, we should not introduce it. AFAICT,
double-connect is an attempt to cope with a bug in higher-level code.
We
should fix that bug and see if that is sufficient. If it is not
sufficient, we should evaluate why and only then add appropriate
low-level code if needed.
It's there to fix an async race condition of Squid vs TCP:
1) the timeout occurs ... timeout AsyncCall is scheduled.
2) the socket TCP completes ... completed AsyncCall is scheduled.
3) timeout call is handled:
a) if FD is ready use it, terminate the job early as a success (side
effect: cancel the pending completed Call)
b) else, terminate the job early as a fail.
4) connection completed call is dropped is handled.
The 3a conditional test is what comm_connect_addr() is doing on
timeout. The switch case in ConnOpener::connect() is handling the
multi-state output of the 3a check.
If you can find a cleaner way to rewrite the ConnOpener::connect() and
comm_connect_addr() call chain for the timeout case it should be
duplicated and polished into the timeout() handler.
Amos
