From: Amos Jeffries Date: Sat, 17 Jul 2010 06:08:57 +0000 (+1200) Subject: Convert ConnOpener callbacks to Async member-function Calls X-Git-Tag: take08~55^2~124^2~118 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e52e78c4f68966e63437ee9c34f738c7c3400cc1;p=thirdparty%2Fsquid.git Convert ConnOpener callbacks to Async member-function Calls * Also hold on to the EarlyAbort and Timeout call objects This fixes EarlyAbort crashes by allowing proper cancellation * Also fix httpsAccept() build error. --- diff --git a/src/client_side.cc b/src/client_side.cc index b01e699ba4..8e7bfc2471 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -3291,7 +3291,7 @@ clientNegotiateSSL(int fd, void *data) /** handle a new HTTPS connection */ static void -httpsAccept(int sock, int newfd, Comm::ConnectionPointer details, +httpsAccept(int sock, int newfd, Comm::ConnectionPointer& details, comm_err_t flag, int xerrno, void *data) { https_port_list *s = (https_port_list *)data; diff --git a/src/comm/ConnOpener.cc b/src/comm/ConnOpener.cc index 4a4659ebd5..56babe393f 100644 --- a/src/comm/ConnOpener.cc +++ b/src/comm/ConnOpener.cc @@ -2,7 +2,6 @@ #include "comm/ConnOpener.h" #include "comm/Connection.h" #include "comm.h" -#include "CommCalls.h" #include "fde.h" #include "icmp/net_db.h" #include "SquidTime.h" @@ -18,12 +17,16 @@ ConnOpener::ConnOpener(Comm::ConnectionPointer &c, AsyncCall::Pointer handler) : total_tries(0), fail_retries(0), connstart(0) -{} +{ + memset(&calls, 0, sizeof(calls)); +} ConnOpener::~ConnOpener() { safe_free(host); solo = NULL; + calls.earlyabort = NULL; + calls.timeout = NULL; } bool @@ -40,6 +43,25 @@ ConnOpener::doneAll() const return true; } +void +ConnOpener::swanSong() +{ + // recover what we can from the job + if (solo != NULL && solo->fd > -1) { + callCallback(COMM_ERR_CONNECT, 0); + } + + // cancel any event watchers + if (calls.earlyabort != NULL) { + calls.earlyabort->cancel("ConnOpener::swanSong"); + calls.earlyabort = NULL; + } + if (calls.timeout != NULL) { + calls.timeout->cancel("ConnOpener::swanSong"); + calls.timeout = NULL; + } +} + void ConnOpener::setHost(const char * new_host) { @@ -63,8 +85,16 @@ ConnOpener::callCallback(comm_err_t status, int xerrno) { /* remove handlers we don't want to happen anymore */ if (solo != NULL && solo->fd > 0) { - comm_remove_close_handler(solo->fd, ConnOpener::EarlyAbort, this); - commSetTimeout(solo->fd, -1, NULL, NULL); + if (calls.earlyabort != NULL) { + comm_remove_close_handler(solo->fd, calls.earlyabort); + calls.earlyabort->cancel("ConnOpener completed."); + calls.earlyabort = NULL; + } + if (calls.timeout != NULL) { + commSetTimeout(solo->fd, -1, NULL, NULL); + calls.timeout->cancel("ConnOpener completed."); + calls.timeout = NULL; + } } if (callback != NULL) { @@ -98,14 +128,20 @@ ConnOpener::start() return; } - AsyncCall::Pointer ea_call = commCbCall(5,4, "ConnOpener::EarlyAbort", - CommCloseCbPtrFun(ConnOpener::EarlyAbort, this)); - comm_add_close_handler(solo->fd, ea_call); + if (calls.earlyabort == NULL) { + typedef CommCbMemFunT Dialer; + calls.earlyabort = asyncCall(5, 4, "ConnOpener::earlyAbort", + Dialer(this, &ConnOpener::earlyAbort)); + } + comm_add_close_handler(solo->fd, calls.earlyabort); - AsyncCall::Pointer timeout_call = commCbCall(5,4, "ConnOpener::ConnectTimeout", - CommTimeoutCbPtrFun(ConnOpener::ConnectTimeout, this)); + if (calls.timeout == NULL) { + typedef CommCbMemFunT Dialer; + calls.timeout = asyncCall(5, 4, "ConnOpener::connect Timeout", + Dialer(this, &ConnOpener::timeout)); + } debugs(5, 3, HERE << "FD " << solo->fd << " timeout " << connect_timeout); - commSetTimeout(solo->fd, connect_timeout, timeout_call); + commSetTimeout(solo->fd, connect_timeout, calls.timeout); if (connstart == 0) { connstart = squid_curtime; @@ -176,29 +212,26 @@ ConnOpener::start() } void -ConnOpener::EarlyAbort(int fd, void *data) +ConnOpener::earlyAbort(const CommConnectCbParams &io) { - ConnOpener *cs = static_cast(data); - debugs(5, 3, HERE << "FD " << fd); - cs->callCallback(COMM_ERR_CLOSING, errno); // NP: is closing or shutdown better? + debugs(5, 3, HERE << "FD " << io.conn->fd); + callCallback(COMM_ERR_CLOSING, io.xerrno); // NP: is closing or shutdown better? } void -ConnOpener::Connect(void *data) +ConnOpener::connect(const CommConnectCbParams &unused) { - ConnOpener *cs = static_cast(data); - cs->start(); + start(); } void -ConnOpener::ConnectRetry(int fd, void *data) +ConnOpener::timeout(const CommTimeoutCbParams &unused) { - ConnOpener *cs = static_cast(data); - cs->start(); + start(); } void -ConnOpener::ConnectTimeout(int fd, void *data) +ConnOpener::ConnectRetry(int fd, void *data) { ConnOpener *cs = static_cast(data); cs->start(); diff --git a/src/comm/ConnOpener.h b/src/comm/ConnOpener.h index b8632be161..ca271b9b84 100644 --- a/src/comm/ConnOpener.h +++ b/src/comm/ConnOpener.h @@ -4,6 +4,7 @@ #include "base/AsyncCall.h" #include "base/AsyncJob.h" #include "cbdata.h" +#include "CommCalls.h" #include "comm/comm_err_t.h" #include "comm/forward.h" @@ -13,40 +14,44 @@ class ConnOpener : public AsyncJob { public: - /** attempt to open a connection. */ - ConnOpener(Comm::ConnectionPointer &, AsyncCall::Pointer handler); - - ~ConnOpener(); + // ****** AsynJob API implementation ****** /** Actual start opening a TCP connection. */ void start(); virtual bool doneAll() const; + virtual void swanSong(); + + // ****** ConnOpener API iplementation ****** + + /** attempt to open a connection. */ + ConnOpener(Comm::ConnectionPointer &, AsyncCall::Pointer handler); + ~ConnOpener(); + + void setHost(const char *); ///< set the hostname note for this connection + const char * getHost() const; ///< get the hostname noted for this connection + private: /* These objects may NOT be created without connections to act on. Do not define this operator. */ ConnOpener(const ConnOpener &); /* These objects may NOT be copied. Do not define this operator. */ ConnOpener operator =(const ConnOpener &c); - /** - * Wrapper to start the connection attempts happening. + /** Make an FD connection attempt. + * Handles the case(s) when a partially setup connection gets closed early. */ - static void Connect(void *data); - - /** retry */ - static void ConnectRetry(int fd, void *data); + void connect(const CommConnectCbParams &unused); - /** - * Temporary close handler used during connect. + /** Abort connection attempt. * Handles the case(s) when a partially setup connection gets closed early. */ - static void EarlyAbort(int fd, void *data); + void earlyAbort(const CommConnectCbParams &); /** - * Temporary timeout handler used during connect. * Handles the case(s) when a partially setup connection gets timed out. + * NP: When commSetTimeout accepts generic CommCommonCbParams this can die. */ - static void ConnectTimeout(int fd, void *data); + void timeout(const CommTimeoutCbParams &unused); /** * Connection attempt are completed. One way or the other. @@ -54,6 +59,10 @@ private: */ void callCallback(comm_err_t status, int xerrno); + // Legacy Wrapper for the retry event after COMM_INPROGRESS + // As soon as comm IO accepts Async calls we can use a ConnOpener::connect call + static void ConnectRetry(int fd, void *data); + public: /** * time at which to abandon the connection. @@ -61,9 +70,6 @@ public: */ time_t connect_timeout; - void setHost(const char *); ///< set the hostname note for this connection - const char * getHost(void) const; ///< get the hostname noted for this connection - private: char *host; ///< domain name we are trying to connect to. @@ -74,6 +80,12 @@ private: int fail_retries; ///< number of retries current destination has been tried. time_t connstart; ///< time at which this series of connection attempts was started. + /// handles to calls which we may need to cancel. + struct _calls { + AsyncCall::Pointer earlyabort; + AsyncCall::Pointer timeout; + } calls; + CBDATA_CLASS2(ConnOpener); };