From: Amos Jeffries Date: Tue, 22 Nov 2011 12:00:59 +0000 (+1300) Subject: Cleanup: comm Close handlers X-Git-Tag: BumpSslServerFirst.take01~11 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=575d05c4d5e35faf31463d3c0d030c1c4629d105;p=thirdparty%2Fsquid.git Cleanup: comm Close handlers Make handlers take the CommCloseCbParams instead of series of expanded variables. Opening access to the other CommCommonCbParams fields with Connection/FD data. Hiding the deprecated FD parameter from most handlers. Which seem not to have actually needed it in most cases outside Comm. --- diff --git a/src/CommCalls.cc b/src/CommCalls.cc index d16d0af959..77ca002d6c 100644 --- a/src/CommCalls.cc +++ b/src/CommCalls.cc @@ -188,7 +188,7 @@ CommIoCbPtrFun::print(std::ostream &os) const /* CommCloseCbPtrFun */ -CommCloseCbPtrFun::CommCloseCbPtrFun(PF *aHandler, +CommCloseCbPtrFun::CommCloseCbPtrFun(CLCB *aHandler, const CommCloseCbParams &aParams): CommDialerParamsT(aParams), handler(aHandler) @@ -198,7 +198,7 @@ CommCloseCbPtrFun::CommCloseCbPtrFun(PF *aHandler, void CommCloseCbPtrFun::dial() { - handler(params.fd, params.data); + handler(params); } void diff --git a/src/CommCalls.h b/src/CommCalls.h index 065a8b3e32..ef4813a700 100644 --- a/src/CommCalls.h +++ b/src/CommCalls.h @@ -20,6 +20,7 @@ * - connect (CNCB) * - I/O (IOCB) * - timeout (CTCB) + * - close (CLCB) */ class CommAcceptCbParams; @@ -31,6 +32,9 @@ typedef void IOCB(const Comm::ConnectionPointer &conn, char *, size_t size, comm class CommTimeoutCbParams; typedef void CTCB(const CommTimeoutCbParams ¶ms); +class CommCloseCbParams; +typedef void CLCB(const CommCloseCbParams ¶ms); + /* * TODO: When there are no function-pointer-based callbacks left, all * this complexity can be removed. Jobs that need comm services will just @@ -233,20 +237,20 @@ public: }; -// close (PF) dialer +// close (CLCB) dialer class CommCloseCbPtrFun: public CallDialer, public CommDialerParamsT { public: typedef CommCloseCbParams Params; - CommCloseCbPtrFun(PF *aHandler, const Params &aParams); + CommCloseCbPtrFun(CLCB *aHandler, const Params &aParams); void dial(); virtual void print(std::ostream &os) const; public: - PF *handler; + CLCB *handler; }; class CommTimeoutCbPtrFun:public CallDialer, diff --git a/src/CommRead.h b/src/CommRead.h index da23379254..e206822db5 100644 --- a/src/CommRead.h +++ b/src/CommRead.h @@ -80,7 +80,7 @@ public: void kickReads(int const count); private: - static PF CloseHandler; + static CLCB CloseHandler; static DeferredRead popHead(CbDataListContainer &deferredReads); void kickARead(DeferredRead const &); void flushReads(); diff --git a/src/comm.cc b/src/comm.cc index 3fc3a65774..7fb20fed7a 100644 --- a/src/comm.cc +++ b/src/comm.cc @@ -962,6 +962,7 @@ commCallCloseHandlers(int fd) // If call is not canceled schedule it for execution else ignore it if (!call->canceled()) { debugs(5, 5, "commCallCloseHandlers: ch->handler=" << call); + // XXX: this should not be needed. Params can be set by the call creator typedef CommCloseCbParams Params; Params ¶ms = GetCommParams(call); params.fd = fd; @@ -998,10 +999,8 @@ void comm_lingering_close(int fd) { #if USE_SSL - if (fd_table[fd].ssl) - ssl_shutdown_method(fd); - + ssl_shutdown_method(fd_table[fd].ssl); #endif if (shutdown(fd, 1) < 0) { @@ -1047,23 +1046,20 @@ old_comm_reset_close(int fd) comm_close(fd); } +#if USE_SSL void -comm_close_start(int fd, void *data) +commStartSslClose(const CommCloseCbParams ¶ms) { -#if USE_SSL - fde *F = &fd_table[fd]; - if (F->ssl) - ssl_shutdown_method(fd); - -#endif - + assert(&fd_table[params.fd].ssl); + ssl_shutdown_method(fd_table[params.fd].ssl); } +#endif void -comm_close_complete(int fd, void *data) +comm_close_complete(const CommCloseCbParams ¶ms) { #if USE_SSL - fde *F = &fd_table[fd]; + fde *F = &fd_table[params.fd]; if (F->ssl) { SSL_free(F->ssl); @@ -1075,13 +1071,12 @@ comm_close_complete(int fd, void *data) F->dynamicSslContext = NULL; } #endif - fd_close(fd); /* update fdstat */ - - close(fd); + fd_close(params.fd); /* update fdstat */ + close(params.fd); statCounter.syscalls.sock.closes++; - /* When an fd closes, give accept() a chance, if need be */ + /* When one connection closes, give accept() a chance, if need be */ Comm::AcceptLimiter::Instance().kick(); } @@ -1122,12 +1117,16 @@ _comm_close(int fd, char const *file, int line) F->flags.close_request = 1; - AsyncCall::Pointer startCall=commCbCall(5,4, "comm_close_start", - CommCloseCbPtrFun(comm_close_start, NULL)); - typedef CommCloseCbParams Params; - Params &startParams = GetCommParams(startCall); - startParams.fd = fd; - ScheduleCallHere(startCall); +#if USE_SSL + if (F->ssl) { + // XXX: make this a generic async call passing one FD parameter. No need to use CommCloseCbParams + AsyncCall::Pointer startCall=commCbCall(5,4, "commStartSslClose", + CommCloseCbPtrFun(commStartSslClose, NULL)); + CommCloseCbParams &startParams = GetCommParams(startCall); + startParams.fd = fd; + ScheduleCallHere(startCall); + } +#endif // a half-closed fd may lack a reader, so we stop monitoring explicitly if (commHasHalfClosedMonitor(fd)) @@ -1164,7 +1163,7 @@ _comm_close(int fd, char const *file, int line) AsyncCall::Pointer completeCall=commCbCall(5,4, "comm_close_complete", CommCloseCbPtrFun(comm_close_complete, NULL)); - Params &completeParams = GetCommParams(completeCall); + CommCloseCbParams &completeParams = GetCommParams(completeCall); completeParams.fd = fd; // must use async call to wait for all callbacks // scheduled before comm_close() to finish @@ -1215,7 +1214,7 @@ comm_udp_sendto(int fd, } void -comm_add_close_handler(int fd, PF * handler, void *data) +comm_add_close_handler(int fd, CLCB * handler, void *data) { debugs(5, 5, "comm_add_close_handler: FD " << fd << ", handler=" << handler << ", data=" << data); @@ -1242,7 +1241,7 @@ comm_add_close_handler(int fd, AsyncCall::Pointer &call) // remove function-based close handler void -comm_remove_close_handler(int fd, PF * handler, void *data) +comm_remove_close_handler(int fd, CLCB * handler, void *data) { assert (isOpen(fd)); /* Find handler in list */ @@ -2022,6 +2021,7 @@ DeferredReadManager::delayRead(DeferredRead const &aRead) // We have to use a global function as a closer and point to temp // instead of "this" because DeferredReadManager is not a job and // is not even cbdata protected + // XXX: and yet we use cbdata protection functions on it?? AsyncCall::Pointer closer = commCbCall(5,4, "DeferredReadManager::CloseHandler", CommCloseCbPtrFun(&CloseHandler, temp)); @@ -2030,12 +2030,12 @@ DeferredReadManager::delayRead(DeferredRead const &aRead) } void -DeferredReadManager::CloseHandler(int fd, void *thecbdata) +DeferredReadManager::CloseHandler(const CommCloseCbParams ¶ms) { - if (!cbdataReferenceValid (thecbdata)) + if (!cbdataReferenceValid(params.data)) return; - CbDataList *temp = (CbDataList *)thecbdata; + CbDataList *temp = (CbDataList *)params.data; temp->element.closer = NULL; temp->element.markCancelled(); diff --git a/src/comm.h b/src/comm.h index 98c58e403b..5e79dd486a 100644 --- a/src/comm.h +++ b/src/comm.h @@ -72,9 +72,9 @@ SQUIDCEXTERN void checkTimeouts(void); //typedef void IOACB(int fd, int nfd, Comm::ConnectionPointer details, comm_err_t flag, int xerrno, void *data); -extern void comm_add_close_handler(int fd, PF *, void *); +extern void comm_add_close_handler(int fd, CLCB *, void *); extern void comm_add_close_handler(int fd, AsyncCall::Pointer &); -extern void comm_remove_close_handler(int fd, PF *, void *); +extern void comm_remove_close_handler(int fd, CLCB *, void *); extern void comm_remove_close_handler(int fd, AsyncCall::Pointer &); diff --git a/src/dns_internal.cc b/src/dns_internal.cc index 4c3f7ab314..91de65a44c 100644 --- a/src/dns_internal.cc +++ b/src/dns_internal.cc @@ -247,7 +247,7 @@ static PF idnsRead; static EVH idnsCheckQueue; static void idnsTickleQueue(void); static void idnsRcodeCount(int, int); -static void idnsVCClosed(int fd, void *data); +static CLCB idnsVCClosed; static unsigned short idnsQueryID(void); static void @@ -815,9 +815,9 @@ idnsInitVCConnected(const Comm::ConnectionPointer &conn, comm_err_t status, int } static void -idnsVCClosed(int fd, void *data) +idnsVCClosed(const CommCloseCbParams ¶ms) { - nsvc * vc = (nsvc *)data; + nsvc * vc = (nsvc *)params.data; delete vc->queue; delete vc->msg; vc->conn = NULL; diff --git a/src/forward.cc b/src/forward.cc index 7bd660699f..56f3c65471 100644 --- a/src/forward.cc +++ b/src/forward.cc @@ -62,7 +62,7 @@ #endif static PSC fwdPeerSelectionCompleteWrapper; -static PF fwdServerClosedWrapper; +static CLCB fwdServerClosedWrapper; #if USE_SSL static PF fwdNegotiateSSLWrapper; #endif @@ -417,10 +417,10 @@ fwdPeerSelectionCompleteWrapper(Comm::ConnectionList * unused, void *data) } static void -fwdServerClosedWrapper(int fd, void *data) +fwdServerClosedWrapper(const CommCloseCbParams ¶ms) { - FwdState *fwd = (FwdState *) data; - fwd->serverClosed(fd); + FwdState *fwd = (FwdState *)params.data; + fwd->serverClosed(params.fd); } #if USE_SSL diff --git a/src/gopher.cc b/src/gopher.cc index 52ef9adfdf..0eb0d1e646 100644 --- a/src/gopher.cc +++ b/src/gopher.cc @@ -141,7 +141,7 @@ typedef struct gopher_ds { char replybuf[BUFSIZ]; } GopherStateData; -static PF gopherStateFree; +static CLCB gopherStateFree; static void gopherMimeCreate(GopherStateData *); static void gopher_request_parse(const HttpRequest * req, char *type_id, @@ -161,9 +161,9 @@ static char def_gopher_text[] = "text/plain"; /// \ingroup ServerProtocolGopherInternal static void -gopherStateFree(int, void *data) +gopherStateFree(const CommCloseCbParams ¶ms) { - GopherStateData *gopherState = (GopherStateData *)data; + GopherStateData *gopherState = (GopherStateData *)params.data; if (gopherState == NULL) return; diff --git a/src/helper.cc b/src/helper.cc index 6a1ce2aca4..724fd8b4d0 100644 --- a/src/helper.cc +++ b/src/helper.cc @@ -53,8 +53,8 @@ static IOCB helperHandleRead; static IOCB helperStatefulHandleRead; -static PF helperServerFree; -static PF helperStatefulServerFree; +static CLCB helperServerFree; +static CLCB helperStatefulServerFree; static void Enqueue(helper * hlp, helper_request *); static helper_request *Dequeue(helper * hlp); static helper_stateful_request *StatefulDequeue(statefulhelper * hlp); @@ -669,9 +669,9 @@ helper::~helper() /* ====================================================================== */ static void -helperServerFree(int fd, void *data) +helperServerFree(const CommCloseCbParams ¶ms) { - helper_server *srv = (helper_server *)data; + helper_server *srv = (helper_server *)params.data; helper *hlp = srv->parent; helper_request *r; int i, concurrency = hlp->childs.concurrency; @@ -704,7 +704,7 @@ helperServerFree(int fd, void *data) if (!srv->flags.shutdown) { assert(hlp->childs.n_active > 0); hlp->childs.n_active--; - debugs(84, DBG_CRITICAL, "WARNING: " << hlp->id_name << " #" << srv->index + 1 << " (FD " << fd << ") exited"); + debugs(84, DBG_CRITICAL, "WARNING: " << hlp->id_name << " #" << srv->index + 1 << " (FD " << params.fd << ") exited"); if (hlp->childs.needNew() > 0) { debugs(80, 1, "Too few " << hlp->id_name << " processes are running (need " << hlp->childs.needNew() << "/" << hlp->childs.n_max << ")"); @@ -736,9 +736,9 @@ helperServerFree(int fd, void *data) } static void -helperStatefulServerFree(int fd, void *data) +helperStatefulServerFree(const CommCloseCbParams ¶ms) { - helper_stateful_server *srv = (helper_stateful_server *)data; + helper_stateful_server *srv = (helper_stateful_server *)params.data; statefulhelper *hlp = srv->parent; helper_stateful_request *r; @@ -766,7 +766,7 @@ helperStatefulServerFree(int fd, void *data) if (!srv->flags.shutdown) { assert( hlp->childs.n_active > 0); hlp->childs.n_active--; - debugs(84, 0, "WARNING: " << hlp->id_name << " #" << srv->index + 1 << " (FD " << fd << ") exited"); + debugs(84, 0, "WARNING: " << hlp->id_name << " #" << srv->index + 1 << " (FD " << params.fd << ") exited"); if (hlp->childs.needNew() > 0) { debugs(80, 1, "Too few " << hlp->id_name << " processes are running (need " << hlp->childs.needNew() << "/" << hlp->childs.n_max << ")"); diff --git a/src/ident/Ident.cc b/src/ident/Ident.cc index ace6e3c0dd..ec3ee17bd1 100644 --- a/src/ident/Ident.cc +++ b/src/ident/Ident.cc @@ -68,7 +68,7 @@ typedef struct _IdentStateData { // TODO: make these all a series of Async job calls. They are self-contained callbacks now. static IOCB ReadReply; -static PF Close; +static CLCB Close; static CTCB Timeout; static CNCB ConnectDone; static hash_table *ident_hash = NULL; @@ -101,9 +101,9 @@ Ident::identCallback(IdentStateData * state, char *result) } void -Ident::Close(int fdnotused, void *data) +Ident::Close(const CommCloseCbParams ¶ms) { - IdentStateData *state = (IdentStateData *)data; + IdentStateData *state = (IdentStateData *)params.data; identCallback(state, NULL); state->conn->close(); hash_remove_link(ident_hash, (hash_link *) state); diff --git a/src/ssl/support.cc b/src/ssl/support.cc index a5b5ab1e52..dc961c0103 100644 --- a/src/ssl/support.cc +++ b/src/ssl/support.cc @@ -1019,10 +1019,8 @@ ssl_write_method(int fd, const char *buf, int len) } void -ssl_shutdown_method(int fd) +ssl_shutdown_method(SSL *ssl) { - SSL *ssl = fd_table[fd].ssl; - SSL_shutdown(ssl); } diff --git a/src/ssl/support.h b/src/ssl/support.h index beeb87928a..c84b4cf460 100644 --- a/src/ssl/support.h +++ b/src/ssl/support.h @@ -81,7 +81,7 @@ int ssl_read_method(int, char *, int); int ssl_write_method(int, const char *, int); /// \ingroup ServerProtocolSSLAPI -void ssl_shutdown_method(int); +void ssl_shutdown_method(SSL *ssl); /// \ingroup ServerProtocolSSLAPI diff --git a/src/tunnel.cc b/src/tunnel.cc index 6c46e8e310..3344f79926 100644 --- a/src/tunnel.cc +++ b/src/tunnel.cc @@ -124,8 +124,8 @@ static const char *const conn_established = "HTTP/1.1 200 Connection established static CNCB tunnelConnectDone; static ERCB tunnelErrorComplete; -static PF tunnelServerClosed; -static PF tunnelClientClosed; +static CLCB tunnelServerClosed; +static CLCB tunnelClientClosed; static CTCB tunnelTimeout; static PSC tunnelPeerSelectComplete; static void tunnelStateFree(TunnelStateData * tunnelState); @@ -133,10 +133,10 @@ static void tunnelConnected(const Comm::ConnectionPointer &server, void *); static void tunnelRelayConnectRequest(const Comm::ConnectionPointer &server, void *); static void -tunnelServerClosed(int fd, void *data) +tunnelServerClosed(const CommCloseCbParams ¶ms) { - TunnelStateData *tunnelState = (TunnelStateData *)data; - debugs(26, 3, HERE << "FD " << fd); + TunnelStateData *tunnelState = (TunnelStateData *)params.data; + debugs(26, 3, HERE << tunnelState->server.conn); tunnelState->server.conn = NULL; if (tunnelState->noConnections()) { @@ -151,10 +151,10 @@ tunnelServerClosed(int fd, void *data) } static void -tunnelClientClosed(int fd, void *data) +tunnelClientClosed(const CommCloseCbParams ¶ms) { - TunnelStateData *tunnelState = (TunnelStateData *)data; - debugs(26, 3, HERE << "FD " << fd); + TunnelStateData *tunnelState = (TunnelStateData *)params.data; + debugs(26, 3, HERE << tunnelState->client.conn); tunnelState->client.conn = NULL; if (tunnelState->noConnections()) { diff --git a/src/whois.cc b/src/whois.cc index 82dbc0f646..ad30fbfba0 100644 --- a/src/whois.cc +++ b/src/whois.cc @@ -59,7 +59,7 @@ public: bool dataWritten; }; -static PF whoisClose; +static CLCB whoisClose; static CTCB whoisTimeout; static IOCB whoisReadReply; @@ -197,10 +197,10 @@ WhoisState::readReply(const Comm::ConnectionPointer &conn, char *aBuffer, size_t } static void -whoisClose(int fd, void *data) +whoisClose(const CommCloseCbParams ¶ms) { - WhoisState *p = (WhoisState *)data; - debugs(75, 3, "whoisClose: FD " << fd); + WhoisState *p = (WhoisState *)params.data; + debugs(75, 3, "whoisClose: FD " << params.fd); p->entry->unlock(); cbdataFree(p); }