The attached patch moves the body of the timeout handling code from
the ConnOpener::connect method to the ConnOpener::timeout method
instead of invoking ::connect from ::timeout in order to make squid
enforced connect timeouts work (that is, abort the connection) instead
of wrongly making squid believe that the connect succeeded because the
getsockopt call done in comm_connect_addr won't return an error for
this case. This also means that there's now one connect timeout check
done in checkTimeouts instead of two whose results will usually
differ. An internal method named 'stillConnecting' is introduced which
contains the check for an 'abort done by the parent' which is presently
in ConnOpener::connect,
// our parent Jobs signal abort by cancelling their callbacks.
if (callback_ == NULL || callback_->canceled())
return;
This method is called from both ::connect and ::timeout to determine
if the state of the connect request is still undecided by the time
either of both methods executes. This guards against the case that
both ::connect and ::timeout are scheduled before one of them
actually runs and detects when the 'other side' of a full
connection has already gone away or is in the process of going away
(and hence, the already existing 'result state' should be kept).
--- mad-squid/src/comm/ConnOpener.cc 8 Jan 2013 18:30:27 -0000
1.1.1.1.2.1
+++ mad-squid/src/comm/ConnOpener.cc 14 Jan 2013 17:55:26 -0000
1.1.1.1.2.9
@@ -233,25 +233,16 @@
{
Must(conn_ != NULL);
- // our parent Jobs signal abort by cancelling their callbacks.
- if (callback_ == NULL || callback_->canceled())
- return;
+ if (!stillConnecting())
+ return;
++ totalTries_;
switch (comm_connect_addr(temporaryFd_, conn_->remote) ) {
case COMM_INPROGRESS:
- // check for timeout FIRST.
- if (squid_curtime - connectStart_ > connectTimeout_) {
- debugs(5, 5, HERE << conn_ << ": * - ERR took too long already.");
- calls_.earlyAbort_->cancel("Comm::ConnOpener::connect timed out");
- doneConnecting(COMM_TIMEOUT, errno);
- return;
- } else {
- debugs(5, 5, HERE << conn_ << ": COMM_INPROGRESS");
- Comm::SetSelect(temporaryFd_, COMM_SELECT_WRITE,
Comm::ConnOpener::InProgressConnectRetry, new Pointer(this), 0);
- }
+ debugs(5, 5, HERE << conn_ << ": COMM_INPROGRESS");
+ Comm::SetSelect(temporaryFd_, COMM_SELECT_WRITE,
Comm::ConnOpener::InProgressConnectRetry, new Pointer(this), 0);
break;
case COMM_OK:
@@ -263,12 +254,7 @@
default:
++failRetries_;
- // check for timeout FIRST.
- if (squid_curtime - connectStart_ > connectTimeout_) {
- debugs(5, 5, HERE << conn_ << ": * - ERR took too long to receive
response.");
- calls_.earlyAbort_->cancel("Comm::ConnOpener::connect timed out");
- doneConnecting(COMM_TIMEOUT, errno);
- } else if (failRetries_ < Config.connect_retries) {
+ if (failRetries_ < Config.connect_retries) {
debugs(5, 5, HERE << conn_ << ": * - try again");
eventAdd("Comm::ConnOpener::DelayedConnectRetry",
Comm::ConnOpener::DelayedConnectRetry, new Pointer(this), 0.05, 0, false);
return;
@@ -281,6 +267,16 @@
}
}
+/** Check if the result of a connect request is still undecided.
+ * Used by ::timeout and ::connect to determine if there is
+ * still something to do.
+ */
+bool
+Comm::ConnOpener::stillConnecting()
+{
+ return callback_ != 0 && !callback_->canceled();
+}
+
/**
* Lookup local-end address and port of the TCP link just opened.
* This ensure the connection local details are set correctly
@@ -319,7 +315,12 @@
void
Comm::ConnOpener::timeout(const CommTimeoutCbParams &)
{
- connect();
+ if (!stillConnecting())
+ return;
+
+ debugs(5, 5, HERE << conn_ << ": * - ERR took too long to receive
response.");
+ calls_.earlyAbort_->cancel("Comm::ConnOpener::connect timed out");
+ doneConnecting(COMM_TIMEOUT, ETIMEDOUT);
}
/* Legacy Wrapper for the retry event after COMM_INPROGRESS
--- mad-squid/src/comm/ConnOpener.h 7 Jan 2013 20:15:28 -0000 1.1.1.1
+++ mad-squid/src/comm/ConnOpener.h 14 Jan 2013 17:38:32 -0000
1.1.1.1.2.1
@@ -46,6 +46,7 @@
void connect();
void connected();
void lookupLocalAddress();
+ bool stillConnecting();
private:
char *host_; ///< domain name we are trying to
connect to.
