From: Eduard Bagdasaryan Date: Thu, 28 Sep 2023 18:00:59 +0000 (+0000) Subject: Converted helperOpenServers() to a virtual Helper::Client member (#1489) X-Git-Tag: SQUID_7_0_1~344 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=bd71920d64045d5965be3afa4f48047dfba1aadc;p=thirdparty%2Fsquid.git Converted helperOpenServers() to a virtual Helper::Client member (#1489) addressing a TODO. No Squid functionality changes are expected. --- diff --git a/src/auth/basic/Config.cc b/src/auth/basic/Config.cc index afbccfb237..7154c5375e 100644 --- a/src/auth/basic/Config.cc +++ b/src/auth/basic/Config.cc @@ -312,7 +312,7 @@ Auth::Basic::Config::init(Auth::SchemeConfig *) basicauthenticators->ipc_type = IPC_STREAM; - helperOpenServers(basicauthenticators); + basicauthenticators->openSessions(); } } diff --git a/src/auth/digest/Config.cc b/src/auth/digest/Config.cc index 46d028b306..4fc3666b83 100644 --- a/src/auth/digest/Config.cc +++ b/src/auth/digest/Config.cc @@ -533,7 +533,7 @@ Auth::Digest::Config::init(Auth::SchemeConfig *) digestauthenticators->ipc_type = IPC_STREAM; - helperOpenServers(digestauthenticators); + digestauthenticators->openSessions(); } } diff --git a/src/auth/negotiate/Config.cc b/src/auth/negotiate/Config.cc index e1f5e2669b..d449185577 100644 --- a/src/auth/negotiate/Config.cc +++ b/src/auth/negotiate/Config.cc @@ -102,7 +102,7 @@ Auth::Negotiate::Config::init(Auth::SchemeConfig *) negotiateauthenticators->ipc_type = IPC_STREAM; - helperStatefulOpenServers(negotiateauthenticators); + negotiateauthenticators->openSessions(); } } diff --git a/src/auth/ntlm/Config.cc b/src/auth/ntlm/Config.cc index 0928a970ef..58e226560a 100644 --- a/src/auth/ntlm/Config.cc +++ b/src/auth/ntlm/Config.cc @@ -101,7 +101,7 @@ Auth::Ntlm::Config::init(Auth::SchemeConfig *) ntlmauthenticators->ipc_type = IPC_STREAM; - helperStatefulOpenServers(ntlmauthenticators); + ntlmauthenticators->openSessions(); } } diff --git a/src/external_acl.cc b/src/external_acl.cc index 04320ff4e8..1895d7b225 100644 --- a/src/external_acl.cc +++ b/src/external_acl.cc @@ -1123,7 +1123,7 @@ externalAclInit(void) p->theHelper->addr = p->local_addr; - helperOpenServers(p->theHelper); + p->theHelper->openSessions(); } externalAclRegisterWithCacheManager(); diff --git a/src/helper.cc b/src/helper.cc index eed0324f1b..1092a3731d 100644 --- a/src/helper.cc +++ b/src/helper.cc @@ -193,7 +193,7 @@ helper_stateful_server::~helper_stateful_server() } void -helperOpenServers(const Helper::Client::Pointer &hlp) +Helper::Client::openSessions() { char *s; char *progname; @@ -208,6 +208,8 @@ helperOpenServers(const Helper::Client::Pointer &hlp) int wfd; void * hIpc; wordlist *w; + // Helps reducing diff. TODO: remove + const auto hlp = this; if (hlp->cmdline == nullptr) return; @@ -327,18 +329,15 @@ helperOpenServers(const Helper::Client::Pointer &hlp) helperKickQueue(hlp); } -/** - * DPW 2007-05-08 - * - * helperStatefulOpenServers: create the stateful child helper processes - */ void -helperStatefulOpenServers(const statefulhelper::Pointer &hlp) +statefulhelper::openSessions() { char *shortname; const char *args[HELPER_MAX_ARGS+1]; // save space for a NULL terminator char fd_note_buf[FD_DESC_SZ]; int nargs = 0; + // Helps reducing diff. TODO: remove + const auto hlp = this; if (hlp->cmdline == nullptr) return; @@ -891,38 +890,30 @@ Helper::Client::handleFewerServers(const bool madeProgress) } void -Helper::Session::HelperServerClosed(Session * const srv) +Helper::Client::sessionClosed(SessionBase &session) { - const auto hlp = srv->parent; - bool needsNewServers = false; - hlp->handleKilledServer(srv, needsNewServers); + handleKilledServer(&session, needsNewServers); if (needsNewServers) { debugs(80, DBG_IMPORTANT, "Starting new helpers"); - helperOpenServers(hlp); + openSessions(); } +} +void +Helper::Session::HelperServerClosed(Session * const srv) +{ + srv->parent->sessionClosed(*srv); srv->dropQueued(); - delete srv; } -// XXX: Almost duplicates Helper::Session::HelperServerClosed() because helperOpenServers() is not a virtual method of the `Helper::Client` class -// TODO: Fix the `Helper::Client` class hierarchy to use virtual functions. +// XXX: Essentially duplicates Helper::Session::HelperServerClosed() until we add SessionBase::helper(). void helper_stateful_server::HelperServerClosed(helper_stateful_server *srv) { - const auto hlp = srv->parent; - - bool needsNewServers = false; - hlp->handleKilledServer(srv, needsNewServers); - if (needsNewServers) { - debugs(80, DBG_IMPORTANT, "Starting new helpers"); - helperStatefulOpenServers(hlp); - } - + srv->parent->sessionClosed(*srv); srv->dropQueued(); - delete srv; } @@ -1248,8 +1239,7 @@ Enqueue(Helper::Client * const hlp, Helper::Xaction * const r) /* do this first so idle=N has a chance to grow the child pool before it hits critical. */ if (hlp->childs.needNew() > 0) { - debugs(84, DBG_CRITICAL, "Starting new " << hlp->id_name << " helpers..."); - helperOpenServers(hlp); + hlp->openSessions(); return; } @@ -1277,8 +1267,7 @@ StatefulEnqueue(statefulhelper * hlp, Helper::Xaction * r) /* do this first so idle=N has a chance to grow the child pool before it hits critical. */ if (hlp->childs.needNew() > 0) { - debugs(84, DBG_CRITICAL, "Starting new " << hlp->id_name << " helpers..."); - helperStatefulOpenServers(hlp); + hlp->openSessions(); return; } diff --git a/src/helper.h b/src/helper.h index a32eaf209c..fbf2572e2f 100644 --- a/src/helper.h +++ b/src/helper.h @@ -100,6 +100,13 @@ public: /// \param madeProgress whether the died helper(s) responded to any requests void handleFewerServers(bool madeProgress); + /// Starts required helper process(es). + /// The caller is responsible for checking that new processes are needed. + virtual void openSessions(); + + /// handles exited helper process + void sessionClosed(SessionBase &); + public: wordlist *cmdline = nullptr; dlink_list servers; @@ -156,6 +163,9 @@ public: /// undo reserveServer(), clear the reservation and kick the queue void cancelReservation(const Helper::ReservationId reservation); + /* Helper::Client API */ + void openSessions() override; + private: friend void helperStatefulSubmit(const statefulhelper::Pointer &, const char *buf, HLPCB *, void *cbData, const Helper::ReservationId &); @@ -321,8 +331,6 @@ public: time_t reservationStart; ///< when the last `reservation` was made }; -void helperOpenServers(const Helper::Client::Pointer &); -void helperStatefulOpenServers(const statefulhelper::Pointer &); void helperSubmit(const Helper::Client::Pointer &, const char *buf, HLPCB *, void *cbData); void helperStatefulSubmit(const statefulhelper::Pointer &, const char *buf, HLPCB *, void *cbData, uint64_t reservation); void helperShutdown(const Helper::Client::Pointer &); diff --git a/src/redirect.cc b/src/redirect.cc index aee5d79b7e..ddf7901823 100644 --- a/src/redirect.cc +++ b/src/redirect.cc @@ -363,7 +363,7 @@ redirectInit(void) if (Config.onUrlRewriteTimeout.action == toutActUseConfiguredResponse) redirectors->onTimedOutResponse.assign(Config.onUrlRewriteTimeout.response); - helperOpenServers(redirectors); + redirectors->openSessions(); } if (Config.Program.store_id) { @@ -384,7 +384,7 @@ redirectInit(void) storeIds->retryBrokenHelper = true; // XXX: make this configurable ? - helperOpenServers(storeIds); + storeIds->openSessions(); } if (Config.redirector_extras) { diff --git a/src/ssl/helper.cc b/src/ssl/helper.cc index d90f0c3183..1d37c5e776 100644 --- a/src/ssl/helper.cc +++ b/src/ssl/helper.cc @@ -102,7 +102,7 @@ void Ssl::Helper::Init() } safe_free(tmp_begin); } - helperOpenServers(ssl_crtd); + ssl_crtd->openSessions(); } void Ssl::Helper::Shutdown() @@ -222,7 +222,7 @@ void Ssl::CertValidationHelper::Init() } xfree(tmp_begin); } - helperOpenServers(ssl_crt_validator); + ssl_crt_validator->openSessions(); //WARNING: initializing static member in an object initialization method assert(HelperCache == nullptr); diff --git a/src/tests/stub_helper.cc b/src/tests/stub_helper.cc index 593ce43853..a0411d0711 100644 --- a/src/tests/stub_helper.cc +++ b/src/tests/stub_helper.cc @@ -16,9 +16,9 @@ void helperSubmit(const Helper::Client::Pointer &, const char *, HLPCB *, void * void helperStatefulSubmit(const statefulhelper::Pointer &, const char *, HLPCB *, void *, const Helper::ReservationId &) STUB Helper::Client::~Client() STUB void Helper::Client::packStatsInto(Packable *, const char *) const STUB +void Helper::Client::openSessions() STUB void helperShutdown(const Helper::Client::Pointer &) STUB void helperStatefulShutdown(const statefulhelper::Pointer &) STUB -void helperOpenServers(const Helper::Client::Pointer &) STUB -void helperStatefulOpenServers(const statefulhelper::Pointer &) STUB +void statefulhelper::openSessions() STUB