--- mad-squid-timeout/src/comm/ConnOpener.cc 8 Jan 2013 17:36:44 -0000 1.1.1.2 +++ mad-squid-timeout/src/comm/ConnOpener.cc 15 Jan 2013 22:10:25 -0000 1.1.1.2.2.10 @@ -139,16 +139,11 @@ if (temporaryFd_ >= 0) { debugs(5, 4, HERE << conn_ << " closing temp FD " << temporaryFd_); - // it never reached fully open, so cleanup the FD handlers - // Note that comm_close() sequence does not happen for partially open FD - Comm::SetSelect(temporaryFd_, COMM_SELECT_WRITE, NULL, NULL, 0); calls_.earlyAbort_ = NULL; if (calls_.timeout_ != NULL) { calls_.timeout_->cancel("Comm::ConnOpener::doneConnecting"); calls_.timeout_ = NULL; } - fd_table[temporaryFd_].timeoutHandler = NULL; - fd_table[temporaryFd_].timeout = 0; close(temporaryFd_); fd_close(temporaryFd_); temporaryFd_ = -1; @@ -192,9 +187,9 @@ typedef CommTimeoutCbParams Params; Params ¶ms = GetCommParams(calls_.timeout_); params.conn = conn_; - fd_table[temporaryFd_].timeoutHandler = calls_.timeout_; - fd_table[temporaryFd_].timeout = squid_curtime + (time_t) connectTimeout_; + eventAdd("ConnectTimeout", Comm::ConnOpener::ConnectTimeout, new Pointer(this), connectTimeout_, 0, false); + connectStart_ = squid_curtime; connect(); } @@ -233,25 +228,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 +249,14 @@ 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) { + comm_remove_close_handler(temporaryFd_, calls_.earlyAbort_); + calls_.earlyAbort_ = NULL; + + close(temporaryFd_); + fd_close(temporaryFd_); + temporaryFd_ = -1; + debugs(5, 5, HERE << conn_ << ": * - try again"); eventAdd("Comm::ConnOpener::DelayedConnectRetry", Comm::ConnOpener::DelayedConnectRetry, new Pointer(this), 0.05, 0, false); return; @@ -281,6 +269,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 +317,24 @@ void Comm::ConnOpener::timeout(const CommTimeoutCbParams &) { - connect(); + fde *F; + Pointer *ptr; + + if (!stillConnecting()) + return; + + if (temporaryFd_ > -1) { + F = &fd_table[temporaryFd_]; + if (F->write_handler != NULL) { + ptr = static_cast(F->write_data); + delete ptr; + } + + calls_.earlyAbort_->cancel("Comm::ConnOpener::connect timed out"); + } + + debugs(5, 5, HERE << conn_ << ": * - ERR took too long to receive response."); + doneConnecting(COMM_TIMEOUT, ETIMEDOUT); } /* Legacy Wrapper for the retry event after COMM_INPROGRESS @@ -349,6 +364,19 @@ Pointer *ptr = static_cast(data); assert(ptr); if (ConnOpener *cs = ptr->valid()) { + cs->temporaryFd_ = comm_openex(SOCK_STREAM, IPPROTO_TCP, + cs->conn_->local, cs->conn_->flags, + cs->conn_->tos, cs->conn_->nfmark, + cs->host_); + if (cs->temporaryFd_ < 0) { + cs->doneConnecting(COMM_ERR_CONNECT, 0); + return; + } + + typedef CommCbMemFunT abortDialer; + cs->calls_.earlyAbort_ = JobCallback(5, 4, abortDialer, cs, Comm::ConnOpener::earlyAbort); + comm_add_close_handler(cs->temporaryFd_, cs->calls_.earlyAbort_); + // Ew. we are now outside the all AsyncJob protections. // get back inside by scheduling another call... typedef NullaryMemFunT Dialer; @@ -357,3 +385,16 @@ } delete ptr; } + +/* Legacy wrapper for invoking the timeout call via event scheduler. + See above for XXX. +*/ +void +Comm::ConnOpener::ConnectTimeout(void *data) +{ + Pointer *ptr = static_cast(data); + assert(ptr); + if (ConnOpener *cs = ptr->valid()) + ScheduleCallHere(cs->calls_.timeout_); + delete ptr; +} --- mad-squid-timeout/src/comm/ConnOpener.h 7 Jan 2013 20:15:28 -0000 1.1.1.1 +++ mad-squid-timeout/src/comm/ConnOpener.h 15 Jan 2013 22:15:46 -0000 @@ -43,9 +43,11 @@ void doneConnecting(comm_err_t status, int xerrno); static void InProgressConnectRetry(int fd, void *data); static void DelayedConnectRetry(void *data); + static void ConnectTimeout(void *data); void connect(); void connected(); void lookupLocalAddress(); + bool stillConnecting(); private: char *host_; ///< domain name we are trying to connect to.