From: Alex Rousskov Date: Sun, 4 Sep 2022 23:40:41 +0000 (+0000) Subject: Replaced most custom high-level callbacks with a unified API (#1094) X-Git-Tag: SQUID_6_0_1~117 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e5ddd4cec4afc7f0d5020e322e66f978eb145888;p=thirdparty%2Fsquid.git Replaced most custom high-level callbacks with a unified API (#1094) Regular asynchronous calls remember call arguments at call creation time, when the type of the call is well known. Service callbacks do not know call arguments until it is time for the service to (asynchronously) call the previously supplied callback. Thus, the service has to get access to call parameters inside the stored generic (from service p.o.v.) async call pointer. Services got that access by declaring a custom callback dialer class and expecting the service user code to use that dialer when creating an async call object. That was problematic: 1. Each service class had to create nearly identical dialer classes, at least one for each unique answer type. Code duplication was rampant. 2. Each service dialer class was specific to one service user type (e.g., HappyConnOpener answer dialer did not support AsyncJob users). 3. A compiler could not check whether the service got the right callback: A dynamic_cast operation converted a generic dialer to the service-declared dialer class, asserting the right type at runtime. This change replaces all but one custom service callback with three general service-independent dialers, one for each callback style: a job call, a call to a cbdata-protected class method, and a stand-alone function call. This unified API solves problem 1 mentioned above. A new Callback template supports all three dialers (addressing problem 2) and checks the callback answer type at compile time (addressing problem 3). Replaced custom HappyConnOpener, Security::PeerConnector, Downloader, CertValidationHelper, Http::Tunneler, Ipc::Inquirer, and most Ipc::StartListening() callbacks (and more such callbacks would have been added by upcoming PRs!). Kept ListeningStartedDialer as detailed below. Replaced Ipc::Inquirer callback with a regular call: The class does not need a callback mechanism because it can (and should and now does) create an async call from scratch when the call arguments become known. Kept ListeningStartedDialer because client_side.cc relies on delivering additional, non-answer parameters with the callback. Normally, this additional information is stored inside the job object. In this case, no job awaits the port opening, so we use a custom dialer hack to keep that information during the wait. Improving that code is out of scope. This may be the only custom high-level callback left in the code! The asyncCall() return type improvement preserves the exact AsyncCall type for the Callback constructor to pick up. It will also remove explicit/imprecise AsyncCall::Pointer uses throughout the code. Also fixed reporting of small responses received by Downloader. Unfortunately, we cannot easily unify the old JobCallback() with the new asyncCallback() because we do not yet know how to detect that we need to use one of the Comm-specific dialers (e.g., CommCbMemFunT) that feed the job pointer to the CommCommonCbParams constructor. CommCommonCbParams do not have a default constructor because they require, probably incorrectly, that the recipient (e.g., job) pointer is stored together with the future callback argument(s). This is why JobCallback() requires a Dialer as a parameter -- it does not know how to guess it either. Comm callback improvements will need a dedicated PR. --- diff --git a/src/Downloader.cc b/src/Downloader.cc index c1295e0476..e3228a888c 100644 --- a/src/Downloader.cc +++ b/src/Downloader.cc @@ -58,18 +58,21 @@ DownloaderContext::finished() http = nullptr; } -void -Downloader::CbDialer::print(std::ostream &os) const +std::ostream & +operator <<(std::ostream &os, const DownloaderAnswer &answer) { - os << " Http Status:" << status << Raw("body data", object.rawContent(), 64).hex(); + os << "outcome=" << answer.outcome; + if (answer.outcome == Http::scOkay) + os << ", resource.size=" << answer.resource.length(); + return os; } -Downloader::Downloader(const SBuf &url, const AsyncCall::Pointer &aCallback, const MasterXactionPointer &masterXaction, unsigned int level): +Downloader::Downloader(const SBuf &url, const AsyncCallback &cb, const MasterXactionPointer &mx, const unsigned int level): AsyncJob("Downloader"), url_(url), - callback_(aCallback), + callback_(cb), level_(level), - masterXaction_(masterXaction) + masterXaction_(mx) { } @@ -256,13 +259,11 @@ void Downloader::callBack(Http::StatusCode const statusCode) { assert(callback_); - CbDialer *dialer = dynamic_cast(callback_->getDialer()); - Must(dialer); - dialer->status = statusCode; + auto &answer = callback_.answer(); + answer.outcome = statusCode; if (statusCode == Http::scOkay) - dialer->object = object_; - ScheduleCallHere(callback_); - callback_ = nullptr; + answer.resource = object_; + ScheduleCallHere(callback_.release()); // We cannot deleteThis() because we may be called synchronously from // doCallouts() via handleReply() (XXX), and doCallouts() may crash if we diff --git a/src/Downloader.h b/src/Downloader.h index 957afdc21f..ec06b2ffdf 100644 --- a/src/Downloader.h +++ b/src/Downloader.h @@ -9,6 +9,7 @@ #ifndef SQUID_DOWNLOADER_H #define SQUID_DOWNLOADER_H +#include "base/AsyncCallbacks.h" #include "base/AsyncJob.h" #include "defines.h" #include "http/forward.h" @@ -23,6 +24,20 @@ typedef RefCount DownloaderContextPointer; class MasterXaction; using MasterXactionPointer = RefCount; +/// download result +class DownloaderAnswer { +public: + // The content of a successfully received HTTP 200 OK reply to our GET request. + // Unused unless outcome is Http::scOkay. + SBuf resource; + + /// Download result summary. + /// May differ from the status code of the downloaded HTTP reply. + Http::StatusCode outcome = Http::scNone; +}; + +std::ostream &operator <<(std::ostream &, const DownloaderAnswer &); + /// The Downloader class fetches SBuf-storable things for other Squid /// components/transactions using internal requests. For example, it is used /// to fetch missing intermediate certificates when validating origin server @@ -31,23 +46,9 @@ class Downloader: virtual public AsyncJob { CBDATA_CLASS(Downloader); public: + using Answer = DownloaderAnswer; - /// Callback data to use with Downloader callbacks. - class CbDialer: public CallDialer { - public: - CbDialer(): status(Http::scNone) {} - virtual ~CbDialer() {} - - /* CallDialer API */ - virtual bool canDial(AsyncCall &call) = 0; - virtual void dial(AsyncCall &call) = 0; - virtual void print(std::ostream &os) const; - - SBuf object; - Http::StatusCode status; - }; - - Downloader(const SBuf &url, const AsyncCall::Pointer &aCallback, const MasterXactionPointer &, unsigned int level = 0); + Downloader(const SBuf &url, const AsyncCallback &, const MasterXactionPointer &, unsigned int level = 0); virtual ~Downloader(); virtual void swanSong(); @@ -74,7 +75,10 @@ private: static const size_t MaxObjectSize = 1*1024*1024; SBuf url_; ///< the url to download - AsyncCall::Pointer callback_; ///< callback to call when download finishes + + /// answer destination + AsyncCallback callback_; + SBuf object_; ///< the object body data const unsigned int level_; ///< holds the nested downloads level MasterXactionPointer masterXaction_; ///< download transaction context diff --git a/src/FwdState.cc b/src/FwdState.cc index 3f6f9a933e..8bdf5206f1 100644 --- a/src/FwdState.cc +++ b/src/FwdState.cc @@ -14,6 +14,8 @@ #include "acl/FilledChecklist.h" #include "acl/Gadgets.h" #include "anyp/PortCfg.h" +#include "base/AsyncCallbacks.h" +#include "base/AsyncCbdataCalls.h" #include "CacheManager.h" #include "CachePeer.h" #include "client_side.h" @@ -77,30 +79,6 @@ PconnPool *fwdPconnPool = new PconnPool("server-peers", nullptr); CBDATA_CLASS_INIT(FwdState); -class FwdStatePeerAnswerDialer: public CallDialer, public Security::PeerConnector::CbDialer -{ -public: - typedef void (FwdState::*Method)(Security::EncryptorAnswer &); - - FwdStatePeerAnswerDialer(Method method, FwdState *fwd): - method_(method), fwd_(fwd), answer_() {} - - /* CallDialer API */ - virtual bool canDial(AsyncCall &) { return fwd_.valid(); } - void dial(AsyncCall &) { ((&(*fwd_))->*method_)(answer_); } - virtual void print(std::ostream &os) const { - os << '(' << fwd_.get() << ", " << answer_ << ')'; - } - - /* Security::PeerConnector::CbDialer API */ - virtual Security::EncryptorAnswer &answer() { return answer_; } - -private: - Method method_; - CbcPointer fwd_; - Security::EncryptorAnswer answer_; -}; - void FwdState::HandleStoreAbort(FwdState *fwd) { @@ -934,9 +912,7 @@ FwdState::noteConnection(HappyConnOpener::Answer &answer) void FwdState::establishTunnelThruProxy(const Comm::ConnectionPointer &conn) { - AsyncCall::Pointer callback = asyncCall(17,4, - "FwdState::tunnelEstablishmentDone", - Http::Tunneler::CbDialer(&FwdState::tunnelEstablishmentDone, this)); + const auto callback = asyncCallback(17, 4, FwdState::tunnelEstablishmentDone, this); HttpRequest::Pointer requestPointer = request; const auto tunneler = new Http::Tunneler(conn, requestPointer, callback, connectingTimeout(conn), al); @@ -1025,9 +1001,7 @@ void FwdState::secureConnectionToPeer(const Comm::ConnectionPointer &conn) { HttpRequest::Pointer requestPointer = request; - AsyncCall::Pointer callback = asyncCall(17,4, - "FwdState::ConnectedToPeer", - FwdStatePeerAnswerDialer(&FwdState::connectedToPeer, this)); + const auto callback = asyncCallback(17, 4, FwdState::connectedToPeer, this); const auto sslNegotiationTimeout = connectingTimeout(conn); Security::PeerConnector *connector = nullptr; #if USE_OPENSSL @@ -1145,8 +1119,7 @@ FwdState::connectStart() request->hier.startPeerClock(); - AsyncCall::Pointer callback = asyncCall(17, 5, "FwdState::noteConnection", HappyConnOpener::CbDialer(&FwdState::noteConnection, this)); - + const auto callback = asyncCallback(17, 5, FwdState::noteConnection, this); HttpRequest::Pointer cause = request; const auto cs = new HappyConnOpener(destinations, callback, cause, start_t, n_tries, al); cs->setHost(request->url.host()); diff --git a/src/HappyConnOpener.cc b/src/HappyConnOpener.cc index e008ca16d0..e1eed3a24b 100644 --- a/src/HappyConnOpener.cc +++ b/src/HappyConnOpener.cc @@ -8,6 +8,7 @@ #include "squid.h" #include "AccessLogEntry.h" +#include "base/AsyncCallbacks.h" #include "base/CodeContext.h" #include "CachePeer.h" #include "errorpage.h" @@ -326,10 +327,10 @@ HappyConnOpenerAnswer::~HappyConnOpenerAnswer() /* HappyConnOpener */ -HappyConnOpener::HappyConnOpener(const ResolvedPeers::Pointer &dests, const AsyncCall::Pointer &aCall, HttpRequest::Pointer &request, const time_t aFwdStart, int tries, const AccessLogEntry::Pointer &anAle): +HappyConnOpener::HappyConnOpener(const ResolvedPeers::Pointer &dests, const AsyncCallback &callback, const HttpRequest::Pointer &request, const time_t aFwdStart, const int tries, const AccessLogEntry::Pointer &anAle): AsyncJob("HappyConnOpener"), fwdStart(aFwdStart), - callback_(aCall), + callback_(callback), destinations(dests), prime(&HappyConnOpener::notePrimeConnectDone, "HappyConnOpener::notePrimeConnectDone"), spare(&HappyConnOpener::noteSpareConnectDone, "HappyConnOpener::noteSpareConnectDone"), @@ -338,7 +339,6 @@ HappyConnOpener::HappyConnOpener(const ResolvedPeers::Pointer &dests, const Asyn n_tries(tries) { assert(destinations); - assert(dynamic_cast(callback_->getDialer())); } HappyConnOpener::~HappyConnOpener() @@ -485,12 +485,12 @@ HappyConnOpener::Answer * HappyConnOpener::futureAnswer(const PeerConnectionPointer &conn) { if (callback_ && !callback_->canceled()) { - const auto answer = dynamic_cast(callback_->getDialer()); - assert(answer); - answer->conn = conn; - answer->n_tries = n_tries; - return answer; + auto &answer = callback_.answer(); + answer.conn = conn; + answer.n_tries = n_tries; + return &answer; } + (void)callback_.release(); return nullptr; } @@ -502,9 +502,8 @@ HappyConnOpener::sendSuccess(const PeerConnectionPointer &conn, const bool reuse if (auto *answer = futureAnswer(conn)) { answer->reused = reused; assert(!answer->error); - ScheduleCallHere(callback_); + ScheduleCallHere(callback_.release()); } - callback_ = nullptr; } /// cancels the in-progress attempt, making its path a future candidate @@ -527,9 +526,8 @@ HappyConnOpener::sendFailure() answer->error = lastError; assert(answer->error.valid()); lastError = nullptr; // the answer owns it now - ScheduleCallHere(callback_); + ScheduleCallHere(callback_.release()); } - callback_ = nullptr; } void diff --git a/src/HappyConnOpener.h b/src/HappyConnOpener.h index a934924cd6..fbde216285 100644 --- a/src/HappyConnOpener.h +++ b/src/HappyConnOpener.h @@ -105,30 +105,8 @@ class HappyConnOpener: public AsyncJob public: typedef HappyConnOpenerAnswer Answer; - /// AsyncCall dialer for our callback. Gives us access to callback Answer. - template - class CbDialer: public CallDialer, public Answer { - public: - // initiator method to receive our answer - typedef void (Initiator::*Method)(Answer &); - - CbDialer(Method method, Initiator *initiator): initiator_(initiator), method_(method) {} - virtual ~CbDialer() = default; - - /* CallDialer API */ - bool canDial(AsyncCall &) { return initiator_.valid(); } - void dial(AsyncCall &) {((*initiator_).*method_)(*this); } - virtual void print(std::ostream &os) const override { - os << '(' << static_cast(*this) << ')'; - } - - private: - CbcPointer initiator_; ///< object to deliver the answer to - Method method_; ///< initiator_ method to call with the answer - }; - public: - HappyConnOpener(const ResolvedPeersPointer &, const AsyncCall::Pointer &, HttpRequestPointer &, const time_t aFwdStart, int tries, const AccessLogEntryPointer &al); + HappyConnOpener(const ResolvedPeersPointer &, const AsyncCallback &, const HttpRequestPointer &, time_t aFwdStart, int tries, const AccessLogEntryPointer &); virtual ~HappyConnOpener() override; /// configures reuse of old connections @@ -216,7 +194,8 @@ private: const time_t fwdStart; ///< requestor start time - AsyncCall::Pointer callback_; ///< handler to be called on connection completion. + /// answer destination + AsyncCallback callback_; /// Candidate paths. Shared with the initiator. May not be finalized yet. ResolvedPeersPointer destinations; diff --git a/src/PeerPoolMgr.cc b/src/PeerPoolMgr.cc index b33c92b3cd..a3502613bd 100644 --- a/src/PeerPoolMgr.cc +++ b/src/PeerPoolMgr.cc @@ -8,7 +8,7 @@ #include "squid.h" #include "AccessLogEntry.h" -#include "base/AsyncJobCalls.h" +#include "base/AsyncCallbacks.h" #include "base/RunnersRegistry.h" #include "CachePeer.h" #include "comm/Connection.h" @@ -27,18 +27,6 @@ CBDATA_CLASS_INIT(PeerPoolMgr); -/// Gives Security::PeerConnector access to Answer in the PeerPoolMgr callback dialer. -class MyAnswerDialer: public UnaryMemFunT, - public Security::PeerConnector::CbDialer -{ -public: - MyAnswerDialer(const JobPointer &aJob, Method aMethod): - UnaryMemFunT(aJob, aMethod, Security::EncryptorAnswer()) {} - - /* Security::PeerConnector::CbDialer API */ - virtual Security::EncryptorAnswer &answer() { return arg1; } -}; - PeerPoolMgr::PeerPoolMgr(CachePeer *aPeer): AsyncJob("PeerPoolMgr"), peer(cbdataReference(aPeer)), request(), @@ -108,8 +96,7 @@ PeerPoolMgr::handleOpenedConnection(const CommConnectCbParams ¶ms) // Handle TLS peers. if (peer->secure.encryptTransport) { // XXX: Exceptions orphan params.conn - AsyncCall::Pointer callback = asyncCall(48, 4, "PeerPoolMgr::handleSecuredPeer", - MyAnswerDialer(this, &PeerPoolMgr::handleSecuredPeer)); + const auto callback = asyncCallback(48, 4, PeerPoolMgr::handleSecuredPeer, this); const auto peerTimeout = peer->connectTimeout(); const int timeUsed = squid_curtime - params.conn->startTime(); diff --git a/src/SquidMath.h b/src/SquidMath.h index 81d816e01f..225f80da5a 100644 --- a/src/SquidMath.h +++ b/src/SquidMath.h @@ -11,9 +11,9 @@ #include "base/forward.h" #include "base/Optional.h" +#include "base/TypeTraits.h" #include -#include // TODO: Move to src/base/Math.h and drop the Math namespace @@ -32,11 +32,6 @@ double doubleAverage(const double, const double, int, const int); // If Sum() performance becomes important, consider using GCC and clang // built-ins like __builtin_add_overflow() instead of manual overflow checks. -/// std::enable_if_t replacement until C++14 -/// simplifies declarations further below -template -using EnableIfType = typename std::enable_if::type; - /// detects a pair of unsigned types /// reduces code duplication in declarations further below template diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc index 3dea5e54ef..47469d2787 100644 --- a/src/adaptation/icap/Xaction.cc +++ b/src/adaptation/icap/Xaction.cc @@ -13,6 +13,7 @@ #include "adaptation/icap/Config.h" #include "adaptation/icap/Launcher.h" #include "adaptation/icap/Xaction.h" +#include "base/AsyncCallbacks.h" #include "base/JobWait.h" #include "base/Optional.h" #include "base/TextException.h" @@ -33,18 +34,6 @@ #include "security/PeerConnector.h" #include "SquidConfig.h" -/// Gives Security::PeerConnector access to Answer in the PeerPoolMgr callback dialer. -class MyIcapAnswerDialer: public UnaryMemFunT, - public Security::PeerConnector::CbDialer -{ -public: - MyIcapAnswerDialer(const JobPointer &aJob, Method aMethod): - UnaryMemFunT(aJob, aMethod, Security::EncryptorAnswer()) {} - - /* Security::PeerConnector::CbDialer API */ - virtual Security::EncryptorAnswer &answer() { return arg1; } -}; - namespace Ssl { /// A simple PeerConnector for Secure ICAP services. No SslBump capabilities. @@ -54,7 +43,7 @@ public: IcapPeerConnector( Adaptation::Icap::ServiceRep::Pointer &service, const Comm::ConnectionPointer &aServerConn, - AsyncCall::Pointer &aCallback, + const AsyncCallback &aCallback, AccessLogEntry::Pointer const &alp, const time_t timeout = 0): AsyncJob("Ssl::IcapPeerConnector"), @@ -270,10 +259,7 @@ Adaptation::Icap::Xaction::useTransportConnection(const Comm::ConnectionPointer const auto &ssl = fd_table[conn->fd].ssl; if (!ssl && service().cfg().secure.encryptTransport) { // XXX: Exceptions orphan conn. - CbcPointer me(this); - AsyncCall::Pointer callback = asyncCall(93, 4, "Adaptation::Icap::Xaction::handleSecuredPeer", - MyIcapAnswerDialer(me, &Adaptation::Icap::Xaction::handleSecuredPeer)); - + const auto callback = asyncCallback(93, 4, Adaptation::Icap::Xaction::handleSecuredPeer, this); const auto sslConnector = new Ssl::IcapPeerConnector(theService, conn, callback, masterLogEntry(), TheConfig.connect_timeout(service().cfg().bypass)); encryptionWait.start(sslConnector, callback); diff --git a/src/base/AsyncCall.cc b/src/base/AsyncCall.cc index f48c8080b4..887d58acba 100644 --- a/src/base/AsyncCall.cc +++ b/src/base/AsyncCall.cc @@ -91,7 +91,7 @@ AsyncCall::dequeue(AsyncCall::Pointer &head, AsyncCall::Pointer &prev) } bool -ScheduleCall(const char *fileName, int fileLine, AsyncCall::Pointer &call) +ScheduleCall(const char *fileName, int fileLine, const AsyncCall::Pointer &call) { debugs(call->debugSection, call->debugLevel, fileName << "(" << fileLine << ") will call " << *call << " [" << call->id << ']' ); diff --git a/src/base/AsyncCall.h b/src/base/AsyncCall.h index c5853b2700..ae43662bf0 100644 --- a/src/base/AsyncCall.h +++ b/src/base/AsyncCall.h @@ -49,7 +49,7 @@ public: // can be called from canFire() for debugging; always returns false bool cancel(const char *reason); - bool canceled() { return isCanceled != nullptr; } + bool canceled() const { return isCanceled != nullptr; } virtual CallDialer *getDialer() = 0; @@ -119,10 +119,12 @@ public: \ingroup AsyncCallAPI * This template implements an AsyncCall using a specified Dialer class */ -template +template class AsyncCallT: public AsyncCall { public: + using Dialer = DialerClass; + AsyncCallT(int aDebugSection, int aDebugLevel, const char *aName, const Dialer &aDialer): AsyncCall(aDebugSection, aDebugLevel, aName), dialer(aDialer) {} @@ -135,6 +137,8 @@ public: CallDialer *getDialer() { return &dialer; } + Dialer dialer; + protected: virtual bool canFire() { return AsyncCall::canFire() && @@ -142,15 +146,12 @@ protected: } virtual void fire() { dialer.dial(*this); } - Dialer dialer; - private: AsyncCallT & operator=(const AsyncCallT &); // not defined. call assignments not permitted. }; template -inline -AsyncCall * +inline RefCount< AsyncCallT > asyncCall(int aDebugSection, int aDebugLevel, const char *aName, const Dialer &aDialer) { @@ -158,7 +159,7 @@ asyncCall(int aDebugSection, int aDebugLevel, const char *aName, } /** Call scheduling helper. Use ScheduleCallHere if you can. */ -bool ScheduleCall(const char *fileName, int fileLine, AsyncCall::Pointer &call); +bool ScheduleCall(const char *fileName, int fileLine, const AsyncCall::Pointer &); /** Call scheduling helper. */ #define ScheduleCallHere(call) ScheduleCall(__FILE__, __LINE__, (call)) diff --git a/src/base/AsyncCallbacks.h b/src/base/AsyncCallbacks.h new file mode 100644 index 0000000000..68fce514b0 --- /dev/null +++ b/src/base/AsyncCallbacks.h @@ -0,0 +1,206 @@ +/* + * Copyright (C) 1996-2022 The Squid Software Foundation and contributors + * + * Squid software is distributed under GPLv2+ license and includes + * contributions from numerous individuals and organizations. + * Please see the COPYING and CONTRIBUTORS files for details. + */ + +#ifndef SQUID_SRC_BASE_ASYNCCALLBACKS_H +#define SQUID_SRC_BASE_ASYNCCALLBACKS_H + +#include "base/AsyncCall.h" +#include "base/AsyncJobCalls.h" +#include "base/TypeTraits.h" + +/// access to a callback result carried by an asynchronous CallDialer +template +class WithAnswer +{ +public: + using Answer = AnswerT; + + virtual ~WithAnswer() = default; + + /// callback results setter + virtual Answer &answer() = 0; +}; + +/// a smart AsyncCall pointer for delivery of future results +template +class AsyncCallback +{ +public: + // all generated copying/moving functions are correct + AsyncCallback() = default; + + template + explicit AsyncCallback(const RefCount &call): + call_(call), + answer_(&(call->dialer.answer())) + { + } + + Answer &answer() + { + assert(answer_); + return *answer_; + } + + /// make this smart pointer nil + /// \return the AsyncCall pointer we used to manage before this call + AsyncCall::Pointer release() + { + answer_ = nullptr; + const auto call = call_; + call_ = nullptr; + return call; + } + + /// whether the callback has been set but not released + explicit operator bool() const { return answer_; } + + /* methods for decaying into an AsyncCall pointer w/o access to answer */ + operator const AsyncCall::Pointer &() const { return call_; } + const AsyncCall &operator *() const { return call_.operator*(); } + const AsyncCall *operator ->() const { return call_.operator->(); } + +private: + /// callback carrying the answer + AsyncCall::Pointer call_; + + /// (future) answer inside this->call, obtained when it was still possible + /// to reach it without dynamic casts and virtual methods + Answer *answer_ = nullptr; +}; + +/// CallDialer for single-parameter callback functions +template +class UnaryFunCallbackDialer: + public CallDialer, + public WithAnswer +{ +public: + // stand-alone function that receives our answer + using Handler = void (Argument1 &); + + explicit UnaryFunCallbackDialer(Handler * const aHandler): handler(aHandler) {} + virtual ~UnaryFunCallbackDialer() = default; + + /* CallDialer API */ + bool canDial(AsyncCall &) { return bool(handler); } + void dial(AsyncCall &) { handler(arg1); } + virtual void print(std::ostream &os) const final { os << '(' << arg1 << ')'; } + + /* WithAnswer API */ + virtual Argument1 &answer() final { return arg1; } + +private: + Handler *handler; ///< the function to call + Argument1 arg1; ///< actual call parameter +}; + +/// CallDialer for single-parameter callback methods of cbdata-protected classes +/// that are not AsyncJobs (use UnaryJobCallbackDialer for the latter). +template +class UnaryCbcCallbackDialer: + public CallDialer, + public WithAnswer +{ +public: + // class member function that receives our answer + typedef void (Destination::*Method)(Argument1 &); + + UnaryCbcCallbackDialer(Method method, Destination *destination): destination_(destination), method_(method) {} + virtual ~UnaryCbcCallbackDialer() = default; + + /* CallDialer API */ + bool canDial(AsyncCall &) { return destination_.valid(); } + void dial(AsyncCall &) {((*destination_).*method_)(arg1_); } + virtual void print(std::ostream &os) const final { os << '(' << arg1_ << ')'; } + + /* WithAnswer API */ + virtual Argument1 &answer() final { return arg1_; } + +private: + CbcPointer destination_; ///< object to deliver the answer to + Method method_; ///< Destination method to call with the answer + Argument1 arg1_; +}; + +/// CallDialer for single-parameter callback methods of AsyncJob classes. +/// \sa UnaryCbcCallbackDialer and UnaryFunCallbackDialer. +template +class UnaryJobCallbackDialer: + public UnaryMemFunT, + public WithAnswer +{ +public: + using Base = UnaryMemFunT; + + UnaryJobCallbackDialer(const CbcPointer &aJob, typename Base::Method aMethod): + Base(aJob, aMethod, {}) {} + + /* WithAnswer API */ + virtual Argument1 &answer() final { return this->arg1; } +}; + +/// whether the given type is an AsyncJob +/// reduces code duplication in declarations further below +template +using IsAsyncJob = typename std::conditional< + std::is_base_of::value, + std::true_type, + std::false_type + >::type; + +/// helper function to simplify UnaryCbcCallbackDialer creation +template ::value, int> = 0> +UnaryCbcCallbackDialer +callbackDialer(void (Destination::*method)(Argument1 &), Destination * const destination) +{ + static_assert(!std::is_base_of::value, "wrong wrapper"); + return UnaryCbcCallbackDialer(method, destination); +} + +/// helper function to simplify UnaryJobCallbackDialer creation +template ::value, int> = 0> +UnaryJobCallbackDialer +callbackDialer(void (Destination::*method)(Argument1 &), Destination * const destination) +{ + static_assert(std::is_base_of::value, "wrong wrapper"); + return UnaryJobCallbackDialer(destination, method); +} + +/// helper function to simplify UnaryFunCallbackDialer creation +template +UnaryFunCallbackDialer +callbackDialer(void (*destination)(Argument1 &)) +{ + return UnaryFunCallbackDialer(destination); +} + +/// helper function to create an AsyncCallback object that matches an AsyncCall +/// based on a WithAnswer answer dialer. +template +AsyncCallback +AsyncCallback_(const RefCount &call) +{ + return AsyncCallback(call); +} + +/// AsyncCall for calling back a class method compatible with +/// callbackDialer(). TODO: Unify with JobCallback() which requires dialers +/// that feed the job pointer to the non-default CommCommonCbParams constructor. +#define asyncCallback(dbgSection, dbgLevel, method, object) \ + AsyncCallback_(asyncCall((dbgSection), (dbgLevel), #method, \ + callbackDialer(&method, (object)))) + +// TODO: Use C++20 __VA_OPT__ to merge this with asyncCallback(). +/// AsyncCall for calling back a function +#define asyncCallbackFun(dbgSection, dbgLevel, function) \ + AsyncCallback_(asyncCall((dbgSection), (dbgLevel), #function, \ + callbackDialer(&function))) + +#endif // SQUID_SRC_BASE_ASYNCCALLBACKS_H + diff --git a/src/base/Makefile.am b/src/base/Makefile.am index 0056b12a06..4ff71b87ac 100644 --- a/src/base/Makefile.am +++ b/src/base/Makefile.am @@ -19,6 +19,7 @@ libbase_la_SOURCES = \ AsyncCallList.h \ AsyncCallQueue.cc \ AsyncCallQueue.h \ + AsyncCallbacks.h \ AsyncCbdataCalls.h \ AsyncFunCalls.h \ AsyncJob.cc \ diff --git a/src/base/TypeTraits.h b/src/base/TypeTraits.h index c9f02049b7..31cc9c2f44 100644 --- a/src/base/TypeTraits.h +++ b/src/base/TypeTraits.h @@ -9,6 +9,8 @@ #ifndef SQUID_SRC_BASE_TYPETRAITS_H #define SQUID_SRC_BASE_TYPETRAITS_H +#include + namespace TypeTraits_ { // a hack to prevent "unintended ADL" // TODO: Extract reusable paradigms into other mixins (e.g., NonCopyable). @@ -37,5 +39,9 @@ protected: // prevents accidental creation of Interface instances using Interface = TypeTraits_::Interface; +/// std::enable_if_t replacement until C++14 +template +using EnableIfType = typename std::enable_if::type; + #endif /* SQUID_SRC_BASE_TYPETRAITS_H */ diff --git a/src/base/forward.h b/src/base/forward.h index ba5b57519a..a0ed6f4786 100644 --- a/src/base/forward.h +++ b/src/base/forward.h @@ -25,6 +25,7 @@ template class Optional; template class CbcPointer; template class RefCount; template class JobWait; +template class AsyncCallback; typedef CbcPointer AsyncJobPointer; typedef RefCount CodeContextPointer; diff --git a/src/client_side.cc b/src/client_side.cc index 7867898ede..287d37636c 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -60,6 +60,7 @@ #include "squid.h" #include "acl/FilledChecklist.h" #include "anyp/PortCfg.h" +#include "base/AsyncCallbacks.h" #include "base/Subscription.h" #include "base/TextException.h" #include "CachePeer.h" @@ -145,26 +146,36 @@ #include #endif +// TODO: Remove this custom dialer and simplify by creating the TcpAcceptor +// subscription later, inside clientListenerConnectionOpened() callback, just +// like htcpOpenPorts(), icpOpenPorts(), and snmpPortOpened() do it. /// dials clientListenerConnectionOpened call -class ListeningStartedDialer: public CallDialer, public Ipc::StartListeningCb +class ListeningStartedDialer: + public CallDialer, + public WithAnswer { public: typedef void (*Handler)(AnyP::PortCfgPointer &portCfg, const Ipc::FdNoteId note, const Subscription::Pointer &sub); ListeningStartedDialer(Handler aHandler, AnyP::PortCfgPointer &aPortCfg, const Ipc::FdNoteId note, const Subscription::Pointer &aSub): handler(aHandler), portCfg(aPortCfg), portTypeNote(note), sub(aSub) {} + /* CallDialer API */ virtual void print(std::ostream &os) const { - startPrint(os) << - ", " << FdNote(portTypeNote) << " port=" << (void*)&portCfg << ')'; + os << '(' << answer_ << ", " << FdNote(portTypeNote) << " port=" << (void*)&portCfg << ')'; } virtual bool canDial(AsyncCall &) const { return true; } virtual void dial(AsyncCall &) { (handler)(portCfg, portTypeNote, sub); } + /* WithAnswer API */ + virtual Ipc::StartListeningAnswer &answer() { return answer_; } + public: Handler handler; private: + // answer_.conn (set/updated by IPC code) is portCfg.listenConn (used by us) + Ipc::StartListeningAnswer answer_; ///< StartListening() results AnyP::PortCfgPointer portCfg; ///< from HttpPortList Ipc::FdNoteId portTypeNote; ///< Type of IPC socket being opened Subscription::Pointer sub; ///< The handler to be subscribed for this connection listener @@ -3364,11 +3375,12 @@ clientStartListeningOn(AnyP::PortCfgPointer &port, const RefCount< CommCbFunPtrC // route new connections to subCall typedef CommCbFunPtrCallT AcceptCall; Subscription::Pointer sub = new CallSubscription(subCall); - AsyncCall::Pointer listenCall = + const auto listenCall = asyncCall(33, 2, "clientListenerConnectionOpened", ListeningStartedDialer(&clientListenerConnectionOpened, port, fdNote, sub)); - Ipc::StartListening(SOCK_STREAM, IPPROTO_TCP, port->listenConn, fdNote, listenCall); + AsyncCallback callback(listenCall); + Ipc::StartListening(SOCK_STREAM, IPPROTO_TCP, port->listenConn, fdNote, callback); assert(NHttpSockets < MAXTCPLISTENPORTS); HttpSockets[NHttpSockets] = -1; diff --git a/src/clients/FtpGateway.cc b/src/clients/FtpGateway.cc index 7343976e7a..087bc2a7af 100644 --- a/src/clients/FtpGateway.cc +++ b/src/clients/FtpGateway.cc @@ -449,7 +449,7 @@ Ftp::Gateway::listenForDataChannel(const Comm::ConnectionPointer &conn) typedef CommCbMemFunT AcceptDialer; typedef AsyncCallT AcceptCall; - RefCount call = static_cast(JobCallback(11, 5, AcceptDialer, this, Ftp::Gateway::ftpAcceptDataConnection)); + const auto call = JobCallback(11, 5, AcceptDialer, this, Ftp::Gateway::ftpAcceptDataConnection); Subscription::Pointer sub = new CallSubscription(call); const char *note = entry->url(); diff --git a/src/clients/HttpTunneler.cc b/src/clients/HttpTunneler.cc index 6b3f037832..19e3f4efef 100644 --- a/src/clients/HttpTunneler.cc +++ b/src/clients/HttpTunneler.cc @@ -26,7 +26,7 @@ CBDATA_NAMESPACED_CLASS_INIT(Http, Tunneler); -Http::Tunneler::Tunneler(const Comm::ConnectionPointer &conn, const HttpRequest::Pointer &req, AsyncCall::Pointer &aCallback, time_t timeout, const AccessLogEntryPointer &alp): +Http::Tunneler::Tunneler(const Comm::ConnectionPointer &conn, const HttpRequest::Pointer &req, const AsyncCallback &aCallback, const time_t timeout, const AccessLogEntryPointer &alp): AsyncJob("Http::Tunneler"), noteFwdPconnUse(false), connection(conn), @@ -39,11 +39,8 @@ Http::Tunneler::Tunneler(const Comm::ConnectionPointer &conn, const HttpRequest: tunnelEstablished(false) { debugs(83, 5, "Http::Tunneler constructed, this=" << (void*)this); - // detect callers supplying cb dialers that are not our CbDialer assert(request); assert(connection); - assert(callback); - assert(dynamic_cast(callback->getDialer())); url = request->url.authority(true); watchForClosures(); } @@ -59,16 +56,6 @@ Http::Tunneler::doneAll() const return !callback || (requestWritten && tunnelEstablished); } -/// convenience method to get to the answer fields -Http::TunnelerAnswer & -Http::Tunneler::answer() -{ - Must(callback); - const auto tunnelerAnswer = dynamic_cast(callback->getDialer()); - Must(tunnelerAnswer); - return *tunnelerAnswer; -} - void Http::Tunneler::start() { @@ -322,7 +309,7 @@ Http::Tunneler::handleResponse(const bool eof) } // CONNECT response was successfully parsed - auto &futureAnswer = answer(); + auto &futureAnswer = callback.answer(); futureAnswer.peerResponseStatus = rep->sline.status(); request->hier.peer_reply_status = rep->sline.status(); @@ -364,7 +351,7 @@ void Http::Tunneler::bailWith(ErrorState *error) { Must(error); - answer().squidError = error; + callback.answer().squidError = error; if (const auto failingConnection = connection) { // TODO: Reuse to-peer connections after a CONNECT error response. @@ -379,9 +366,9 @@ Http::Tunneler::bailWith(ErrorState *error) void Http::Tunneler::sendSuccess() { - assert(answer().positive()); + assert(callback.answer().positive()); assert(Comm::IsConnOpen(connection)); - answer().conn = connection; + callback.answer().conn = connection; disconnect(); callBack(); } @@ -422,11 +409,9 @@ Http::Tunneler::disconnect() void Http::Tunneler::callBack() { - debugs(83, 5, answer().conn << status()); - assert(!connection); // returned inside answer() or gone - auto cb = callback; - callback = nullptr; - ScheduleCallHere(cb); + debugs(83, 5, callback.answer().conn << status()); + assert(!connection); // returned inside callback.answer() or gone + ScheduleCallHere(callback.release()); } void diff --git a/src/clients/HttpTunneler.h b/src/clients/HttpTunneler.h index e88d17610f..21edc6250b 100644 --- a/src/clients/HttpTunneler.h +++ b/src/clients/HttpTunneler.h @@ -9,7 +9,7 @@ #ifndef SQUID_SRC_CLIENTS_HTTP_TUNNELER_H #define SQUID_SRC_CLIENTS_HTTP_TUNNELER_H -#include "base/AsyncCbdataCalls.h" +#include "base/AsyncCallbacks.h" #include "base/AsyncJob.h" #include "clients/forward.h" #include "clients/HttpTunnelerAnswer.h" @@ -34,30 +34,9 @@ class Tunneler: virtual public AsyncJob CBDATA_CLASS(Tunneler); public: - /// Callback dialer API to allow Tunneler to set the answer. - template - class CbDialer: public CallDialer, public Http::TunnelerAnswer - { - public: - // initiator method to receive our answer - typedef void (Initiator::*Method)(Http::TunnelerAnswer &); - - CbDialer(Method method, Initiator *initiator): initiator_(initiator), method_(method) {} - virtual ~CbDialer() = default; - - /* CallDialer API */ - bool canDial(AsyncCall &) { return initiator_.valid(); } - void dial(AsyncCall &) {((*initiator_).*method_)(*this); } - virtual void print(std::ostream &os) const override { - os << '(' << static_cast(*this) << ')'; - } - private: - CbcPointer initiator_; ///< object to deliver the answer to - Method method_; ///< initiator_ method to call with the answer - }; + using Answer = TunnelerAnswer; -public: - Tunneler(const Comm::ConnectionPointer &conn, const HttpRequestPointer &req, AsyncCall::Pointer &aCallback, time_t timeout, const AccessLogEntryPointer &alp); + Tunneler(const Comm::ConnectionPointer &, const HttpRequestPointer &, const AsyncCallback &, time_t timeout, const AccessLogEntryPointer &); Tunneler(const Tunneler &) = delete; Tunneler &operator =(const Tunneler &) = delete; @@ -103,15 +82,13 @@ private: /// updates connection usage history before the connection is closed void countFailingConnection(); - TunnelerAnswer &answer(); - AsyncCall::Pointer writer; ///< called when the request has been written AsyncCall::Pointer reader; ///< called when the response should be read AsyncCall::Pointer closer; ///< called when the connection is being closed Comm::ConnectionPointer connection; ///< TCP connection to the cache_peer HttpRequestPointer request; ///< peer connection trigger or cause - AsyncCall::Pointer callback; ///< we call this with the results + AsyncCallback callback; ///< answer destination SBuf url; ///< request-target for the CONNECT request time_t lifetimeLimit; ///< do not run longer than this AccessLogEntryPointer al; ///< info for the future access.log entry diff --git a/src/comm/Makefile.am b/src/comm/Makefile.am index 27a59e834b..6cac09669e 100644 --- a/src/comm/Makefile.am +++ b/src/comm/Makefile.am @@ -36,7 +36,6 @@ libcomm_la_SOURCES = \ Tcp.h \ TcpAcceptor.cc \ TcpAcceptor.h \ - UdpOpenDialer.h \ Write.cc \ Write.h \ comm_internal.h \ diff --git a/src/comm/UdpOpenDialer.h b/src/comm/UdpOpenDialer.h deleted file mode 100644 index 5833cc350a..0000000000 --- a/src/comm/UdpOpenDialer.h +++ /dev/null @@ -1,36 +0,0 @@ -/* - * Copyright (C) 1996-2022 The Squid Software Foundation and contributors - * - * Squid software is distributed under GPLv2+ license and includes - * contributions from numerous individuals and organizations. - * Please see the COPYING and CONTRIBUTORS files for details. - */ - -#ifndef SQUID_COMM_UDPOPENDIALER_H -#define SQUID_COMM_UDPOPENDIALER_H - -#include "ipc/StartListening.h" - -namespace Comm -{ - -/// dials a UDP port-opened call -class UdpOpenDialer: public CallDialer, - public Ipc::StartListeningCb -{ -public: - typedef void (*Handler)(const Comm::ConnectionPointer &conn, int errNo); - UdpOpenDialer(Handler aHandler): handler(aHandler) {} - - virtual void print(std::ostream &os) const { startPrint(os) << ')'; } - virtual bool canDial(AsyncCall &) const { return true; } - virtual void dial(AsyncCall &) { (handler)(conn, errNo); } - -public: - Handler handler; -}; - -} // namespace Comm - -#endif /* SQUID_COMM_UDPOPENDIALER_H */ - diff --git a/src/htcp.cc b/src/htcp.cc index cf27d8d47e..144704879f 100644 --- a/src/htcp.cc +++ b/src/htcp.cc @@ -12,11 +12,11 @@ #include "AccessLogEntry.h" #include "acl/Acl.h" #include "acl/FilledChecklist.h" +#include "base/AsyncCallbacks.h" #include "CachePeer.h" #include "comm.h" #include "comm/Connection.h" #include "comm/Loops.h" -#include "comm/UdpOpenDialer.h" #include "compat/xalloc.h" #include "debug/Messages.h" #include "globals.h" @@ -26,6 +26,7 @@ #include "HttpRequest.h" #include "icmp/net_db.h" #include "ip/tools.h" +#include "ipc/StartListening.h" #include "md5.h" #include "mem/forward.h" #include "MemBuf.h" @@ -232,7 +233,7 @@ enum { RR_RESPONSE }; -static void htcpIncomingConnectionOpened(const Comm::ConnectionPointer &conn, int errNo); +static void htcpIncomingConnectionOpened(Ipc::StartListeningAnswer&); static uint32_t msg_id_counter = 0; static Comm::ConnectionPointer htcpOutgoingConn = nullptr; @@ -1459,10 +1460,7 @@ htcpOpenPorts(void) htcpIncomingConn->local.setIPv4(); } - AsyncCall::Pointer call = asyncCall(31, 2, - "htcpIncomingConnectionOpened", - Comm::UdpOpenDialer(&htcpIncomingConnectionOpened)); - + auto call = asyncCallbackFun(31, 2, htcpIncomingConnectionOpened); Ipc::StartListening(SOCK_DGRAM, IPPROTO_UDP, htcpIncomingConn, @@ -1497,8 +1495,10 @@ htcpOpenPorts(void) } static void -htcpIncomingConnectionOpened(const Comm::ConnectionPointer &conn, int) +htcpIncomingConnectionOpened(Ipc::StartListeningAnswer &answer) { + const auto &conn = answer.conn; + if (!Comm::IsConnOpen(conn)) fatal("Cannot open HTCP Socket"); diff --git a/src/icp_v2.cc b/src/icp_v2.cc index b2d5c73466..fe9b7cd76d 100644 --- a/src/icp_v2.cc +++ b/src/icp_v2.cc @@ -17,17 +17,18 @@ #include "AccessLogEntry.h" #include "acl/Acl.h" #include "acl/FilledChecklist.h" +#include "base/AsyncCallbacks.h" #include "client_db.h" #include "comm.h" #include "comm/Connection.h" #include "comm/Loops.h" -#include "comm/UdpOpenDialer.h" #include "fd.h" #include "HttpRequest.h" #include "icmp/net_db.h" #include "ICP.h" #include "ip/Address.h" #include "ip/tools.h" +#include "ipc/StartListening.h" #include "ipcache.h" #include "md5.h" #include "multicast.h" @@ -53,7 +54,7 @@ public: struct timeval queue_time = {}; ///< queuing timestamp }; -static void icpIncomingConnectionOpened(const Comm::ConnectionPointer &conn, int errNo); +static void icpIncomingConnectionOpened(Ipc::StartListeningAnswer &); /// \ingroup ServerProtocolICPInternal2 static void icpLogIcp(const Ip::Address &, const LogTags_ot, int, const char *, const int, AccessLogEntryPointer &); @@ -713,10 +714,7 @@ icpOpenPorts(void) icpIncomingConn->local.setIPv4(); } - AsyncCall::Pointer call = asyncCall(12, 2, - "icpIncomingConnectionOpened", - Comm::UdpOpenDialer(&icpIncomingConnectionOpened)); - + auto call = asyncCallbackFun(12, 2, icpIncomingConnectionOpened); Ipc::StartListening(SOCK_DGRAM, IPPROTO_UDP, icpIncomingConn, @@ -751,8 +749,10 @@ icpOpenPorts(void) } static void -icpIncomingConnectionOpened(const Comm::ConnectionPointer &conn, int) +icpIncomingConnectionOpened(Ipc::StartListeningAnswer &answer) { + const auto &conn = answer.conn; + if (!Comm::IsConnOpen(conn)) fatal("Cannot open ICP Port"); diff --git a/src/ipc/Inquirer.cc b/src/ipc/Inquirer.cc index 4fd74f2e8f..05d3b360a2 100644 --- a/src/ipc/Inquirer.cc +++ b/src/ipc/Inquirer.cc @@ -15,14 +15,48 @@ #include "ipc/Inquirer.h" #include "ipc/Port.h" #include "ipc/TypedMsgHdr.h" +#include "mem/PoolingAllocator.h" #include "MemBuf.h" + #include +#include CBDATA_NAMESPACED_CLASS_INIT(Ipc, Inquirer); -Ipc::Inquirer::RequestsMap Ipc::Inquirer::TheRequestsMap; Ipc::RequestId::Index Ipc::Inquirer::LastRequestId = 0; +namespace Ipc { + +/// maps request->id to the Inquirer waiting for the response to that request +using InquirerPointer = CbcPointer; +using WaitingInquiriesItem = std::pair; +using WaitingInquiries = std::unordered_map< + RequestId::Index, + InquirerPointer, + std::hash, + std::equal_to, + PoolingAllocator >; + +/// pending Inquirer requests for this process +static WaitingInquiries TheWaitingInquirers; + +/// returns and forgets the Inquirer waiting for the given requests +static InquirerPointer +DequeueRequest(const RequestId::Index requestId) +{ + debugs(54, 3, "requestId " << requestId); + Assure(requestId != 0); + const auto request = TheWaitingInquirers.find(requestId); + if (request != TheWaitingInquirers.end()) { + const auto inquirer = request->second; + TheWaitingInquirers.erase(request); + return inquirer; // may already be gone by now + } + return nullptr; +} + +} // namespace Ipc + /// compare Ipc::StrandCoord using kidId, for std::sort() below static bool LesserStrandByKidId(const Ipc::StrandCoord &c1, const Ipc::StrandCoord &c2) @@ -68,14 +102,12 @@ Ipc::Inquirer::inquire() } Must(request->requestId == 0); - AsyncCall::Pointer callback = asyncCall(54, 5, "Mgr::Inquirer::handleRemoteAck", - HandleAckDialer(this, &Inquirer::handleRemoteAck, nullptr)); if (++LastRequestId == 0) // don't use zero value as request->requestId ++LastRequestId; request->requestId = LastRequestId; const int kidId = pos->kidId; debugs(54, 4, "inquire kid: " << kidId << status()); - TheRequestsMap[request->requestId] = callback; + TheWaitingInquirers[request->requestId] = this; TypedMsgHdr message; request->pack(message); SendMessage(Port::MakeAddr(strandAddrLabel, kidId), message); @@ -137,32 +169,17 @@ Ipc::Inquirer::callException(const std::exception& e) AsyncJob::callException(e); } -/// returns and forgets the right Inquirer callback for strand request -AsyncCall::Pointer -Ipc::Inquirer::DequeueRequest(const RequestId::Index requestId) -{ - debugs(54, 3, " requestId " << requestId); - Must(requestId != 0); - AsyncCall::Pointer call; - RequestsMap::iterator request = TheRequestsMap.find(requestId); - if (request != TheRequestsMap.end()) { - call = request->second; - Must(call != nullptr); - TheRequestsMap.erase(request); - } - return call; -} - void Ipc::Inquirer::HandleRemoteAck(const Response& response) { Must(response.requestId != 0); - AsyncCall::Pointer call = DequeueRequest(response.requestId); - if (call != nullptr) { - HandleAckDialer* dialer = dynamic_cast(call->getDialer()); - Must(dialer); - dialer->arg1 = response.clone(); - ScheduleCallHere(call); + const auto inquirer = DequeueRequest(response.requestId); + if (inquirer.valid()) { + CallService(inquirer->codeContext, [&] { + const auto call = asyncCall(54, 5, "Ipc::Inquirer::handleRemoteAck", + JobMemFun(inquirer, &Inquirer::handleRemoteAck, response.clone())); + ScheduleCallHere(call); + }); } } diff --git a/src/ipc/Inquirer.h b/src/ipc/Inquirer.h index 8d279007e7..7bd5508368 100644 --- a/src/ipc/Inquirer.h +++ b/src/ipc/Inquirer.h @@ -18,7 +18,6 @@ #include "ipc/Request.h" #include "ipc/Response.h" #include "ipc/StrandCoords.h" -#include namespace Ipc { @@ -60,12 +59,8 @@ protected: virtual bool aggregate(Response::Pointer aResponse) = 0; private: - typedef UnaryMemFunT HandleAckDialer; - void handleRemoteAck(Response::Pointer response); - static AsyncCall::Pointer DequeueRequest(RequestId::Index); - static void RequestTimedOut(void* param); void requestTimedOut(); void removeTimeoutEvent(); @@ -78,10 +73,6 @@ protected: const double timeout; ///< number of seconds to wait for strand response - /// maps request->id to Inquirer::handleRemoteAck callback - typedef std::map RequestsMap; - static RequestsMap TheRequestsMap; ///< pending strand requests - static RequestId::Index LastRequestId; ///< last requestId used }; diff --git a/src/ipc/SharedListen.cc b/src/ipc/SharedListen.cc index 587ab52340..86e434bca1 100644 --- a/src/ipc/SharedListen.cc +++ b/src/ipc/SharedListen.cc @@ -9,6 +9,7 @@ /* DEBUG: section 54 Interprocess Communication */ #include "squid.h" +#include "base/AsyncCallbacks.h" #include "base/TextException.h" #include "comm.h" #include "comm/Connection.h" @@ -29,7 +30,7 @@ class PendingOpenRequest { public: Ipc::OpenListenerParams params; ///< actual comm_open_sharedListen() parameters - AsyncCall::Pointer callback; // who to notify + Ipc::StartListeningCallback callback; // who to notify }; /// maps ID assigned at request time to the response callback @@ -139,7 +140,7 @@ kickDelayedRequest() } void -Ipc::JoinSharedListen(const OpenListenerParams ¶ms, AsyncCall::Pointer &cb) +Ipc::JoinSharedListen(const OpenListenerParams ¶ms, StartListeningCallback &cb) { PendingOpenRequest por; por.params = params; @@ -170,26 +171,26 @@ void Ipc::SharedListenJoined(const SharedListenResponse &response) Must(por.callback); TheSharedListenRequestMap.erase(pori); - StartListeningCb *cbd = dynamic_cast(por.callback->getDialer()); - assert(cbd && cbd->conn != nullptr); - Must(cbd && cbd->conn != nullptr); - cbd->conn->fd = response.fd; + auto &answer = por.callback.answer(); + Assure(answer.conn); + auto &conn = answer.conn; + conn->fd = response.fd; - if (Comm::IsConnOpen(cbd->conn)) { + if (Comm::IsConnOpen(conn)) { OpenListenerParams &p = por.params; - cbd->conn->local = p.addr; - cbd->conn->flags = p.flags; + conn->local = p.addr; + conn->flags = p.flags; // XXX: leave the comm AI stuff to comm_import_opened()? struct addrinfo *AI = nullptr; p.addr.getAddrInfo(AI); AI->ai_socktype = p.sock_type; AI->ai_protocol = p.proto; - comm_import_opened(cbd->conn, FdNote(p.fdNote), AI); + comm_import_opened(conn, FdNote(p.fdNote), AI); Ip::Address::FreeAddr(AI); } - cbd->errNo = response.errNo; - ScheduleCallHere(por.callback); + answer.errNo = response.errNo; + ScheduleCallHere(por.callback.release()); kickDelayedRequest(); } diff --git a/src/ipc/SharedListen.h b/src/ipc/SharedListen.h index 3f72c3dfd8..09465d407c 100644 --- a/src/ipc/SharedListen.h +++ b/src/ipc/SharedListen.h @@ -15,6 +15,7 @@ #include "base/Subscription.h" #include "ipc/QuestionerId.h" #include "ipc/RequestId.h" +#include "ipc/StartListening.h" namespace Ipc { @@ -74,7 +75,7 @@ public: }; /// prepare and send SharedListenRequest to Coordinator -void JoinSharedListen(const OpenListenerParams &, AsyncCall::Pointer &); +void JoinSharedListen(const OpenListenerParams &, StartListeningCallback &); /// process Coordinator response to SharedListenRequest void SharedListenJoined(const SharedListenResponse &response); diff --git a/src/ipc/StartListening.cc b/src/ipc/StartListening.cc index c20ccba962..2f65d74cc7 100644 --- a/src/ipc/StartListening.cc +++ b/src/ipc/StartListening.cc @@ -9,6 +9,7 @@ /* DEBUG: section 54 Interprocess Communication */ #include "squid.h" +#include "base/AsyncCallbacks.h" #include "base/TextException.h" #include "comm.h" #include "comm/Connection.h" @@ -18,26 +19,21 @@ #include -Ipc::StartListeningCb::StartListeningCb(): conn(nullptr), errNo(0) +std::ostream & +Ipc::operator <<(std::ostream &os, const StartListeningAnswer &answer) { -} - -Ipc::StartListeningCb::~StartListeningCb() -{ -} - -std::ostream &Ipc::StartListeningCb::startPrint(std::ostream &os) const -{ - return os << "(" << conn << ", err=" << errNo; + os << answer.conn; + if (answer.errNo) + os << ", err=" << answer.errNo; + return os; } void Ipc::StartListening(int sock_type, int proto, const Comm::ConnectionPointer &listenConn, - FdNoteId fdNote, AsyncCall::Pointer &callback) + const FdNoteId fdNote, StartListeningCallback &callback) { - StartListeningCb *cbd = dynamic_cast(callback->getDialer()); - Must(cbd); - cbd->conn = listenConn; + auto &answer = callback.answer(); + answer.conn = listenConn; const auto giveEachWorkerItsOwnQueue = listenConn->flags & COMM_REUSEPORT; if (!giveEachWorkerItsOwnQueue && UsingSmp()) { @@ -49,16 +45,18 @@ Ipc::StartListening(int sock_type, int proto, const Comm::ConnectionPointer &lis p.addr = listenConn->local; p.flags = listenConn->flags; p.fdNote = fdNote; - Ipc::JoinSharedListen(p, callback); + JoinSharedListen(p, callback); return; // wait for the call back } enter_suid(); - comm_open_listener(sock_type, proto, cbd->conn, FdNote(fdNote)); - cbd->errNo = Comm::IsConnOpen(cbd->conn) ? 0 : errno; + comm_open_listener(sock_type, proto, answer.conn, FdNote(fdNote)); + const auto savedErrno = errno; leave_suid(); - debugs(54, 3, "opened listen " << cbd->conn); - ScheduleCallHere(callback); + answer.errNo = Comm::IsConnOpen(answer.conn) ? 0 : savedErrno; + + debugs(54, 3, "opened listen " << answer); + ScheduleCallHere(callback.release()); } diff --git a/src/ipc/StartListening.h b/src/ipc/StartListening.h index 23d962b9c9..40977944db 100644 --- a/src/ipc/StartListening.h +++ b/src/ipc/StartListening.h @@ -12,6 +12,7 @@ #define SQUID_IPC_START_LISTENING_H #include "base/AsyncCall.h" +#include "base/forward.h" #include "base/Subscription.h" #include "comm/forward.h" #include "ip/forward.h" @@ -22,25 +23,22 @@ namespace Ipc { -/// common API for all StartListening() callbacks -class StartListeningCb +/// StartListening() result +class StartListeningAnswer { -public: - StartListeningCb(); - virtual ~StartListeningCb(); - - /// starts printing arguments, return os - std::ostream &startPrint(std::ostream &os) const; - public: Comm::ConnectionPointer conn; ///< opened listening socket - int errNo; ///< errno value from the comm_open_listener() call + int errNo = 0; ///< errno value from the comm_open_listener() call }; +using StartListeningCallback = AsyncCallback; + /// Depending on whether SMP is on, either ask Coordinator to send us /// the listening FD or open a listening socket directly. void StartListening(int sock_type, int proto, const Comm::ConnectionPointer &listenConn, - FdNoteId fdNote, AsyncCall::Pointer &callback); + FdNoteId, StartListeningCallback &); + +std::ostream &operator <<(std::ostream &, const StartListeningAnswer &); } // namespace Ipc; diff --git a/src/security/BlindPeerConnector.h b/src/security/BlindPeerConnector.h index 37e2fc500e..0fc406f84c 100644 --- a/src/security/BlindPeerConnector.h +++ b/src/security/BlindPeerConnector.h @@ -22,7 +22,7 @@ class BlindPeerConnector: public Security::PeerConnector { public: BlindPeerConnector(HttpRequestPointer &aRequest, const Comm::ConnectionPointer &aServerConn, - AsyncCall::Pointer &aCallback, + const AsyncCallback &aCallback, const AccessLogEntryPointer &alp, const time_t timeout = 0) : AsyncJob("Security::BlindPeerConnector"), diff --git a/src/security/PeerConnector.cc b/src/security/PeerConnector.cc index 9b4b993bca..3729b47100 100644 --- a/src/security/PeerConnector.cc +++ b/src/security/PeerConnector.cc @@ -10,6 +10,7 @@ #include "squid.h" #include "acl/FilledChecklist.h" +#include "base/AsyncCallbacks.h" #include "base/IoManip.h" #include "comm/Loops.h" #include "comm/Read.h" @@ -35,7 +36,7 @@ CBDATA_NAMESPACED_CLASS_INIT(Security, PeerConnector); -Security::PeerConnector::PeerConnector(const Comm::ConnectionPointer &aServerConn, AsyncCall::Pointer &aCallback, const AccessLogEntryPointer &alp, const time_t timeout) : +Security::PeerConnector::PeerConnector(const Comm::ConnectionPointer &aServerConn, const AsyncCallback &aCallback, const AccessLogEntryPointer &alp, const time_t timeout): AsyncJob("Security::PeerConnector"), noteFwdPconnUse(false), serverConn(aServerConn), @@ -48,9 +49,6 @@ Security::PeerConnector::PeerConnector(const Comm::ConnectionPointer &aServerCon { debugs(83, 5, serverConn); - // if this throws, the caller's cb dialer is not our CbDialer - Must(dynamic_cast(callback->getDialer())); - // watch for external connection closures Must(Comm::IsConnOpen(serverConn)); Must(!fd_table[serverConn->fd].closing()); @@ -306,7 +304,7 @@ Security::PeerConnector::sslFinalized() validationRequest.errors = errs; try { debugs(83, 5, "Sending SSL certificate for validation to ssl_crtvd."); - AsyncCall::Pointer call = asyncCall(83,5, "Security::PeerConnector::sslCrtvdHandleReply", Ssl::CertValidationHelper::CbDialer(this, &Security::PeerConnector::sslCrtvdHandleReply, nullptr)); + const auto call = asyncCallback(83, 5, Security::PeerConnector::sslCrtvdHandleReply, this); Ssl::CertValidationHelper::Submit(validationRequest, call); return false; } catch (const std::exception &e) { @@ -330,7 +328,7 @@ Security::PeerConnector::sslFinalized() #if USE_OPENSSL void -Security::PeerConnector::sslCrtvdHandleReply(Ssl::CertValidationResponse::Pointer validationResponse) +Security::PeerConnector::sslCrtvdHandleReply(Ssl::CertValidationResponse::Pointer &validationResponse) { Must(validationResponse != nullptr); Must(Comm::IsConnOpen(serverConnection())); @@ -500,9 +498,7 @@ Security::EncryptorAnswer & Security::PeerConnector::answer() { assert(callback); - const auto dialer = dynamic_cast(callback->getDialer()); - assert(dialer); - return dialer->answer(); + return callback.answer(); } void @@ -561,12 +557,8 @@ void Security::PeerConnector::callBack() { debugs(83, 5, "TLS setup ended for " << answer().conn); - - AsyncCall::Pointer cb = callback; - // Do this now so that if we throw below, swanSong() assert that we _tried_ - // to call back holds. - callback = nullptr; // this should make done() true - ScheduleCallHere(cb); + ScheduleCallHere(callback.release()); + Assure(done()); } void @@ -606,23 +598,6 @@ Security::PeerConnector::status() const } #if USE_OPENSSL -/// CallDialer to allow use Downloader objects within PeerConnector class. -class PeerConnectorCertDownloaderDialer: public Downloader::CbDialer -{ -public: - typedef void (Security::PeerConnector::*Method)(SBuf &object, int status); - - PeerConnectorCertDownloaderDialer(Method method, Security::PeerConnector *pc): - method_(method), - peerConnector_(pc) {} - - /* CallDialer API */ - virtual bool canDial(AsyncCall &) { return peerConnector_.valid(); } - virtual void dial(AsyncCall &) { ((&(*peerConnector_))->*method_)(object, status); } - Method method_; ///< The Security::PeerConnector method to dial - CbcPointer peerConnector_; ///< The Security::PeerConnector object -}; - /// the number of concurrent PeerConnector jobs waiting for us unsigned int Security::PeerConnector::certDownloadNestingLevel() const @@ -640,10 +615,7 @@ Security::PeerConnector::certDownloadNestingLevel() const void Security::PeerConnector::startCertDownloading(SBuf &url) { - AsyncCall::Pointer certCallback = asyncCall(81, 4, - "Security::PeerConnector::certDownloadingDone", - PeerConnectorCertDownloaderDialer(&Security::PeerConnector::certDownloadingDone, this)); - + const auto certCallback = asyncCallback(81, 4, Security::PeerConnector::certDownloadingDone, this); const auto dl = new Downloader(url, certCallback, MasterXaction::MakePortless(), certDownloadNestingLevel() + 1); @@ -651,16 +623,17 @@ Security::PeerConnector::startCertDownloading(SBuf &url) } void -Security::PeerConnector::certDownloadingDone(SBuf &obj, int downloadStatus) +Security::PeerConnector::certDownloadingDone(DownloaderAnswer &downloaderAnswer) { certDownloadWait.finish(); ++certsDownloads; - debugs(81, 5, "Certificate downloading status: " << downloadStatus << " certificate size: " << obj.length()); + debugs(81, 5, "outcome: " << downloaderAnswer.outcome << "; certificate size: " << downloaderAnswer.resource.length()); Must(Comm::IsConnOpen(serverConnection())); const auto &sconn = *fd_table[serverConnection()->fd].ssl; + // XXX: Do not parse the response when the download has failed. // Parse Certificate. Assume that it is in DER format. // According to RFC 4325: // The server must provide a DER encoded certificate or a collection @@ -668,8 +641,8 @@ Security::PeerConnector::certDownloadingDone(SBuf &obj, int downloadStatus) // The applications MUST accept DER encoded certificates and SHOULD // be able to accept collection of certificates. // TODO: support collection of certificates - const unsigned char *raw = (const unsigned char*)obj.rawContent(); - if (X509 *cert = d2i_X509(nullptr, &raw, obj.length())) { + auto raw = reinterpret_cast(downloaderAnswer.resource.rawContent()); + if (auto cert = d2i_X509(nullptr, &raw, downloaderAnswer.resource.length())) { debugs(81, 5, "Retrieved certificate: " << *cert); if (!downloadedCerts) diff --git a/src/security/PeerConnector.h b/src/security/PeerConnector.h index c5e58dffce..de242001b0 100644 --- a/src/security/PeerConnector.h +++ b/src/security/PeerConnector.h @@ -11,7 +11,7 @@ #include "acl/Acl.h" #include "acl/ChecklistFiller.h" -#include "base/AsyncCbdataCalls.h" +#include "base/AsyncCallbacks.h" #include "base/AsyncJob.h" #include "base/JobWait.h" #include "CommCalls.h" @@ -28,6 +28,7 @@ class ErrorState; class Downloader; +class DownloaderAnswer; class AccessLogEntry; typedef RefCount AccessLogEntryPointer; @@ -52,18 +53,8 @@ class PeerConnector: virtual public AsyncJob, public Acl::ChecklistFiller public: typedef CbcPointer Pointer; - /// Callback dialer API to allow PeerConnector to set the answer. - class CbDialer - { - public: - virtual ~CbDialer() {} - /// gives PeerConnector access to the in-dialer answer - virtual Security::EncryptorAnswer &answer() = 0; - }; - -public: PeerConnector(const Comm::ConnectionPointer &aServerConn, - AsyncCall::Pointer &aCallback, + const AsyncCallback &, const AccessLogEntryPointer &alp, const time_t timeout = 0); virtual ~PeerConnector(); @@ -125,7 +116,7 @@ protected: void startCertDownloading(SBuf &url); /// Called by Downloader after a certificate object downloaded. - void certDownloadingDone(SBuf &object, int status); + void certDownloadingDone(DownloaderAnswer &); #endif /// Called when the openSSL SSL_connect function needs to write data to @@ -175,7 +166,10 @@ protected: HttpRequestPointer request; ///< peer connection trigger or cause Comm::ConnectionPointer serverConn; ///< TCP connection to the peer AccessLogEntryPointer al; ///< info for the future access.log entry - AsyncCall::Pointer callback; ///< we call this with the results + + /// answer destination + AsyncCallback callback; + private: PeerConnector(const PeerConnector &); // not implemented PeerConnector &operator =(const PeerConnector &); // not implemented @@ -184,7 +178,7 @@ private: unsigned int certDownloadNestingLevel() const; /// Process response from cert validator helper - void sslCrtvdHandleReply(Ssl::CertValidationResponsePointer); + void sslCrtvdHandleReply(Ssl::CertValidationResponsePointer &); /// Check SSL errors returned from cert validator against sslproxy_cert_error access list Security::CertErrors *sslCrtvdCheckForErrors(Ssl::CertValidationResponse const &, ErrorDetailPointer &); diff --git a/src/servers/FtpServer.cc b/src/servers/FtpServer.cc index 1eb2603bc4..9cacf35c75 100644 --- a/src/servers/FtpServer.cc +++ b/src/servers/FtpServer.cc @@ -386,7 +386,7 @@ Ftp::Server::listenForDataConnection() typedef CommCbMemFunT AcceptDialer; typedef AsyncCallT AcceptCall; - RefCount call = static_cast(JobCallback(5, 5, AcceptDialer, this, Ftp::Server::acceptDataConnection)); + const auto call = JobCallback(5, 5, AcceptDialer, this, Ftp::Server::acceptDataConnection); Subscription::Pointer sub = new CallSubscription(call); listener = call.getRaw(); dataListenConn = conn; diff --git a/src/snmp_core.cc b/src/snmp_core.cc index fbdd8b97e5..42bf096a25 100644 --- a/src/snmp_core.cc +++ b/src/snmp_core.cc @@ -10,16 +10,17 @@ #include "squid.h" #include "acl/FilledChecklist.h" +#include "base/AsyncCallbacks.h" #include "base/CbcPointer.h" #include "CachePeer.h" #include "client_db.h" #include "comm.h" #include "comm/Connection.h" #include "comm/Loops.h" -#include "comm/UdpOpenDialer.h" #include "fatal.h" #include "ip/Address.h" #include "ip/tools.h" +#include "ipc/StartListening.h" #include "snmp/Forwarder.h" #include "snmp_agent.h" #include "snmp_core.h" @@ -27,7 +28,7 @@ #include "SquidConfig.h" #include "tools.h" -static void snmpPortOpened(const Comm::ConnectionPointer &conn, int errNo); +static void snmpPortOpened(Ipc::StartListeningAnswer&); mib_tree_entry *mib_tree_head; mib_tree_entry *mib_tree_last; @@ -273,8 +274,7 @@ snmpOpenPorts(void) snmpIncomingConn->local.setIPv4(); } - AsyncCall::Pointer call = asyncCall(49, 2, "snmpIncomingConnectionOpened", - Comm::UdpOpenDialer(&snmpPortOpened)); + auto call = asyncCallbackFun(49, 2, snmpPortOpened); Ipc::StartListening(SOCK_DGRAM, IPPROTO_UDP, snmpIncomingConn, Ipc::fdnInSnmpSocket, call); if (!Config.Addrs.snmp_outgoing.isNoAddr()) { @@ -290,8 +290,8 @@ snmpOpenPorts(void) if (Ip::EnableIpv6&IPV6_SPECIAL_SPLITSTACK && snmpOutgoingConn->local.isAnyAddr()) { snmpOutgoingConn->local.setIPv4(); } - AsyncCall::Pointer c = asyncCall(49, 2, "snmpOutgoingConnectionOpened", - Comm::UdpOpenDialer(&snmpPortOpened)); + // TODO: Add/use snmpOutgoingPortOpened() instead of snmpPortOpened(). + auto c = asyncCallbackFun(49, 2, snmpPortOpened); Ipc::StartListening(SOCK_DGRAM, IPPROTO_UDP, snmpOutgoingConn, Ipc::fdnOutSnmpSocket, c); } else { snmpOutgoingConn = snmpIncomingConn; @@ -300,8 +300,10 @@ snmpOpenPorts(void) } static void -snmpPortOpened(const Comm::ConnectionPointer &conn, int) +snmpPortOpened(Ipc::StartListeningAnswer &answer) { + const auto &conn = answer.conn; + if (!Comm::IsConnOpen(conn)) fatalf("Cannot open SNMP %s Port",(conn->fd == snmpIncomingConn->fd?"receiving":"sending")); diff --git a/src/ssl/PeekingPeerConnector.cc b/src/ssl/PeekingPeerConnector.cc index 7b7cc2c662..24fd710ad1 100644 --- a/src/ssl/PeekingPeerConnector.cc +++ b/src/ssl/PeekingPeerConnector.cc @@ -28,7 +28,7 @@ CBDATA_NAMESPACED_CLASS_INIT(Ssl, PeekingPeerConnector); Ssl::PeekingPeerConnector::PeekingPeerConnector(HttpRequestPointer &aRequest, const Comm::ConnectionPointer &aServerConn, const Comm::ConnectionPointer &aClientConn, - AsyncCall::Pointer &aCallback, + const AsyncCallback &aCallback, const AccessLogEntryPointer &alp, const time_t timeout): AsyncJob("Ssl::PeekingPeerConnector"), diff --git a/src/ssl/PeekingPeerConnector.h b/src/ssl/PeekingPeerConnector.h index b50899d90c..73f7107255 100644 --- a/src/ssl/PeekingPeerConnector.h +++ b/src/ssl/PeekingPeerConnector.h @@ -23,7 +23,7 @@ public: PeekingPeerConnector(HttpRequestPointer &aRequest, const Comm::ConnectionPointer &aServerConn, const Comm::ConnectionPointer &aClientConn, - AsyncCall::Pointer &aCallback, + const AsyncCallback &aCallback, const AccessLogEntryPointer &alp, time_t timeout = 0); diff --git a/src/ssl/helper.cc b/src/ssl/helper.cc index 3fb5bd84ee..8d9d121c51 100644 --- a/src/ssl/helper.cc +++ b/src/ssl/helper.cc @@ -9,6 +9,7 @@ #include "squid.h" #include "../helper.h" #include "anyp/PortCfg.h" +#include "base/AsyncCallbacks.h" #include "cache_cf.h" #include "fs_io.h" #include "helper/Reply.h" @@ -258,7 +259,7 @@ class submitData public: SBuf query; - AsyncCall::Pointer callback; + Ssl::CertValidationHelper::Callback callback; Security::SessionPointer ssl; }; CBDATA_CLASS_INIT(submitData); @@ -285,10 +286,8 @@ sslCrtvdHandleReplyWrapper(void *data, const ::Helper::Reply &reply) } else validationResponse->resultCode = reply.result; - Ssl::CertValidationHelper::CbDialer *dialer = dynamic_cast(crtdvdData->callback->getDialer()); - Must(dialer); - dialer->arg1 = validationResponse; - ScheduleCallHere(crtdvdData->callback); + crtdvdData->callback.answer() = validationResponse; + ScheduleCallHere(crtdvdData->callback.release()); if (Ssl::CertValidationHelper::HelperCache && (validationResponse->resultCode == ::Helper::Okay || validationResponse->resultCode == ::Helper::Error)) { @@ -298,7 +297,8 @@ sslCrtvdHandleReplyWrapper(void *data, const ::Helper::Reply &reply) delete crtdvdData; } -void Ssl::CertValidationHelper::Submit(Ssl::CertValidationRequest const &request, AsyncCall::Pointer &callback) +void +Ssl::CertValidationHelper::Submit(const Ssl::CertValidationRequest &request, const Callback &callback) { Ssl::CertValidationMsg message(Ssl::CrtdMessage::REQUEST); message.setCode(Ssl::CertValidationMsg::code_cert_validate); @@ -315,10 +315,8 @@ void Ssl::CertValidationHelper::Submit(Ssl::CertValidationRequest const &request if (CertValidationHelper::HelperCache && (validationResponse = CertValidationHelper::HelperCache->get(crtdvdData->query))) { - CertValidationHelper::CbDialer *dialer = dynamic_cast(callback->getDialer()); - Must(dialer); - dialer->arg1 = *validationResponse; - ScheduleCallHere(callback); + crtdvdData->callback.answer() = *validationResponse; + ScheduleCallHere(crtdvdData->callback.release()); delete crtdvdData; return; } @@ -330,10 +328,8 @@ void Ssl::CertValidationHelper::Submit(Ssl::CertValidationRequest const &request Ssl::CertValidationResponse::Pointer resp = new Ssl::CertValidationResponse(crtdvdData->ssl); resp->resultCode = ::Helper::BrokenHelper; - Ssl::CertValidationHelper::CbDialer *dialer = dynamic_cast(callback->getDialer()); - Must(dialer); - dialer->arg1 = resp; - ScheduleCallHere(callback); + crtdvdData->callback.answer() = resp; + ScheduleCallHere(crtdvdData->callback.release()); delete crtdvdData; return; } diff --git a/src/ssl/helper.h b/src/ssl/helper.h index 42e9215be3..5a8bdabf85 100644 --- a/src/ssl/helper.h +++ b/src/ssl/helper.h @@ -43,14 +43,15 @@ class CertValidationResponse; class CertValidationHelper { public: - typedef UnaryMemFunT CbDialer; + using Answer = CertValidationResponse::Pointer; + using Callback = AsyncCallback; typedef void CVHCB(void *, Ssl::CertValidationResponse const &); static void Init(); ///< Init helper structure. static void Shutdown(); ///< Shutdown helper structure. static void Reconfigure(); ///< Reconfigure helper structure /// Submit crtd request message to external crtd server. - static void Submit(Ssl::CertValidationRequest const & request, AsyncCall::Pointer &); + static void Submit(const Ssl::CertValidationRequest &, const Callback &); private: static helper * ssl_crt_validator; ///< helper for management of ssl_crtd. public: diff --git a/src/tests/stub_libsecurity.cc b/src/tests/stub_libsecurity.cc index c006ae9efe..e3053b7946 100644 --- a/src/tests/stub_libsecurity.cc +++ b/src/tests/stub_libsecurity.cc @@ -9,7 +9,6 @@ #include "squid.h" #include "AccessLogEntry.h" #include "comm/Connection.h" -#include "Downloader.h" #include "HttpRequest.h" #define STUB_API "security/libsecurity.la" @@ -77,7 +76,7 @@ class TlsNegotiationDetails: public RefCountable {}; CBDATA_NAMESPACED_CLASS_INIT(Security, PeerConnector); namespace Security { -PeerConnector::PeerConnector(const Comm::ConnectionPointer &, AsyncCall::Pointer &, const AccessLogEntryPointer &, const time_t) : +PeerConnector::PeerConnector(const Comm::ConnectionPointer &, const AsyncCallback &, const AccessLogEntryPointer &, const time_t): AsyncJob("Security::PeerConnector") {STUB} PeerConnector::~PeerConnector() STUB void PeerConnector::start() STUB diff --git a/src/tunnel.cc b/src/tunnel.cc index 9503381a25..5761d11d48 100644 --- a/src/tunnel.cc +++ b/src/tunnel.cc @@ -10,6 +10,7 @@ #include "squid.h" #include "acl/FilledChecklist.h" +#include "base/AsyncCallbacks.h" #include "base/CbcPointer.h" #include "base/JobWait.h" #include "base/Raw.h" @@ -232,31 +233,6 @@ public: void sendError(ErrorState *finalError, const char *reason); private: - /// Gives Security::PeerConnector access to Answer in the TunnelStateData callback dialer. - class MyAnswerDialer: public CallDialer, public Security::PeerConnector::CbDialer - { - public: - typedef void (TunnelStateData::*Method)(Security::EncryptorAnswer &); - - MyAnswerDialer(Method method, TunnelStateData *tunnel): - method_(method), tunnel_(tunnel), answer_() {} - - /* CallDialer API */ - virtual bool canDial(AsyncCall &) { return tunnel_.valid(); } - void dial(AsyncCall &) { ((&(*tunnel_))->*method_)(answer_); } - virtual void print(std::ostream &os) const { - os << '(' << tunnel_.get() << ", " << answer_ << ')'; - } - - /* Security::PeerConnector::CbDialer API */ - virtual Security::EncryptorAnswer &answer() { return answer_; } - - private: - Method method_; - CbcPointer tunnel_; - Security::EncryptorAnswer answer_; - }; - void usePinned(); /// callback handler for the Security::PeerConnector encryptor @@ -1177,8 +1153,7 @@ TunnelStateData::connectToPeer(const Comm::ConnectionPointer &conn) void TunnelStateData::secureConnectionToPeer(const Comm::ConnectionPointer &conn) { - AsyncCall::Pointer callback = asyncCall(5,4, "TunnelStateData::noteSecurityPeerConnectorAnswer", - MyAnswerDialer(&TunnelStateData::noteSecurityPeerConnectorAnswer, this)); + const auto callback = asyncCallback(5, 4, TunnelStateData::noteSecurityPeerConnectorAnswer, this); const auto connector = new Security::BlindPeerConnector(request, conn, callback, al); encryptionWait.start(connector, callback); } @@ -1239,9 +1214,7 @@ TunnelStateData::connectedToPeer(const Comm::ConnectionPointer &conn) void TunnelStateData::establishTunnelThruProxy(const Comm::ConnectionPointer &conn) { - AsyncCall::Pointer callback = asyncCall(5,4, - "TunnelStateData::tunnelEstablishmentDone", - Http::Tunneler::CbDialer(&TunnelStateData::tunnelEstablishmentDone, this)); + const auto callback = asyncCallback(5, 4, TunnelStateData::tunnelEstablishmentDone, this); const auto tunneler = new Http::Tunneler(conn, request, callback, Config.Timeout.lifetime, al); #if USE_DELAY_POOLS tunneler->setDelayId(server.delayId); @@ -1384,7 +1357,7 @@ TunnelStateData::startConnecting() assert(!destinations->empty()); assert(!transporting()); - AsyncCall::Pointer callback = asyncCall(17, 5, "TunnelStateData::noteConnection", HappyConnOpener::CbDialer(&TunnelStateData::noteConnection, this)); + const auto callback = asyncCallback(17, 5, TunnelStateData::noteConnection, this); const auto cs = new HappyConnOpener(destinations, callback, request, startTime, 0, al); cs->setHost(request->url.host()); cs->setRetriable(false);