From: Alex Rousskov Date: Mon, 23 Aug 2010 23:15:26 +0000 (-0600) Subject: Bug #2583 fix: pure virtual method called X-Git-Tag: take1~344 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=4299f87;p=thirdparty%2Fsquid.git Bug #2583 fix: pure virtual method called When a cbdata-protected class holds its own cbdata and has virtual toCbdata(), there is a catch22 problem: we need cbdata to know whether the pointer to the class object is valid, and we need to dereference that pointer to get cbdata. Added CbcPointer class to hold both a pointer to a potentially freed class object and the cbdata pointer protecting that object. Keeping the cbdata pointer allows us to test whether the object is still there without dereferencing the object pointer. Use the CbcPointer class to hold safe pointers to AsyncJobs. This prevents "pure virtual method called" failures because we no longer dereference freed job pointers. Removed Initiator parameter from many initiatee constructors. The Adaptation::Initiator::initiateAdaptation method now sets the initiator of the job. This makes the constructor profile simpler and removes the need to propagate Initiator changes through all the [nested] constructors. Renamed AsyncJob::AsyncStart() to AsyncJob::Start(). I had to change the callers code anyway and it was a good opportunity to remove the redundant "Async". Special thanks to Stefan Fritsch for updating and testing an earlier version of this patch. --- diff --git a/src/BodyPipe.cc b/src/BodyPipe.cc index 7717c2bf11..e967bda499 100644 --- a/src/BodyPipe.cc +++ b/src/BodyPipe.cc @@ -1,5 +1,6 @@ #include "squid.h" +#include "base/AsyncJobCalls.h" #include "base/TextException.h" #include "BodyPipe.h" @@ -40,8 +41,9 @@ class BodyProducerDialer: public UnaryMemFunT public: typedef UnaryMemFunT Parent; - BodyProducerDialer(BodyProducer *aProducer, Parent::Method aHandler, - BodyPipe::Pointer bp): Parent(aProducer, aHandler, bp) {} + BodyProducerDialer(const BodyProducer::Pointer &aProducer, + Parent::Method aHandler, BodyPipe::Pointer bp): + Parent(aProducer, aHandler, bp) {} virtual bool canDial(AsyncCall &call); }; @@ -54,8 +56,9 @@ class BodyConsumerDialer: public UnaryMemFunT public: typedef UnaryMemFunT Parent; - BodyConsumerDialer(BodyConsumer *aConsumer, Parent::Method aHandler, - BodyPipe::Pointer bp): Parent(aConsumer, aHandler, bp) {} + BodyConsumerDialer(const BodyConsumer::Pointer &aConsumer, + Parent::Method aHandler, BodyPipe::Pointer bp): + Parent(aConsumer, aHandler, bp) {} virtual bool canDial(AsyncCall &call); }; @@ -66,7 +69,7 @@ BodyProducerDialer::canDial(AsyncCall &call) if (!Parent::canDial(call)) return false; - BodyProducer *producer = object; + const BodyProducer::Pointer &producer = job; BodyPipe::Pointer pipe = arg1; if (!pipe->stillProducing(producer)) { debugs(call.debugSection, call.debugLevel, HERE << producer << @@ -83,7 +86,7 @@ BodyConsumerDialer::canDial(AsyncCall &call) if (!Parent::canDial(call)) return false; - BodyConsumer *consumer = object; + const BodyConsumer::Pointer &consumer = job; BodyPipe::Pointer pipe = arg1; if (!pipe->stillConsuming(consumer)) { debugs(call.debugSection, call.debugLevel, HERE << consumer << @@ -192,9 +195,9 @@ void BodyPipe::expectProductionEndAfter(uint64_t size) void BodyPipe::clearProducer(bool atEof) { - if (theProducer) { + if (theProducer.set()) { debugs(91,7, HERE << "clearing BodyPipe producer" << status()); - theProducer = NULL; + theProducer.clear(); if (atEof) { if (!bodySizeKnown()) theBodySize = thePutSize; @@ -224,10 +227,10 @@ BodyPipe::putMoreData(const char *aBuffer, size_t size) } bool -BodyPipe::setConsumerIfNotLate(Consumer *aConsumer) +BodyPipe::setConsumerIfNotLate(const Consumer::Pointer &aConsumer) { assert(!theConsumer); - assert(aConsumer); + assert(aConsumer.set()); // but might be invalid // TODO: convert this into an exception and remove IfNotLate suffix // If there is something consumed already, we are in an auto-consuming mode @@ -256,9 +259,9 @@ BodyPipe::setConsumerIfNotLate(Consumer *aConsumer) void BodyPipe::clearConsumer() { - if (theConsumer) { + if (theConsumer.set()) { debugs(91,7, HERE << "clearing consumer" << status()); - theConsumer = NULL; + theConsumer.clear(); if (consumedSize() && !exhausted()) { AsyncCall::Pointer call= asyncCall(91, 7, "BodyProducer::noteBodyConsumerAborted", @@ -386,7 +389,7 @@ BodyPipe::postAppend(size_t size) void BodyPipe::scheduleBodyDataNotification() { - if (theConsumer) { + if (theConsumer.valid()) { // TODO: allow asyncCall() to check this instead AsyncCall::Pointer call = asyncCall(91, 7, "BodyConsumer::noteMoreBodyDataAvailable", BodyConsumerDialer(theConsumer, @@ -398,7 +401,7 @@ BodyPipe::scheduleBodyDataNotification() void BodyPipe::scheduleBodyEndNotification() { - if (theConsumer) { + if (theConsumer.valid()) { // TODO: allow asyncCall() to check this instead if (bodySizeKnown() && bodySize() == thePutSize) { AsyncCall::Pointer call = asyncCall(91, 7, "BodyConsumer::noteBodyProductionEnded", @@ -432,10 +435,10 @@ const char *BodyPipe::status() const outputBuffer.Printf(" %d+%d", (int)theBuf.contentSize(), (int)theBuf.spaceSize()); outputBuffer.Printf(" pipe%p", this); - if (theProducer) - outputBuffer.Printf(" prod%p", theProducer); - if (theConsumer) - outputBuffer.Printf(" cons%p", theConsumer); + if (theProducer.set()) + outputBuffer.Printf(" prod%p", theProducer.get()); + if (theConsumer.set()) + outputBuffer.Printf(" cons%p", theConsumer.get()); if (mustAutoConsume) outputBuffer.append(" A", 2); diff --git a/src/BodyPipe.h b/src/BodyPipe.h index e3793ee412..c9e7c93c97 100644 --- a/src/BodyPipe.h +++ b/src/BodyPipe.h @@ -2,8 +2,8 @@ #define SQUID_BODY_PIPE_H #include "MemBuf.h" -#include "base/AsyncCall.h" #include "base/AsyncJob.h" +#include "base/CbcPointer.h" class BodyPipe; @@ -14,6 +14,8 @@ class BodyPipe; class BodyProducer: virtual public AsyncJob { public: + typedef CbcPointer Pointer; + BodyProducer():AsyncJob("BodyProducer") {} virtual ~BodyProducer() {} @@ -32,6 +34,8 @@ protected: class BodyConsumer: virtual public AsyncJob { public: + typedef CbcPointer Pointer; + BodyConsumer():AsyncJob("BodyConsumer") {} virtual ~BodyConsumer() {} @@ -103,17 +107,17 @@ public: bool mayNeedMoreData() const { return !bodySizeKnown() || needsMoreData(); } bool needsMoreData() const { return bodySizeKnown() && unproducedSize() > 0; } uint64_t unproducedSize() const; // size of still unproduced data - bool stillProducing(const Producer *producer) const { return theProducer == producer; } + bool stillProducing(const Producer::Pointer &producer) const { return theProducer == producer; } void expectProductionEndAfter(uint64_t extraSize); ///< sets or checks body size // called by consumers - bool setConsumerIfNotLate(Consumer *aConsumer); + bool setConsumerIfNotLate(const Consumer::Pointer &aConsumer); void clearConsumer(); // aborts if still piping size_t getMoreData(MemBuf &buf); void consume(size_t size); bool expectMoreAfter(uint64_t offset) const; bool exhausted() const; // saw eof/abort and all data consumed - bool stillConsuming(const Consumer *consumer) const { return theConsumer == consumer; } + bool stillConsuming(const Consumer::Pointer &consumer) const { return theConsumer == consumer; } // start or continue consuming when there is no consumer void enableAutoConsumption(); @@ -139,8 +143,8 @@ protected: private: int64_t theBodySize; // expected total content length, if known - Producer *theProducer; // content producer, if any - Consumer *theConsumer; // content consumer, if any + Producer::Pointer theProducer; // content producer, if any + Consumer::Pointer theConsumer; // content consumer, if any uint64_t thePutSize; // ever-increasing total uint64_t theGetSize; // ever-increasing total diff --git a/src/CommCalls.h b/src/CommCalls.h index 5012e836b2..e00754214a 100644 --- a/src/CommCalls.h +++ b/src/CommCalls.h @@ -141,17 +141,18 @@ Params &GetCommParams(AsyncCall::Pointer &call) // All job dialers with comm parameters are merged into one since they // all have exactly one callback argument and differ in Params type only template -class CommCbMemFunT: public JobDialer, public CommDialerParamsT +class CommCbMemFunT: public JobDialer, public CommDialerParamsT { public: typedef Params_ Params; typedef void (C::*Method)(const Params &io); - CommCbMemFunT(C *obj, Method meth): JobDialer(obj), - CommDialerParamsT(obj), object(obj), method(meth) {} + CommCbMemFunT(const CbcPointer &job, Method meth): JobDialer(job), + CommDialerParamsT(job.get()), + method(meth) {} virtual bool canDial(AsyncCall &c) { - return JobDialer::canDial(c) && + return JobDialer::canDial(c) && this->params.syncWithComm(); } @@ -162,11 +163,10 @@ public: } public: - C *object; Method method; protected: - virtual void doDial() { (object->*method)(this->params); } + virtual void doDial() { ((&(*this->job))->*method)(this->params); } }; diff --git a/src/Server.cc b/src/Server.cc index c215c0b224..a1eeee04dc 100644 --- a/src/Server.cc +++ b/src/Server.cc @@ -417,8 +417,8 @@ ServerStateData::sendMoreRequestBody() if (requestBodySource->getMoreData(buf)) { debugs(9,3, HERE << "will write " << buf.contentSize() << " request body bytes"); typedef CommCbMemFunT Dialer; - requestSender = asyncCall(93,3, "ServerStateData::sentRequestBody", - Dialer(this, &ServerStateData::sentRequestBody)); + requestSender = JobCallback(93,3, + Dialer, this, ServerStateData::sentRequestBody); comm_write_mbuf(fd, &buf, requestSender); } else { debugs(9,3, HERE << "will wait for more request body bytes or eof"); @@ -544,8 +544,8 @@ ServerStateData::startAdaptation(const Adaptation::ServiceGroupPointer &group, H } adaptedHeadSource = initiateAdaptation( - new Adaptation::Iterator(this, vrep, cause, group)); - startedAdaptation = adaptedHeadSource != NULL; + new Adaptation::Iterator(vrep, cause, group)); + startedAdaptation = initiated(adaptedHeadSource); Must(startedAdaptation); } diff --git a/src/Server.h b/src/Server.h index f9988825ec..1338567906 100644 --- a/src/Server.h +++ b/src/Server.h @@ -188,7 +188,7 @@ protected: #if USE_ADAPTATION BodyPipe::Pointer virginBodyDestination; /**< to provide virgin response body */ - Adaptation::Initiate *adaptedHeadSource; /**< to get adapted response headers */ + CbcPointer adaptedHeadSource; /**< to get adapted response headers */ BodyPipe::Pointer adaptedBodySource; /**< to consume adated response body */ bool adaptationAccessCheckPending; diff --git a/src/adaptation/AccessCheck.cc b/src/adaptation/AccessCheck.cc index 953c2e84a1..c399636ab5 100644 --- a/src/adaptation/AccessCheck.cc +++ b/src/adaptation/AccessCheck.cc @@ -23,8 +23,9 @@ Adaptation::AccessCheck::Start(Method method, VectPoint vp, if (Config::Enabled) { // the new check will call the callback and delete self, eventually - return AsyncStart(new AccessCheck( - ServiceFilter(method, vp, req, rep), cb, cbdata)); + AsyncJob::Start(new AccessCheck( // we do not store so not a CbcPointer + ServiceFilter(method, vp, req, rep), cb, cbdata)); + return true; } debugs(83, 3, HERE << "adaptation off, skipping"); diff --git a/src/adaptation/Initiate.cc b/src/adaptation/Initiate.cc index 110c4108ba..dcec474770 100644 --- a/src/adaptation/Initiate.cc +++ b/src/adaptation/Initiate.cc @@ -6,6 +6,7 @@ #include "HttpMsg.h" #include "adaptation/Initiator.h" #include "adaptation/Initiate.h" +#include "base/AsyncJobCalls.h" namespace Adaptation { @@ -17,11 +18,13 @@ class AnswerDialer: public UnaryMemFunT public: typedef UnaryMemFunT Parent; - AnswerDialer(Initiator *obj, Parent::Method meth, HttpMsg *msg): - Parent(obj, meth, msg) { HTTPMSGLOCK(arg1); } - AnswerDialer(const AnswerDialer &d): - Parent(d) { HTTPMSGLOCK(arg1); } + AnswerDialer(const Parent::JobPointer &job, Parent::Method meth, + HttpMsg *msg): Parent(job, meth, msg) { HTTPMSGLOCK(arg1); } + AnswerDialer(const AnswerDialer &d): Parent(d) { HTTPMSGLOCK(arg1); } virtual ~AnswerDialer() { HTTPMSGUNLOCK(arg1); } + +private: + AnswerDialer &operator =(const AnswerDialer &); // not implemented }; } // namespace Adaptation @@ -29,10 +32,8 @@ public: /* Initiate */ -Adaptation::Initiate::Initiate(const char *aTypeName, Initiator *anInitiator): - AsyncJob(aTypeName), theInitiator(anInitiator) +Adaptation::Initiate::Initiate(const char *aTypeName): AsyncJob(aTypeName) { - assert(theInitiator); } Adaptation::Initiate::~Initiate() @@ -42,12 +43,21 @@ Adaptation::Initiate::~Initiate() // can assert(!(wasStarted && theInitiator)). } +void +Adaptation::Initiate::initiator(const CbcPointer &i) +{ + Must(!theInitiator); + Must(i.valid()); + theInitiator = i; +} + + // internal cleanup void Adaptation::Initiate::swanSong() { debugs(93, 5, HERE << "swan sings" << status()); - if (theInitiator) { + if (theInitiator.set()) { debugs(93, 3, HERE << "fatal failure; sending abort notification"); tellQueryAborted(true); // final by default } @@ -57,27 +67,22 @@ void Adaptation::Initiate::swanSong() void Adaptation::Initiate::clearInitiator() { - if (theInitiator) - theInitiator.clear(); + theInitiator.clear(); } void Adaptation::Initiate::sendAnswer(HttpMsg *msg) { assert(msg); - if (theInitiator.isThere()) { - CallJob(93, 5, __FILE__, __LINE__, "Initiator::noteAdaptAnswer", - AnswerDialer(theInitiator.ptr(), &Initiator::noteAdaptationAnswer, msg)); - } + CallJob(93, 5, __FILE__, __LINE__, "Initiator::noteAdaptationAnswer", + AnswerDialer(theInitiator, &Initiator::noteAdaptationAnswer, msg)); clearInitiator(); } void Adaptation::Initiate::tellQueryAborted(bool final) { - if (theInitiator.isThere()) { - CallJobHere1(93, 5, theInitiator.ptr(), - Initiator::noteAdaptationQueryAbort, final); - } + CallJobHere1(93, 5, theInitiator, + Initiator, noteAdaptationQueryAbort, final); clearInitiator(); } @@ -85,57 +90,3 @@ const char *Adaptation::Initiate::status() const { return AsyncJob::status(); // for now } - - -/* InitiatorHolder */ - -Adaptation::InitiatorHolder::InitiatorHolder(Initiator *anInitiator): - prime(0), cbdata(0) -{ - if (anInitiator) { - cbdata = cbdataReference(anInitiator->toCbdata()); - prime = anInitiator; - } -} - -Adaptation::InitiatorHolder::InitiatorHolder(const InitiatorHolder &anInitiator): - prime(0), cbdata(0) -{ - if (anInitiator != NULL && cbdataReferenceValid(anInitiator.cbdata)) { - cbdata = cbdataReference(anInitiator.cbdata); - prime = anInitiator.prime; - } -} - -Adaptation::InitiatorHolder::~InitiatorHolder() -{ - clear(); -} - -void Adaptation::InitiatorHolder::clear() -{ - if (prime) { - prime = NULL; - cbdataReferenceDone(cbdata); - } -} - -Adaptation::Initiator *Adaptation::InitiatorHolder::ptr() -{ - assert(isThere()); - return prime; -} - -bool -Adaptation::InitiatorHolder::isThere() -{ - return prime && cbdataReferenceValid(cbdata); -} - -// should not be used -Adaptation::InitiatorHolder & -Adaptation::InitiatorHolder::operator =(const InitiatorHolder &anInitiator) -{ - assert(false); - return *this; -} diff --git a/src/adaptation/Initiate.h b/src/adaptation/Initiate.h index 1cca091995..1baa959c27 100644 --- a/src/adaptation/Initiate.h +++ b/src/adaptation/Initiate.h @@ -1,8 +1,8 @@ #ifndef SQUID_ADAPTATION__INITIATE_H #define SQUID_ADAPTATION__INITIATE_H -#include "base/AsyncCall.h" #include "base/AsyncJob.h" +#include "base/CbcPointer.h" #include "adaptation/forward.h" class HttpMsg; @@ -10,37 +10,6 @@ class HttpMsg; namespace Adaptation { -/* Initiator holder associtates an initiator with its cbdata. It is used as - * a temporary hack to make cbdata work with multiple inheritance. We need - * this hack because we cannot know whether the initiator pointer is still - * valid without dereferencing it to call toCbdata() - * TODO: JobDialer uses the same trick. Factor out or move this code. */ -class InitiatorHolder -{ -public: - InitiatorHolder(Initiator *anInitiator); - InitiatorHolder(const InitiatorHolder &anInitiator); - ~InitiatorHolder(); - - void clear(); - - // to make comparison with NULL possible - operator void*() { return prime; } - bool operator == (void *) const { return prime == NULL; } - bool operator != (void *) const { return prime != NULL; } - bool operator !() const { return !prime; } - - bool isThere(); // we have a valid initiator pointer - Initiator *ptr(); // asserts isThere() - void *theCbdata() { return cbdata;} - -private: - InitiatorHolder &operator =(const InitiatorHolder &anInitiator); - - Initiator *prime; - void *cbdata; -}; - /* * The Initiate is a common base for queries or transactions * initiated by an Initiator. This interface exists to allow an @@ -56,9 +25,11 @@ class Initiate: virtual public AsyncJob { public: - Initiate(const char *aTypeName, Initiator *anInitiator); + Initiate(const char *aTypeName); virtual ~Initiate(); + void initiator(const CbcPointer &i); ///< sets initiator + // communication with the initiator virtual void noteInitiatorAborted() = 0; @@ -71,7 +42,7 @@ protected: virtual const char *status() const; // for debugging - InitiatorHolder theInitiator; + CbcPointer theInitiator; private: Initiate(const Initiate &); // no definition diff --git a/src/adaptation/Initiator.cc b/src/adaptation/Initiator.cc index d5ba04223e..f72e69f8ea 100644 --- a/src/adaptation/Initiator.cc +++ b/src/adaptation/Initiator.cc @@ -5,27 +5,26 @@ #include "squid.h" #include "adaptation/Initiate.h" #include "adaptation/Initiator.h" +#include "base/AsyncJobCalls.h" -Adaptation::Initiate * -Adaptation::Initiator::initiateAdaptation(Adaptation::Initiate *x) +CbcPointer +Adaptation::Initiator::initiateAdaptation(Initiate *x) { - if ((x = dynamic_cast(Initiate::AsyncStart(x)))) - x = cbdataReference(x); - return x; + CbcPointer i(x); + x->initiator(this); + Start(x); + return i; } void -Adaptation::Initiator::clearAdaptation(Initiate *&x) +Adaptation::Initiator::clearAdaptation(CbcPointer &x) { - assert(x); - cbdataReferenceDone(x); + x.clear(); } void -Adaptation::Initiator::announceInitiatorAbort(Initiate *&x) +Adaptation::Initiator::announceInitiatorAbort(CbcPointer &x) { - if (x) { - CallJobHere(93, 5, x, Initiate::noteInitiatorAborted); - clearAdaptation(x); - } + CallJobHere(93, 5, x, Initiate, noteInitiatorAborted); + clearAdaptation(x); } diff --git a/src/adaptation/Initiator.h b/src/adaptation/Initiator.h index ba2a1254f1..32c6a249d7 100644 --- a/src/adaptation/Initiator.h +++ b/src/adaptation/Initiator.h @@ -2,6 +2,7 @@ #define SQUID_ADAPTATION__INITIATOR_H #include "base/AsyncJob.h" +#include "base/CbcPointer.h" #include "adaptation/forward.h" /* @@ -32,13 +33,17 @@ public: virtual void noteAdaptationQueryAbort(bool final) = 0; protected: - Initiate *initiateAdaptation(Initiate *x); // locks and returns x + ///< starts freshly created initiate and returns a safe pointer to it + CbcPointer initiateAdaptation(Initiate *x); - // done with x (and not calling announceInitiatorAbort) - void clearAdaptation(Initiate *&x); // unlocks x + /// clears the pointer (does not call announceInitiatorAbort) + void clearAdaptation(CbcPointer &x); - // inform the transaction about abnormal termination and clear it - void announceInitiatorAbort(Initiate *&x); // unlocks x + /// inform the transaction about abnormal termination and clear the pointer + void announceInitiatorAbort(CbcPointer &x); + + /// Must(initiated(initiate)) instead of Must(initiate.set()), for clarity + bool initiated(const CbcPointer &job) const { return job.set(); } }; } // namespace Adaptation diff --git a/src/adaptation/Iterator.cc b/src/adaptation/Iterator.cc index a85932dfca..57a277ea61 100644 --- a/src/adaptation/Iterator.cc +++ b/src/adaptation/Iterator.cc @@ -14,11 +14,11 @@ #include "HttpMsg.h" -Adaptation::Iterator::Iterator(Adaptation::Initiator *anInitiator, - HttpMsg *aMsg, HttpRequest *aCause, - const ServiceGroupPointer &aGroup): +Adaptation::Iterator::Iterator( + HttpMsg *aMsg, HttpRequest *aCause, + const ServiceGroupPointer &aGroup): AsyncJob("Iterator"), - Adaptation::Initiate("Iterator", anInitiator), + Adaptation::Initiate("Iterator"), theGroup(aGroup), theMsg(HTTPMSGLOCK(aMsg)), theCause(aCause ? HTTPMSGLOCK(aCause) : NULL), @@ -69,8 +69,8 @@ void Adaptation::Iterator::step() debugs(93,5, HERE << "using adaptation service: " << service->cfg().key); theLauncher = initiateAdaptation( - service->makeXactLauncher(this, theMsg, theCause)); - Must(theLauncher); + service->makeXactLauncher(theMsg, theCause)); + Must(initiated(theLauncher)); Must(!done()); } @@ -148,10 +148,10 @@ bool Adaptation::Iterator::doneAll() const void Adaptation::Iterator::swanSong() { - if (theInitiator) + if (theInitiator.set()) tellQueryAborted(true); // abnormal condition that should not happen - if (theLauncher) + if (initiated(theLauncher)) clearAdaptation(theLauncher); Adaptation::Initiate::swanSong(); diff --git a/src/adaptation/Iterator.h b/src/adaptation/Iterator.h index 0a0b35483a..b1313bf190 100644 --- a/src/adaptation/Iterator.h +++ b/src/adaptation/Iterator.h @@ -21,8 +21,7 @@ namespace Adaptation class Iterator: public Initiate, public Initiator { public: - Iterator(Adaptation::Initiator *anInitiator, - HttpMsg *virginHeader, HttpRequest *virginCause, + Iterator(HttpMsg *virginHeader, HttpRequest *virginCause, const Adaptation::ServiceGroupPointer &aGroup); virtual ~Iterator(); @@ -52,7 +51,7 @@ protected: ServicePlan thePlan; ///< which services to use and in what order HttpMsg *theMsg; ///< the message being adapted (virgin for each step) HttpRequest *theCause; ///< the cause of the original virgin message - Adaptation::Initiate *theLauncher; ///< current transaction launcher + CbcPointer theLauncher; ///< current transaction launcher int iterations; ///< number of steps initiated bool adapted; ///< whether the virgin message has been replaced diff --git a/src/adaptation/Service.h b/src/adaptation/Service.h index 50a727c7ae..59ea039f4c 100644 --- a/src/adaptation/Service.h +++ b/src/adaptation/Service.h @@ -31,7 +31,7 @@ public: virtual bool broken() const; virtual bool up() const = 0; // see comments above - virtual Initiate *makeXactLauncher(Initiator *, HttpMsg *virginHeader, HttpRequest *virginCause) = 0; + virtual Initiate *makeXactLauncher(HttpMsg *virginHeader, HttpRequest *virginCause) = 0; bool wants(const ServiceFilter &filter) const; diff --git a/src/adaptation/ecap/ServiceRep.cc b/src/adaptation/ecap/ServiceRep.cc index 8244accf17..0a5e52dcd9 100644 --- a/src/adaptation/ecap/ServiceRep.cc +++ b/src/adaptation/ecap/ServiceRep.cc @@ -57,11 +57,11 @@ bool Adaptation::Ecap::ServiceRep::wantsUrl(const String &urlPath) const } Adaptation::Initiate * -Adaptation::Ecap::ServiceRep::makeXactLauncher(Adaptation::Initiator *initiator, - HttpMsg *virgin, HttpRequest *cause) +Adaptation::Ecap::ServiceRep::makeXactLauncher(HttpMsg *virgin, + HttpRequest *cause) { Must(up()); - XactionRep *rep = new XactionRep(initiator, virgin, cause, Pointer(this)); + XactionRep *rep = new XactionRep(virgin, cause, Pointer(this)); XactionRep::AdapterXaction x(theService->makeXaction(rep)); rep->master(x); return rep; diff --git a/src/adaptation/ecap/ServiceRep.h b/src/adaptation/ecap/ServiceRep.h index 27046b9a98..9c858ccf26 100644 --- a/src/adaptation/ecap/ServiceRep.h +++ b/src/adaptation/ecap/ServiceRep.h @@ -33,7 +33,7 @@ public: virtual bool probed() const; virtual bool up() const; - Adaptation::Initiate *makeXactLauncher(Adaptation::Initiator *, HttpMsg *virginHeader, HttpRequest *virginCause); + Adaptation::Initiate *makeXactLauncher(HttpMsg *virginHeader, HttpRequest *virginCause); // the methods below can only be called on an up() service virtual bool wantsUrl(const String &urlPath) const; diff --git a/src/adaptation/ecap/XactionRep.cc b/src/adaptation/ecap/XactionRep.cc index d73e10d00d..3bc8bf11ac 100644 --- a/src/adaptation/ecap/XactionRep.cc +++ b/src/adaptation/ecap/XactionRep.cc @@ -14,11 +14,11 @@ CBDATA_NAMESPACED_CLASS_INIT(Adaptation::Ecap::XactionRep, XactionRep); -Adaptation::Ecap::XactionRep::XactionRep(Adaptation::Initiator *anInitiator, +Adaptation::Ecap::XactionRep::XactionRep( HttpMsg *virginHeader, HttpRequest *virginCause, const Adaptation::ServicePointer &aService): AsyncJob("Adaptation::Ecap::XactionRep"), - Adaptation::Initiate("Adaptation::Ecap::XactionRep", anInitiator), + Adaptation::Initiate("Adaptation::Ecap::XactionRep"), theService(aService), theVirginRep(virginHeader), theCauseRep(NULL), proxyingVb(opUndecided), proxyingAb(opUndecided), diff --git a/src/adaptation/ecap/XactionRep.h b/src/adaptation/ecap/XactionRep.h index 2930e9e5fd..e07cb981bc 100644 --- a/src/adaptation/ecap/XactionRep.h +++ b/src/adaptation/ecap/XactionRep.h @@ -28,7 +28,7 @@ class XactionRep : public Adaptation::Initiate, public libecap::host::Xaction, public BodyConsumer, public BodyProducer { public: - XactionRep(Adaptation::Initiator *anInitiator, HttpMsg *virginHeader, HttpRequest *virginCause, const Adaptation::ServicePointer &service); + XactionRep(HttpMsg *virginHeader, HttpRequest *virginCause, const Adaptation::ServicePointer &service); virtual ~XactionRep(); typedef libecap::shared_ptr AdapterXaction; diff --git a/src/adaptation/icap/Launcher.cc b/src/adaptation/icap/Launcher.cc index 943994b1a8..a0fc7795fa 100644 --- a/src/adaptation/icap/Launcher.cc +++ b/src/adaptation/icap/Launcher.cc @@ -15,9 +15,9 @@ Adaptation::Icap::Launcher::Launcher(const char *aTypeName, - Adaptation::Initiator *anInitiator, Adaptation::ServicePointer &aService): + Adaptation::ServicePointer &aService): AsyncJob(aTypeName), - Adaptation::Initiate(aTypeName, anInitiator), + Adaptation::Initiate(aTypeName), theService(aService), theXaction(0), theLaunches(0) { } @@ -31,7 +31,7 @@ void Adaptation::Icap::Launcher::start() { Adaptation::Initiate::start(); - Must(theInitiator); + Must(theInitiator.set()); launchXaction("first"); } @@ -47,7 +47,7 @@ void Adaptation::Icap::Launcher::launchXaction(const char *xkind) if (theLaunches >= TheConfig.repeat_limit) x->disableRepeats("over icap_retry_limit"); theXaction = initiateAdaptation(x); - Must(theXaction); + Must(initiated(theXaction)); } void Adaptation::Icap::Launcher::noteAdaptationAnswer(HttpMsg *message) @@ -76,7 +76,7 @@ void Adaptation::Icap::Launcher::noteAdaptationQueryAbort(bool final) Must(done()); // swanSong will notify the initiator } -void Adaptation::Icap::Launcher::noteXactAbort(XactAbortInfo &info) +void Adaptation::Icap::Launcher::noteXactAbort(XactAbortInfo info) { debugs(93,5, HERE << "theXaction:" << theXaction << " launches: " << theLaunches); @@ -102,10 +102,10 @@ bool Adaptation::Icap::Launcher::doneAll() const void Adaptation::Icap::Launcher::swanSong() { - if (theInitiator) + if (theInitiator.set()) tellQueryAborted(true); // always final here because abnormal - if (theXaction) + if (theXaction.set()) clearAdaptation(theXaction); Adaptation::Initiate::swanSong(); diff --git a/src/adaptation/icap/Launcher.h b/src/adaptation/icap/Launcher.h index 23ffffac52..a8f40be0f8 100644 --- a/src/adaptation/icap/Launcher.h +++ b/src/adaptation/icap/Launcher.h @@ -73,7 +73,7 @@ class XactAbortInfo; class Launcher: public Adaptation::Initiate, public Adaptation::Initiator { public: - Launcher(const char *aTypeName, Adaptation::Initiator *anInitiator, Adaptation::ServicePointer &aService); + Launcher(const char *aTypeName, Adaptation::ServicePointer &aService); virtual ~Launcher(); // Adaptation::Initiate: asynchronous communication with the initiator @@ -81,7 +81,7 @@ public: // Adaptation::Initiator: asynchronous communication with the current transaction virtual void noteAdaptationAnswer(HttpMsg *message); - virtual void noteXactAbort(XactAbortInfo &info); + virtual void noteXactAbort(XactAbortInfo info); private: bool canRetry(XactAbortInfo &info) const; //< true if can retry in the case of persistent connection failures @@ -100,7 +100,7 @@ protected: void launchXaction(const char *xkind); Adaptation::ServicePointer theService; ///< ICAP service for all launches - Adaptation::Initiate *theXaction; ///< current ICAP transaction + CbcPointer theXaction; ///< current ICAP transaction int theLaunches; // the number of transaction launches }; @@ -114,6 +114,10 @@ public: XactAbortInfo(const XactAbortInfo &); ~XactAbortInfo(); + std::ostream &print(std::ostream &os) const { + return os << isRetriable << ',' << isRepeatable; + } + HttpRequest *icapRequest; HttpReply *icapReply; bool isRetriable; @@ -123,31 +127,12 @@ private: XactAbortInfo &operator =(const XactAbortInfo &); // undefined }; -/* required by UnaryMemFunT */ -inline std::ostream &operator << (std::ostream &os, Adaptation::Icap::XactAbortInfo info) -{ - // Nothing, it is unused - return os; +inline +std::ostream & +operator <<(std::ostream &os, const XactAbortInfo &xai) { + return xai.print(os); } -/// A Dialer class used to schedule the Adaptation::Icap::Launcher::noteXactAbort call -class XactAbortCall: public UnaryMemFunT -{ -public: - typedef void (Adaptation::Icap::Launcher::*DialMethod)(Adaptation::Icap::XactAbortInfo &); - XactAbortCall(Adaptation::Icap::Launcher *launcer, DialMethod aMethod, - const Adaptation::Icap::XactAbortInfo &info): - UnaryMemFunT(launcer, NULL, info), - dialMethod(aMethod) {} - virtual void print(std::ostream &os) const { os << '(' << "retriable:" << arg1.isRetriable << ", repeatable:" << arg1.isRepeatable << ')'; } - -public: - DialMethod dialMethod; - -protected: - virtual void doDial() { (object->*dialMethod)(arg1); } -}; - } // namespace Icap } // namespace Adaptation diff --git a/src/adaptation/icap/ModXact.cc b/src/adaptation/icap/ModXact.cc index b1b0dabce3..97c917097c 100644 --- a/src/adaptation/icap/ModXact.cc +++ b/src/adaptation/icap/ModXact.cc @@ -37,10 +37,10 @@ Adaptation::Icap::ModXact::State::State() memset(this, 0, sizeof(*this)); } -Adaptation::Icap::ModXact::ModXact(Adaptation::Initiator *anInitiator, HttpMsg *virginHeader, - HttpRequest *virginCause, Adaptation::Icap::ServiceRep::Pointer &aService): +Adaptation::Icap::ModXact::ModXact(HttpMsg *virginHeader, + HttpRequest *virginCause, Adaptation::Icap::ServiceRep::Pointer &aService): AsyncJob("Adaptation::Icap::ModXact"), - Adaptation::Icap::Xaction("Adaptation::Icap::ModXact", anInitiator, aService), + Adaptation::Icap::Xaction("Adaptation::Icap::ModXact", aService), virginConsumed(0), bodyParser(NULL), canStartBypass(false), // too early @@ -95,8 +95,9 @@ void Adaptation::Icap::ModXact::waitForService() Must(!state.serviceWaiting); debugs(93, 7, HERE << "will wait for the ICAP service" << status()); state.serviceWaiting = true; - AsyncCall::Pointer call = asyncCall(93,5, "Adaptation::Icap::ModXact::noteServiceReady", - MemFun(this, &Adaptation::Icap::ModXact::noteServiceReady)); + typedef NullaryMemFunT Dialer; + AsyncCall::Pointer call = JobCallback(93,5, + Dialer, this, Adaptation::Icap::ModXact::noteServiceReady); service().callWhenReady(call); } @@ -1808,9 +1809,9 @@ bool Adaptation::Icap::ModXact::fillVirginHttpHeader(MemBuf &mb) const /* Adaptation::Icap::ModXactLauncher */ -Adaptation::Icap::ModXactLauncher::ModXactLauncher(Adaptation::Initiator *anInitiator, HttpMsg *virginHeader, HttpRequest *virginCause, Adaptation::ServicePointer aService): +Adaptation::Icap::ModXactLauncher::ModXactLauncher(HttpMsg *virginHeader, HttpRequest *virginCause, Adaptation::ServicePointer aService): AsyncJob("Adaptation::Icap::ModXactLauncher"), - Adaptation::Icap::Launcher("Adaptation::Icap::ModXactLauncher", anInitiator, aService) + Adaptation::Icap::Launcher("Adaptation::Icap::ModXactLauncher", aService) { virgin.setHeader(virginHeader); virgin.setCause(virginCause); @@ -1822,7 +1823,7 @@ Adaptation::Icap::Xaction *Adaptation::Icap::ModXactLauncher::createXaction() Adaptation::Icap::ServiceRep::Pointer s = dynamic_cast(theService.getRaw()); Must(s != NULL); - return new Adaptation::Icap::ModXact(this, virgin.header, virgin.cause, s); + return new Adaptation::Icap::ModXact(virgin.header, virgin.cause, s); } void Adaptation::Icap::ModXactLauncher::swanSong() diff --git a/src/adaptation/icap/ModXact.h b/src/adaptation/icap/ModXact.h index d29e89a87e..61bf8130af 100644 --- a/src/adaptation/icap/ModXact.h +++ b/src/adaptation/icap/ModXact.h @@ -136,7 +136,7 @@ class ModXact: public Xaction, public BodyProducer, public BodyConsumer { public: - ModXact(Adaptation::Initiator *anInitiator, HttpMsg *virginHeader, HttpRequest *virginCause, ServiceRep::Pointer &s); + ModXact(HttpMsg *virginHeader, HttpRequest *virginCause, ServiceRep::Pointer &s); // BodyProducer methods virtual void noteMoreBodySpaceAvailable(BodyPipe::Pointer); @@ -161,7 +161,6 @@ public: InOut virgin; InOut adapted; -protected: // bypasses exceptions if needed and possible virtual void callException(const std::exception &e); @@ -341,7 +340,7 @@ private: class ModXactLauncher: public Launcher { public: - ModXactLauncher(Adaptation::Initiator *anInitiator, HttpMsg *virginHeader, HttpRequest *virginCause, Adaptation::ServicePointer s); + ModXactLauncher(HttpMsg *virginHeader, HttpRequest *virginCause, Adaptation::ServicePointer s); protected: virtual Xaction *createXaction(); diff --git a/src/adaptation/icap/OptXact.cc b/src/adaptation/icap/OptXact.cc index 9305971825..6903ca9555 100644 --- a/src/adaptation/icap/OptXact.cc +++ b/src/adaptation/icap/OptXact.cc @@ -17,9 +17,9 @@ CBDATA_NAMESPACED_CLASS_INIT(Adaptation::Icap, OptXact); CBDATA_NAMESPACED_CLASS_INIT(Adaptation::Icap, OptXactLauncher); -Adaptation::Icap::OptXact::OptXact(Adaptation::Initiator *anInitiator, Adaptation::Icap::ServiceRep::Pointer &aService): +Adaptation::Icap::OptXact::OptXact(Adaptation::Icap::ServiceRep::Pointer &aService): AsyncJob("Adaptation::Icap::OptXact"), - Adaptation::Icap::Xaction("Adaptation::Icap::OptXact", anInitiator, aService) + Adaptation::Icap::Xaction("Adaptation::Icap::OptXact", aService) { } @@ -118,9 +118,9 @@ void Adaptation::Icap::OptXact::finalizeLogInfo() /* Adaptation::Icap::OptXactLauncher */ -Adaptation::Icap::OptXactLauncher::OptXactLauncher(Adaptation::Initiator *anInitiator, Adaptation::ServicePointer aService): +Adaptation::Icap::OptXactLauncher::OptXactLauncher(Adaptation::ServicePointer aService): AsyncJob("Adaptation::Icap::OptXactLauncher"), - Adaptation::Icap::Launcher("Adaptation::Icap::OptXactLauncher", anInitiator, aService) + Adaptation::Icap::Launcher("Adaptation::Icap::OptXactLauncher", aService) { } @@ -129,5 +129,5 @@ Adaptation::Icap::Xaction *Adaptation::Icap::OptXactLauncher::createXaction() Adaptation::Icap::ServiceRep::Pointer s = dynamic_cast(theService.getRaw()); Must(s != NULL); - return new Adaptation::Icap::OptXact(this, s); + return new Adaptation::Icap::OptXact(s); } diff --git a/src/adaptation/icap/OptXact.h b/src/adaptation/icap/OptXact.h index c2119d5d34..35561996dc 100644 --- a/src/adaptation/icap/OptXact.h +++ b/src/adaptation/icap/OptXact.h @@ -51,7 +51,7 @@ class OptXact: public Xaction { public: - OptXact(Adaptation::Initiator *anInitiator, ServiceRep::Pointer &aService); + OptXact(ServiceRep::Pointer &aService); protected: virtual void start(); @@ -76,7 +76,7 @@ private: class OptXactLauncher: public Launcher { public: - OptXactLauncher(Adaptation::Initiator *anInitiator, Adaptation::ServicePointer aService); + OptXactLauncher(Adaptation::ServicePointer aService); protected: virtual Xaction *createXaction(); diff --git a/src/adaptation/icap/ServiceRep.cc b/src/adaptation/icap/ServiceRep.cc index cba32c45df..806a6f3344 100644 --- a/src/adaptation/icap/ServiceRep.cc +++ b/src/adaptation/icap/ServiceRep.cc @@ -147,7 +147,7 @@ void Adaptation::Icap::ServiceRep::noteTimeToUpdate() if (!detached()) updateScheduled = false; - if (detached() || theOptionsFetcher) { + if (detached() || theOptionsFetcher.set()) { debugs(93,5, HERE << "ignores options update " << status()); return; } @@ -200,7 +200,7 @@ void Adaptation::Icap::ServiceRep::callWhenReady(AsyncCall::Pointer &cb) i.callback = cb; theClients.push_back(i); - if (theOptionsFetcher || notifying) + if (theOptionsFetcher.set() || notifying) return; // do nothing, we will be picked up in noteTimeToNotify() if (needNewOptions()) @@ -212,7 +212,7 @@ void Adaptation::Icap::ServiceRep::callWhenReady(AsyncCall::Pointer &cb) void Adaptation::Icap::ServiceRep::scheduleNotification() { debugs(93,7, HERE << "will notify " << theClients.size() << " clients"); - CallJobHere(93, 5, this, Adaptation::Icap::ServiceRep::noteTimeToNotify); + CallJobHere(93, 5, this, Adaptation::Icap::ServiceRep, noteTimeToNotify); } bool Adaptation::Icap::ServiceRep::needNewOptions() const @@ -306,7 +306,7 @@ void Adaptation::Icap::ServiceRep::announceStatusChange(const char *downPhrase, // we are receiving ICAP OPTIONS response headers here or NULL on failures void Adaptation::Icap::ServiceRep::noteAdaptationAnswer(HttpMsg *msg) { - Must(theOptionsFetcher); + Must(initiated(theOptionsFetcher)); clearAdaptation(theOptionsFetcher); Must(msg); @@ -326,13 +326,23 @@ void Adaptation::Icap::ServiceRep::noteAdaptationAnswer(HttpMsg *msg) void Adaptation::Icap::ServiceRep::noteAdaptationQueryAbort(bool) { - Must(theOptionsFetcher); + Must(initiated(theOptionsFetcher)); clearAdaptation(theOptionsFetcher); debugs(93,3, HERE << "failed to fetch options " << status()); handleNewOptions(0); } +// we (a) must keep trying to get OPTIONS and (b) are RefCounted so we +// must keep our job alive (XXX: until nobody needs us) +void Adaptation::Icap::ServiceRep::callException(const std::exception &e) +{ + clearAdaptation(theOptionsFetcher); + debugs(93,2, "ICAP probably failed to fetch options (" << e.what() << + ")" << status()); + handleNewOptions(0); +} + void Adaptation::Icap::ServiceRep::handleNewOptions(Adaptation::Icap::Options *newOptions) { // new options may be NULL @@ -349,9 +359,9 @@ void Adaptation::Icap::ServiceRep::startGettingOptions() Must(!theOptionsFetcher); debugs(93,6, HERE << "will get new options " << status()); - // XXX: second "this" is "self"; this works but may stop if API changes - theOptionsFetcher = initiateAdaptation(new Adaptation::Icap::OptXactLauncher(this, this)); - Must(theOptionsFetcher); + // XXX: "this" here is "self"; works until refcounting API changes + theOptionsFetcher = initiateAdaptation( + new Adaptation::Icap::OptXactLauncher(this)); // TODO: timeout in case Adaptation::Icap::OptXact never calls us back? // Such a timeout should probably be a generic AsyncStart feature. } @@ -418,10 +428,10 @@ Adaptation::Icap::ServiceRep::optionsFetchTime() const } Adaptation::Initiate * -Adaptation::Icap::ServiceRep::makeXactLauncher(Adaptation::Initiator *initiator, - HttpMsg *virgin, HttpRequest *cause) +Adaptation::Icap::ServiceRep::makeXactLauncher(HttpMsg *virgin, + HttpRequest *cause) { - return new Adaptation::Icap::ModXactLauncher(initiator, virgin, cause, this); + return new Adaptation::Icap::ModXactLauncher(virgin, cause, this); } // returns a temporary string depicting service status, for debugging @@ -450,7 +460,7 @@ const char *Adaptation::Icap::ServiceRep::status() const if (detached()) buf.append(",detached", 9); - if (theOptionsFetcher) + if (theOptionsFetcher.set()) buf.append(",fetch", 6); if (notifying) diff --git a/src/adaptation/icap/ServiceRep.h b/src/adaptation/icap/ServiceRep.h index 45b32b7fe5..fa67bde23a 100644 --- a/src/adaptation/icap/ServiceRep.h +++ b/src/adaptation/icap/ServiceRep.h @@ -95,7 +95,7 @@ public: virtual bool probed() const; // see comments above virtual bool up() const; // see comments above - virtual Adaptation::Initiate *makeXactLauncher(Adaptation::Initiator *, HttpMsg *virginHeader, HttpRequest *virginCause); + virtual Initiate *makeXactLauncher(HttpMsg *virginHeader, HttpRequest *virginCause); void callWhenReady(AsyncCall::Pointer &cb); @@ -109,6 +109,7 @@ public: //AsyncJob virtual methods virtual bool doneAll() const { return Adaptation::Initiator::doneAll() && false;} + virtual void callException(const std::exception &e); virtual void detach(); virtual bool detached() const; @@ -133,7 +134,7 @@ private: Clients theClients; // all clients waiting for a call back Options *theOptions; - Adaptation::Initiate *theOptionsFetcher; // pending ICAP OPTIONS transaction + CbcPointer theOptionsFetcher; // pending ICAP OPTIONS transaction time_t theLastUpdate; // time the options were last updated FadingCounter theSessionFailures; diff --git a/src/adaptation/icap/Xaction.cc b/src/adaptation/icap/Xaction.cc index 42cb0fcb27..e8bc9b0cb4 100644 --- a/src/adaptation/icap/Xaction.cc +++ b/src/adaptation/icap/Xaction.cc @@ -24,9 +24,10 @@ static PconnPool *icapPconnPool = new PconnPool("ICAP Servers"); //CBDATA_NAMESPACED_CLASS_INIT(Adaptation::Icap, Xaction); -Adaptation::Icap::Xaction::Xaction(const char *aTypeName, Adaptation::Initiator *anInitiator, Adaptation::Icap::ServiceRep::Pointer &aService): +Adaptation::Icap::Xaction::Xaction(const char *aTypeName, + Adaptation::Icap::ServiceRep::Pointer &aService): AsyncJob(aTypeName), - Adaptation::Initiate(aTypeName, anInitiator), + Adaptation::Initiate(aTypeName), icapRequest(NULL), icapReply(NULL), attempts(0), @@ -105,7 +106,8 @@ void Adaptation::Icap::Xaction::openConnection() // fake the connect callback // TODO: can we sync call Adaptation::Icap::Xaction::noteCommConnected here instead? typedef CommCbMemFunT Dialer; - Dialer dialer(this, &Adaptation::Icap::Xaction::noteCommConnected); + CbcPointer self(this); + Dialer dialer(self, &Adaptation::Icap::Xaction::noteCommConnected); dialer.params.fd = connection; dialer.params.flag = COMM_OK; // fake other parameters by copying from the existing connection @@ -136,20 +138,19 @@ void Adaptation::Icap::Xaction::openConnection() // TODO: service bypass status may differ from that of a transaction typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(93, 5, "Adaptation::Icap::Xaction::noteCommTimedout", - TimeoutDialer(this,&Adaptation::Icap::Xaction::noteCommTimedout)); - + AsyncCall::Pointer timeoutCall = JobCallback(93, 5, + TimeoutDialer, this, Adaptation::Icap::Xaction::noteCommTimedout); commSetTimeout(connection, TheConfig.connect_timeout( service().cfg().bypass), timeoutCall); typedef CommCbMemFunT CloseDialer; - closer = asyncCall(93, 5, "Adaptation::Icap::Xaction::noteCommClosed", - CloseDialer(this,&Adaptation::Icap::Xaction::noteCommClosed)); + closer = JobCallback(93, 5, + CloseDialer, this, Adaptation::Icap::Xaction::noteCommClosed); comm_add_close_handler(connection, closer); typedef CommCbMemFunT ConnectDialer; - connector = asyncCall(93,3, "Adaptation::Icap::Xaction::noteCommConnected", - ConnectDialer(this, &Adaptation::Icap::Xaction::noteCommConnected)); + connector = JobCallback(93,3, + ConnectDialer, this, Adaptation::Icap::Xaction::noteCommConnected); commConnectStart(connection, s.cfg().host.termedBuf(), s.cfg().port, connector); } @@ -232,8 +233,8 @@ void Adaptation::Icap::Xaction::scheduleWrite(MemBuf &buf) { // comm module will free the buffer typedef CommCbMemFunT Dialer; - writer = asyncCall(93,3, "Adaptation::Icap::Xaction::noteCommWrote", - Dialer(this, &Adaptation::Icap::Xaction::noteCommWrote)); + writer = JobCallback(93,3, + Dialer, this, Adaptation::Icap::Xaction::noteCommWrote); comm_write_mbuf(connection, &buf, writer); updateTimeout(); @@ -314,8 +315,8 @@ void Adaptation::Icap::Xaction::updateTimeout() // XXX: why does Config.Timeout lacks a write timeout? // TODO: service bypass status may differ from that of a transaction typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer call = asyncCall(93, 5, "Adaptation::Icap::Xaction::noteCommTimedout", - TimeoutDialer(this,&Adaptation::Icap::Xaction::noteCommTimedout)); + AsyncCall::Pointer call = JobCallback(93,5, + TimeoutDialer, this, Adaptation::Icap::Xaction::noteCommTimedout); commSetTimeout(connection, TheConfig.io_timeout(service().cfg().bypass), call); @@ -338,8 +339,8 @@ void Adaptation::Icap::Xaction::scheduleRead() * here instead of reading directly into readBuf.buf. */ typedef CommCbMemFunT Dialer; - reader = asyncCall(93,3, "Adaptation::Icap::Xaction::noteCommRead", - Dialer(this, &Adaptation::Icap::Xaction::noteCommRead)); + reader = JobCallback(93,3, + Dialer, this, Adaptation::Icap::Xaction::noteCommRead); comm_read(connection, commBuf, readBuf.spaceSize(), reader); updateTimeout(); @@ -429,7 +430,7 @@ bool Adaptation::Icap::Xaction::doneWithIo() const void Adaptation::Icap::Xaction::noteInitiatorAborted() { - if (theInitiator) { + if (theInitiator.set()) { clearInitiator(); mustStop("initiator aborted"); } @@ -462,8 +463,7 @@ void Adaptation::Icap::Xaction::swanSong() if (commBuf) memFreeBuf(commBufSize, commBuf); - if (theInitiator) - tellQueryAborted(); + tellQueryAborted(); maybeLog(); @@ -472,12 +472,15 @@ void Adaptation::Icap::Xaction::swanSong() void Adaptation::Icap::Xaction::tellQueryAborted() { - Adaptation::Icap::Launcher *l = dynamic_cast(theInitiator.ptr()); - Adaptation::Icap::XactAbortInfo abortInfo(icapRequest, icapReply, retriable(), repeatable()); - CallJob(91, 5, __FILE__, __LINE__, - "Adaptation::Icap::Launcher::noteXactAbort", - XactAbortCall(l, &Adaptation::Icap::Launcher::noteXactAbort, abortInfo) ); - clearInitiator(); + if (theInitiator.set()) { + Adaptation::Icap::XactAbortInfo abortInfo(icapRequest, icapReply, + retriable(), repeatable()); + Launcher *launcher = dynamic_cast(theInitiator.get()); + // launcher may be nil if initiator is invalid + CallJobHere1(91,5, CbcPointer(launcher), + Launcher, noteXactAbort, abortInfo); + clearInitiator(); + } } diff --git a/src/adaptation/icap/Xaction.h b/src/adaptation/icap/Xaction.h index 3229916019..4e81dfca91 100644 --- a/src/adaptation/icap/Xaction.h +++ b/src/adaptation/icap/Xaction.h @@ -63,7 +63,7 @@ class Xaction: public Adaptation::Initiate { public: - Xaction(const char *aTypeName, Adaptation::Initiator *anInitiator, ServiceRep::Pointer &aService); + Xaction(const char *aTypeName, ServiceRep::Pointer &aService); virtual ~Xaction(); void disableRetries(); @@ -125,10 +125,12 @@ protected: // useful for debugging virtual bool fillVirginHttpHeader(MemBuf&) const; +public: // custom exception handling and end-of-call checks virtual void callException(const std::exception &e); virtual void callEnd(); +protected: // logging void setOutcome(const XactOutcome &xo); virtual void finalizeLogInfo(); diff --git a/src/base/AsyncJob.cc b/src/base/AsyncJob.cc index 11efc212ad..ab8b0ac819 100644 --- a/src/base/AsyncJob.cc +++ b/src/base/AsyncJob.cc @@ -5,6 +5,7 @@ #include "squid.h" #include "base/AsyncCall.h" #include "base/AsyncJob.h" +#include "base/AsyncJobCalls.h" #include "base/TextException.h" #include "cbdata.h" #include "MemBuf.h" @@ -12,10 +13,10 @@ unsigned int AsyncJob::TheLastId = 0; -AsyncJob *AsyncJob::AsyncStart(AsyncJob *job) +AsyncJob::Pointer AsyncJob::Start(AsyncJob *j) { - assert(job); - CallJobHere(93, 5, job, AsyncJob::noteStart); + AsyncJob::Pointer job(j); + CallJobHere(93, 5, job, AsyncJob, start); return job; } @@ -29,11 +30,6 @@ AsyncJob::~AsyncJob() { } -void AsyncJob::noteStart() -{ - start(); -} - void AsyncJob::start() { } @@ -52,8 +48,9 @@ void AsyncJob::deleteThis(const char *aReason) // there is no call wrapper waiting for our return, so we fake it debugs(93, 5, typeName << " will delete this, reason: " << stopReason); + CbcPointer self(this); AsyncCall::Pointer fakeCall = asyncCall(93,4, "FAKE-deleteThis", - MemFun(this, &AsyncJob::deleteThis, aReason)); + JobMemFun(self, &AsyncJob::deleteThis, aReason)); inCall = fakeCall; callEnd(); // delete fakeCall; @@ -164,60 +161,3 @@ const char *AsyncJob::status() const } -/* JobDialer */ - -JobDialer::JobDialer(AsyncJob *aJob): job(NULL), lock(NULL) -{ - if (aJob) { - lock = cbdataReference(aJob->toCbdata()); - job = aJob; - } -} - -JobDialer::JobDialer(const JobDialer &d): CallDialer(d), - job(NULL), lock(NULL) -{ - if (d.lock && cbdataReferenceValid(d.lock)) { - lock = cbdataReference(d.lock); - Must(d.job); - job = d.job; - } -} - -JobDialer::~JobDialer() -{ - cbdataReferenceDone(lock); // lock may be NULL -} - - -bool -JobDialer::canDial(AsyncCall &call) -{ - if (!lock) - return call.cancel("job was gone before the call"); - - if (!cbdataReferenceValid(lock)) - return call.cancel("job gone after the call"); - - Must(job); - return job->canBeCalled(call); -} - -void -JobDialer::dial(AsyncCall &call) -{ - Must(lock && cbdataReferenceValid(lock)); // canDial() checks for this - Must(job); - - job->callStart(call); - - try { - doDial(); - } catch (const std::exception &e) { - debugs(call.debugSection, 3, - HERE << call.name << " threw exception: " << e.what()); - job->callException(e); - } - - job->callEnd(); // may delete job -} diff --git a/src/base/AsyncJob.h b/src/base/AsyncJob.h index a58dc6dca6..df0bf3ca54 100644 --- a/src/base/AsyncJob.h +++ b/src/base/AsyncJob.h @@ -7,6 +7,9 @@ #include "base/AsyncCall.h" +template +class CbcPointer; + /** \defgroup AsyncJobAPI Async-Jobs API \par @@ -18,18 +21,20 @@ // See AsyncJobs.dox for details. /// \ingroup AsyncJobAPI +/// Base class for all asynchronous jobs class AsyncJob { - public: - /// starts the job (i.e., makes the job asynchronous) - static AsyncJob *AsyncStart(AsyncJob *job); + typedef CbcPointer Pointer; +public: AsyncJob(const char *aTypeName); virtual ~AsyncJob(); virtual void *toCbdata() = 0; - void noteStart(); // calls virtual start + + /// starts a freshly created job (i.e., makes the job asynchronous) + static Pointer Start(AsyncJob *job); protected: // XXX: temporary method to replace "delete this" in jobs-in-transition. @@ -64,56 +69,4 @@ private: static unsigned int TheLastId; ///< makes job IDs unique until it wraps }; - -/** - \ingroup AsyncJobAPI - * This is a base class for all job call dialers. It does all the job - * dialing logic (debugging, handling exceptions, etc.) except for calling - * the job method. The latter is not possible without templates and we - * want to keep this class simple and template-free. Thus, we add a dial() - * virtual method that the JobCallT template below will implement for us, - * calling the job. - */ -class JobDialer: public CallDialer -{ -public: - JobDialer(AsyncJob *aJob); - JobDialer(const JobDialer &d); - virtual ~JobDialer(); - - virtual bool canDial(AsyncCall &call); - void dial(AsyncCall &call); - - AsyncJob *job; - void *lock; // job's cbdata - -protected: - virtual void doDial() = 0; // actually calls the job method - -private: - // not implemented and should not be needed - JobDialer &operator =(const JobDialer &); -}; - -#include "base/AsyncJobCalls.h" - -template -bool -CallJob(int debugSection, int debugLevel, const char *fileName, int fileLine, - const char *callName, const Dialer &dialer) -{ - AsyncCall::Pointer call = asyncCall(debugSection, debugLevel, callName, dialer); - return ScheduleCall(fileName, fileLine, call); -} - - -#define CallJobHere(debugSection, debugLevel, job, method) \ - CallJob((debugSection), (debugLevel), __FILE__, __LINE__, #method, \ - MemFun((job), &method)) - -#define CallJobHere1(debugSection, debugLevel, job, method, arg1) \ - CallJob((debugSection), (debugLevel), __FILE__, __LINE__, #method, \ - MemFun((job), &method, (arg1))) - - #endif /* SQUID_ASYNC_JOB_H */ diff --git a/src/base/AsyncJobCalls.h b/src/base/AsyncJobCalls.h index 0657d36720..1d6ebe8d9c 100644 --- a/src/base/AsyncJobCalls.h +++ b/src/base/AsyncJobCalls.h @@ -7,6 +7,66 @@ #define SQUID_ASYNCJOBCALLS_H #include "base/AsyncJob.h" +#include "base/CbcPointer.h" + +/** + \ingroup AsyncJobAPI + * This is a base class for all job call dialers. It does all the job + * dialing logic (debugging, handling exceptions, etc.) except for calling + * the job method. The latter requires knowing the number and type of method + * parameters. Thus, we add a dial() virtual method that the MemFunT templates + * below implement for us, calling the job's method with the right params. + */ +template +class JobDialer: public CallDialer +{ +public: + typedef Job DestClass; + typedef CbcPointer JobPointer; + + JobDialer(const JobPointer &aJob); + JobDialer(const JobDialer &d); + + virtual bool canDial(AsyncCall &call); + void dial(AsyncCall &call); + + JobPointer job; + +protected: + virtual void doDial() = 0; // actually calls the job method + +private: + // not implemented and should not be needed + JobDialer &operator =(const JobDialer &); +}; + +/// schedule an async job call using a dialer; use CallJobHere macros instead +template +bool +CallJob(int debugSection, int debugLevel, const char *fileName, int fileLine, + const char *callName, const Dialer &dialer) +{ + AsyncCall::Pointer call = asyncCall(debugSection, debugLevel, callName, dialer); + return ScheduleCall(fileName, fileLine, call); +} + + +#define CallJobHere(debugSection, debugLevel, job, Class, method) \ + CallJob((debugSection), (debugLevel), __FILE__, __LINE__, \ + (#Class "::" #method), \ + JobMemFun((job), &Class::method)) + +#define CallJobHere1(debugSection, debugLevel, job, Class, method, arg1) \ + CallJob((debugSection), (debugLevel), __FILE__, __LINE__, \ + (#Class "::" #method), \ + JobMemFun((job), &Class::method, (arg1))) + + +/// Convenience macro to create a Dialer-based job callback +#define JobCallback(dbgSection, dbgLevel, Dialer, job, method) \ + asyncCall((dbgSection), (dbgLevel), #method, \ + Dialer(CbcPointer(job), &method)) + /* * *MemFunT are member function (i.e., class method) wrappers. They store @@ -24,42 +84,40 @@ // Arity names are from http://en.wikipedia.org/wiki/Arity -template -class NullaryMemFunT: public JobDialer +template +class NullaryMemFunT: public JobDialer { public: - typedef void (C::*Method)(); - explicit NullaryMemFunT(C *anObject, Method aMethod): - JobDialer(anObject), object(anObject), method(aMethod) {} + typedef void (Job::*Method)(); + explicit NullaryMemFunT(const CbcPointer &aJob, Method aMethod): + JobDialer(aJob), method(aMethod) {} virtual void print(std::ostream &os) const { os << "()"; } public: - C *object; Method method; protected: - virtual void doDial() { (object->*method)(); } + virtual void doDial() { ((&(*this->job))->*method)(); } }; -template -class UnaryMemFunT: public JobDialer +template +class UnaryMemFunT: public JobDialer { public: - typedef void (C::*Method)(Argument1); - explicit UnaryMemFunT(C *anObject, Method aMethod, const Argument1 &anArg1): - JobDialer(anObject), - object(anObject), method(aMethod), arg1(anArg1) {} + typedef void (Job::*Method)(Argument1); + explicit UnaryMemFunT(const CbcPointer &aJob, Method aMethod, + const Argument1 &anArg1): JobDialer(aJob), + method(aMethod), arg1(anArg1) {} virtual void print(std::ostream &os) const { os << '(' << arg1 << ')'; } public: - C *object; Method method; Argument1 arg1; protected: - virtual void doDial() { (object->*method)(arg1); } + virtual void doDial() { ((&(*this->job))->*method)(arg1); } }; // ... add more as needed @@ -71,17 +129,57 @@ protected: template NullaryMemFunT -MemFun(C *object, typename NullaryMemFunT::Method method) +JobMemFun(const CbcPointer &job, typename NullaryMemFunT::Method method) { - return NullaryMemFunT(object, method); + return NullaryMemFunT(job, method); } template UnaryMemFunT -MemFun(C *object, typename UnaryMemFunT::Method method, +JobMemFun(const CbcPointer &job, typename UnaryMemFunT::Method method, Argument1 arg1) { - return UnaryMemFunT(object, method, arg1); + return UnaryMemFunT(job, method, arg1); +} + + +// inlined methods + +template +JobDialer::JobDialer(const JobPointer &aJob): job(aJob) +{ +} + +template +JobDialer::JobDialer(const JobDialer &d): CallDialer(d), job(d.job) +{ +} + +template +bool +JobDialer::canDial(AsyncCall &call) +{ + if (!job) + return call.cancel("job gone"); + + return job->canBeCalled(call); +} + +template +void +JobDialer::dial(AsyncCall &call) +{ + job->callStart(call); + + try { + doDial(); + } catch (const std::exception &e) { + debugs(call.debugSection, 3, + HERE << call.name << " threw exception: " << e.what()); + job->callException(e); + } + + job->callEnd(); // may delete job } #endif /* SQUID_ASYNCJOBCALLS_H */ diff --git a/src/base/AsyncJobs.dox b/src/base/AsyncJobs.dox index 296ed81f4d..c8f72c2d7b 100644 --- a/src/base/AsyncJobs.dox +++ b/src/base/AsyncJobs.dox @@ -6,15 +6,15 @@ - \b Job: an AsyncJob object. - \b Creator: the code creating the job. Usually the Initiator. -- \b Start: the act of calling AsyncStart with a job pointer. +- \b Start: the act of calling AsyncJob::Start with a job pointer. - \b Initiator: the code starting the job. Usually the Creator. \section Life Typical life cycle -# Creator creates and initializes a job. --# Initiator starts the job. If Initiator expects -to communicate with the started job, then it stores the job pointer -returned by AsyncStart. +-# If Initiator expects to communicate with the job after start, + then it stores the job pointer +-# Initiator starts the job by calling AsyncJob::Start. -# The job's start() method is called. The method usually schedules some I/O or registers to receive some other callbacks. -# The job runs and does what it is supposed to do. This usually involves @@ -27,7 +27,7 @@ then notifying Initiator of the final result. If you want to do something before starting the job, do it in the constructor or some custom method that the job creator will call _before_ calling -AsyncStart(): +AsyncJob::Start(): std::auto_ptr job(new MyJob(...)); // sync/blocking job->prepare(...); // sync/blocking @@ -36,15 +36,16 @@ AsyncStart(): If you do not need complex preparations, it is better to do this instead: - AsyncStart(new MyJob(...)); + AsyncJob::Start(new MyJob(...)); Keep in mind that you have no async debugging, cleanup, and protections until -you call AsyncStart with a job pointer. +you call AsyncJob::Start with a job pointer. \section Rules Basic rules -- To start a job, use AsyncStart. Do not start the same job more than once. +- To start a job, use AsyncJob::Start. + Do not start the same job more than once. - Never call start() directly. Treat this method as main() in C/C++. diff --git a/src/base/CbcPointer.h b/src/base/CbcPointer.h new file mode 100644 index 0000000000..62a9463b86 --- /dev/null +++ b/src/base/CbcPointer.h @@ -0,0 +1,156 @@ +/* + * $Id$ + */ + +#ifndef SQUID_CBC_POINTER_H +#define SQUID_CBC_POINTER_H + +#include "base/TextException.h" +#include "cbdata.h" + +/** + \ingroup CBDATAAPI + * + * Safely points to a cbdata-protected class (cbc), such as an AsyncJob. + * When a cbc we communicate with disappears without + * notice or a notice has not reached us yet, this class prevents + * dereferencing the pointer to the gone cbc object. + */ +template +class CbcPointer +{ +public: + CbcPointer(); // a nil pointer + CbcPointer(Cbc *aCbc); + CbcPointer(const CbcPointer &p); + ~CbcPointer(); + + Cbc *raw() const; ///< a temporary raw Cbc pointer; may be invalid + Cbc *get() const; ///< a temporary valid raw Cbc pointer or NULL + Cbc &operator *() const; ///< a valid Cbc reference or exception + Cbc *operator ->() const; ///< a valid Cbc pointer or exception + + // no bool operator because set() != valid() + bool set() const { return cbc != NULL; } ///< was set but may be invalid + Cbc *valid() const { return get(); } ///< was set and is valid + bool operator !() const { return !valid(); } ///< invalid or was not set + bool operator ==(const CbcPointer &o) const { return lock == o.lock; } + + CbcPointer &operator =(const CbcPointer &p); + + /// support converting a child cbc pointer into a parent cbc pointer + template + CbcPointer(const CbcPointer &o): cbc(o.raw()), lock(NULL) { + if (o.valid()) + lock = cbdataReference(o->toCbdata()); + } + + /// support assigning a child cbc pointer to a parent cbc pointer + template + CbcPointer &operator =(const CbcPointer &o) { + clear(); + cbc = o.raw(); // so that set() is accurate + if (o.valid()) + lock = cbdataReference(o->toCbdata()); + return *this; + } + + void clear(); ///< make pointer not set; does not invalidate cbdata + + std::ostream &print(std::ostream &os) const; + +private: + Cbc *cbc; // a possibly invalid pointer to a cbdata class + void *lock; // a valid pointer to cbc's cbdata or nil +}; + +template +inline +std::ostream &operator <<(std::ostream &os, const CbcPointer &p) { + return p.print(os); +} + +// inlined methods + +template +CbcPointer::CbcPointer(): cbc(NULL), lock(NULL) +{ +} + +template +CbcPointer::CbcPointer(Cbc *aCbc): cbc(aCbc), lock(NULL) +{ + if (cbc) + lock = cbdataReference(cbc->toCbdata()); +} + +template +CbcPointer::CbcPointer(const CbcPointer &d): cbc(d.cbc), lock(NULL) +{ + if (d.lock && cbdataReferenceValid(d.lock)) + lock = cbdataReference(d.lock); +} + +template +CbcPointer::~CbcPointer() +{ + clear(); +} + +template +CbcPointer &CbcPointer::operator =(const CbcPointer &d) +{ + clear(); + cbc = d.cbc; + if (d.lock && cbdataReferenceValid(d.lock)) + lock = cbdataReference(d.lock); + return *this; +} + +template +void +CbcPointer::clear() +{ + cbdataReferenceDone(lock); // lock may be nil before and will be nil after + cbc = NULL; +} + +template +Cbc * +CbcPointer::raw() const +{ + return cbc; +} + +template +Cbc * +CbcPointer::get() const +{ + return (lock && cbdataReferenceValid(lock)) ? cbc : NULL; +} + +template +Cbc & +CbcPointer::operator *() const +{ + Cbc *c = get(); + Must(c); + return *c; +} + +template +Cbc * +CbcPointer::operator ->() const +{ + Cbc *c = get(); + Must(c); + return c; +} + +template +std::ostream &CbcPointer::print(std::ostream &os) const { + return os << cbc << '/' << lock; +} + + +#endif /* SQUID_CBC_POINTER_H */ diff --git a/src/base/Makefile.am b/src/base/Makefile.am index 0e1d4a96da..7009fb37be 100644 --- a/src/base/Makefile.am +++ b/src/base/Makefile.am @@ -12,5 +12,6 @@ libbase_la_SOURCES = \ AsyncJobCalls.h \ AsyncCallQueue.cc \ AsyncCallQueue.h \ + CbcPointer.h \ TextException.cc \ TextException.h diff --git a/src/client_side.cc b/src/client_side.cc index dc3306b40b..11c78c3ca1 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -242,8 +242,8 @@ ConnStateData::readSomeData() makeSpaceAvailable(); typedef CommCbMemFunT Dialer; - reader = asyncCall(33, 5, "ConnStateData::clientReadRequest", - Dialer(this, &ConnStateData::clientReadRequest)); + reader = JobCallback(33, 5, + Dialer, this, ConnStateData::clientReadRequest); comm_read(fd, in.addressToReadInto(), getAvailableBufferLength(), reader); } @@ -1397,8 +1397,8 @@ ConnStateData::readNextRequest() * Set the timeout BEFORE calling clientReadRequest(). */ typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(33, 5, "ConnStateData::requestTimeout", - TimeoutDialer(this, &ConnStateData::requestTimeout)); + AsyncCall::Pointer timeoutCall = JobCallback(33, 5, + TimeoutDialer, this, ConnStateData::requestTimeout); commSetTimeout(fd, Config.Timeout.persistent_request, timeoutCall); readSomeData(); @@ -2997,8 +2997,8 @@ ConnStateData::requestTimeout(const CommTimeoutCbParams &io) * if we don't close() here, we still need a timeout handler! */ typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(33, 5, "ConnStateData::requestTimeout", - TimeoutDialer(this,&ConnStateData::requestTimeout)); + AsyncCall::Pointer timeoutCall = JobCallback(33, 5, + TimeoutDialer, this, ConnStateData::requestTimeout); commSetTimeout(io.fd, 30, timeoutCall); /* @@ -3097,16 +3097,16 @@ httpAccept(int sock, int newfd, ConnectionDetail *details, connState = connStateCreate(&details->peer, &details->me, newfd, s); typedef CommCbMemFunT Dialer; - AsyncCall::Pointer call = asyncCall(33, 5, "ConnStateData::connStateClosed", - Dialer(connState, &ConnStateData::connStateClosed)); + AsyncCall::Pointer call = JobCallback(33, 5, + Dialer, connState, ConnStateData::connStateClosed); comm_add_close_handler(newfd, call); if (Config.onoff.log_fqdn) fqdncache_gethostbyaddr(details->peer, FQDN_LOOKUP_IF_MISS); typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(33, 5, "ConnStateData::requestTimeout", - TimeoutDialer(connState,&ConnStateData::requestTimeout)); + AsyncCall::Pointer timeoutCall = JobCallback(33, 5, + TimeoutDialer, connState, ConnStateData::requestTimeout); commSetTimeout(newfd, Config.Timeout.read, timeoutCall); #if USE_IDENT @@ -3308,16 +3308,16 @@ httpsAccept(int sock, int newfd, ConnectionDetail *details, ConnStateData *connState = connStateCreate(details->peer, details->me, newfd, &s->http); typedef CommCbMemFunT Dialer; - AsyncCall::Pointer call = asyncCall(33, 5, "ConnStateData::connStateClosed", - Dialer(connState, &ConnStateData::connStateClosed)); + AsyncCall::Pointer call = JobCallback(33, 5, + Dialer, connState, ConnStateData::connStateClosed); comm_add_close_handler(newfd, call); if (Config.onoff.log_fqdn) fqdncache_gethostbyaddr(details->peer, FQDN_LOOKUP_IF_MISS); typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(33, 5, "ConnStateData::requestTimeout", - TimeoutDialer(connState,&ConnStateData::requestTimeout)); + AsyncCall::Pointer timeoutCall = JobCallback(33, 5, + TimeoutDialer, connState, ConnStateData::requestTimeout); commSetTimeout(newfd, Config.Timeout.request, timeoutCall); #if USE_IDENT @@ -3896,8 +3896,8 @@ void ConnStateData::pinConnection(int pinning_fd, HttpRequest *request, struct p fd_note(pinning_fd, desc); typedef CommCbMemFunT Dialer; - pinning.closeHandler = asyncCall(33, 5, "ConnStateData::clientPinnedConnectionClosed", - Dialer(this, &ConnStateData::clientPinnedConnectionClosed)); + pinning.closeHandler = JobCallback(33, 5, + Dialer, this, ConnStateData::clientPinnedConnectionClosed); comm_add_close_handler(pinning_fd, pinning.closeHandler); } diff --git a/src/client_side_request.cc b/src/client_side_request.cc index 3a11190ea8..211e47f0b3 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -1374,11 +1374,11 @@ ClientHttpRequest::startAdaptation(const Adaptation::ServiceGroupPointer &g) assert(!virginHeadSource); assert(!adaptedBodySource); virginHeadSource = initiateAdaptation( - new Adaptation::Iterator(this, request, NULL, g)); + new Adaptation::Iterator(request, NULL, g)); // we could try to guess whether we can bypass this adaptation // initiation failure, but it should not really happen - assert(virginHeadSource != NULL); // Must, really + Must(initiated(virginHeadSource)); } void diff --git a/src/client_side_request.h b/src/client_side_request.h index 76cd869e2a..facfc4b2d4 100644 --- a/src/client_side_request.h +++ b/src/client_side_request.h @@ -177,7 +177,7 @@ private: void endRequestSatisfaction(); private: - Adaptation::Initiate *virginHeadSource; + CbcPointer virginHeadSource; BodyPipe::Pointer adaptedBodySource; bool request_satisfaction_mode; diff --git a/src/ftp.cc b/src/ftp.cc index 0ecbe623a0..17cf80355f 100644 --- a/src/ftp.cc +++ b/src/ftp.cc @@ -479,8 +479,8 @@ FtpStateData::FtpStateData(FwdState *theFwdState) : AsyncJob("FtpStateData"), Se flags.rest_supported = 1; typedef CommCbMemFunT Dialer; - AsyncCall::Pointer closer = asyncCall(9, 5, "FtpStateData::ctrlClosed", - Dialer(this, &FtpStateData::ctrlClosed)); + AsyncCall::Pointer closer = JobCallback(9, 5, + Dialer, this, FtpStateData::ctrlClosed); ctrl.opened(theFwdState->server_fd, closer); if (request->method == METHOD_PUT) @@ -1158,16 +1158,15 @@ FtpStateData::maybeReadVirginBody() data.read_pending = true; typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(9, 5, "FtpStateData::ftpTimeout", - TimeoutDialer(this,&FtpStateData::ftpTimeout)); + AsyncCall::Pointer timeoutCall = JobCallback(9, 5, + TimeoutDialer, this, FtpStateData::ftpTimeout); commSetTimeout(data.fd, Config.Timeout.read, timeoutCall); debugs(9,5,HERE << "queueing read on FD " << data.fd); typedef CommCbMemFunT Dialer; entry->delayAwareRead(data.fd, data.readBuf->space(), read_sz, - asyncCall(9, 5, "FtpStateData::dataRead", - Dialer(this, &FtpStateData::dataRead))); + JobCallback(9, 5, Dialer, this, FtpStateData::dataRead)); } void @@ -1216,8 +1215,8 @@ FtpStateData::dataRead(const CommIoCbParams &io) if (ignoreErrno(io.xerrno)) { typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(9, 5, "FtpStateData::ftpTimeout", - TimeoutDialer(this,&FtpStateData::ftpTimeout)); + AsyncCall::Pointer timeoutCall = JobCallback(9, 5, + TimeoutDialer, this, FtpStateData::ftpTimeout); commSetTimeout(io.fd, Config.Timeout.read, timeoutCall); maybeReadVirginBody(); @@ -1529,8 +1528,8 @@ FtpStateData::writeCommand(const char *buf) } typedef CommCbMemFunT Dialer; - AsyncCall::Pointer call = asyncCall(9, 5, "FtpStateData::ftpWriteCommandCallback", - Dialer(this, &FtpStateData::ftpWriteCommandCallback)); + AsyncCall::Pointer call = JobCallback(9, 5, + Dialer, this, FtpStateData::ftpWriteCommandCallback); comm_write(ctrl.fd, ctrl.last_command, strlen(ctrl.last_command), @@ -1667,8 +1666,8 @@ FtpStateData::scheduleReadControlReply(int buffered_ok) } else { /* XXX What about Config.Timeout.read? */ typedef CommCbMemFunT Dialer; - AsyncCall::Pointer reader=asyncCall(9, 5, "FtpStateData::ftpReadControlReply", - Dialer(this, &FtpStateData::ftpReadControlReply)); + AsyncCall::Pointer reader = JobCallback(9, 5, + Dialer, this, FtpStateData::ftpReadControlReply); comm_read(ctrl.fd, ctrl.buf + ctrl.offset, ctrl.size - ctrl.offset, reader); /* * Cancel the timeout on the Data socket (if any) and @@ -1681,8 +1680,8 @@ FtpStateData::scheduleReadControlReply(int buffered_ok) } typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(9, 5, "FtpStateData::ftpTimeout", - TimeoutDialer(this,&FtpStateData::ftpTimeout)); + AsyncCall::Pointer timeoutCall = JobCallback(9, 5, + TimeoutDialer, this, FtpStateData::ftpTimeout); commSetTimeout(ctrl.fd, Config.Timeout.read, timeoutCall); } @@ -2565,8 +2564,8 @@ ftpSendPassive(FtpStateData * ftpState) * dont acknowledge PASV commands. */ typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(9, 5, "FtpStateData::ftpTimeout", - TimeoutDialer(ftpState,&FtpStateData::ftpTimeout)); + AsyncCall::Pointer timeoutCall = JobCallback(9, 5, + TimeoutDialer, ftpState, FtpStateData::ftpTimeout); commSetTimeout(ftpState->data.fd, 15, timeoutCall); } @@ -2764,8 +2763,8 @@ ftpOpenListenSocket(FtpStateData * ftpState, int fallback) } typedef CommCbMemFunT acceptDialer; - AsyncCall::Pointer acceptCall = asyncCall(11, 5, "FtpStateData::ftpAcceptDataConnection", - acceptDialer(ftpState, &FtpStateData::ftpAcceptDataConnection)); + AsyncCall::Pointer acceptCall = JobCallback(11, 5, + acceptDialer, ftpState, FtpStateData::ftpAcceptDataConnection); ftpState->data.listener = new Comm::ListenStateData(fd, acceptCall, false); if (!ftpState->data.listener || ftpState->data.listener->errcode != 0) { @@ -2947,8 +2946,8 @@ void FtpStateData::ftpAcceptDataConnection(const CommAcceptCbParams &io) /* we are ony accepting once, so need to re-open the listener socket. */ typedef CommCbMemFunT acceptDialer; - AsyncCall::Pointer acceptCall = asyncCall(11, 5, "FtpStateData::ftpAcceptDataConnection", - acceptDialer(this, &FtpStateData::ftpAcceptDataConnection)); + AsyncCall::Pointer acceptCall = JobCallback(11, 5, + acceptDialer, this, FtpStateData::ftpAcceptDataConnection); data.listener = new Comm::ListenStateData(data.fd, acceptCall, false); return; } @@ -2978,8 +2977,8 @@ void FtpStateData::ftpAcceptDataConnection(const CommAcceptCbParams &io) commSetTimeout(ctrl.fd, -1, nullCall); typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(9, 5, "FtpStateData::ftpTimeout", - TimeoutDialer(this,&FtpStateData::ftpTimeout)); + AsyncCall::Pointer timeoutCall = JobCallback(9, 5, + TimeoutDialer, this, FtpStateData::ftpTimeout); commSetTimeout(data.fd, Config.Timeout.read, timeoutCall); /*\todo XXX We should have a flag to track connect state... @@ -3071,8 +3070,8 @@ void FtpStateData::readStor() commSetTimeout(ctrl.fd, -1, nullCall); typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(9, 5, "FtpStateData::ftpTimeout", - TimeoutDialer(this,&FtpStateData::ftpTimeout)); + AsyncCall::Pointer timeoutCall = JobCallback(9, 5, + TimeoutDialer, this, FtpStateData::ftpTimeout); commSetTimeout(data.fd, Config.Timeout.read, timeoutCall); @@ -3083,8 +3082,8 @@ void FtpStateData::readStor() * When client code is 150 with a hostname, Accept data channel. */ debugs(9, 3, "ftpReadStor: accepting data channel"); typedef CommCbMemFunT acceptDialer; - AsyncCall::Pointer acceptCall = asyncCall(11, 5, "FtpStateData::ftpAcceptDataConnection", - acceptDialer(this, &FtpStateData::ftpAcceptDataConnection)); + AsyncCall::Pointer acceptCall = JobCallback(11, 5, + acceptDialer, this, FtpStateData::ftpAcceptDataConnection); data.listener = new Comm::ListenStateData(data.fd, acceptCall, false); } else { @@ -3219,8 +3218,8 @@ ftpReadList(FtpStateData * ftpState) } else if (code == 150) { /* Accept data channel */ typedef CommCbMemFunT acceptDialer; - AsyncCall::Pointer acceptCall = asyncCall(11, 5, "FtpStateData::ftpAcceptDataConnection", - acceptDialer(ftpState, &FtpStateData::ftpAcceptDataConnection)); + AsyncCall::Pointer acceptCall = JobCallback(11, 5, + acceptDialer, ftpState, FtpStateData::ftpAcceptDataConnection); ftpState->data.listener = new Comm::ListenStateData(ftpState->data.fd, acceptCall, false); /* @@ -3231,8 +3230,8 @@ ftpReadList(FtpStateData * ftpState) commSetTimeout(ftpState->ctrl.fd, -1, nullCall); typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(9, 5, "FtpStateData::ftpTimeout", - TimeoutDialer(ftpState,&FtpStateData::ftpTimeout)); + AsyncCall::Pointer timeoutCall = JobCallback(9, 5, + TimeoutDialer, ftpState,FtpStateData::ftpTimeout); commSetTimeout(ftpState->data.fd, Config.Timeout.read, timeoutCall); return; } else if (!ftpState->flags.tried_nlst && code > 300) { @@ -3281,8 +3280,8 @@ ftpReadRetr(FtpStateData * ftpState) } else if (code == 150) { /* Accept data channel */ typedef CommCbMemFunT acceptDialer; - AsyncCall::Pointer acceptCall = asyncCall(11, 5, "FtpStateData::ftpAcceptDataConnection", - acceptDialer(ftpState, &FtpStateData::ftpAcceptDataConnection)); + AsyncCall::Pointer acceptCall = JobCallback(11, 5, + acceptDialer, ftpState, FtpStateData::ftpAcceptDataConnection); ftpState->data.listener = new Comm::ListenStateData(ftpState->data.fd, acceptCall, false); /* * Cancel the timeout on the Control socket and establish one @@ -3292,8 +3291,8 @@ ftpReadRetr(FtpStateData * ftpState) commSetTimeout(ftpState->ctrl.fd, -1, nullCall); typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(9, 5, "FtpStateData::ftpTimeout", - TimeoutDialer(ftpState,&FtpStateData::ftpTimeout)); + AsyncCall::Pointer timeoutCall = JobCallback(9, 5, + TimeoutDialer, ftpState,FtpStateData::ftpTimeout); commSetTimeout(ftpState->data.fd, Config.Timeout.read, timeoutCall); } else if (code >= 300) { if (!ftpState->flags.try_slash_hack) { @@ -3927,8 +3926,7 @@ AsyncCall::Pointer FtpStateData::dataCloser() { typedef CommCbMemFunT Dialer; - return asyncCall(9, 5, "FtpStateData::dataClosed", - Dialer(this, &FtpStateData::dataClosed)); + return JobCallback(9, 5, Dialer, this, FtpStateData::dataClosed); } /// configures the channel with a descriptor and registers a close handler diff --git a/src/http.cc b/src/http.cc index 4a712aeddb..31f5ad62fa 100644 --- a/src/http.cc +++ b/src/http.cc @@ -142,8 +142,8 @@ HttpStateData::HttpStateData(FwdState *theFwdState) : AsyncJob("HttpStateData"), * register the handler to free HTTP state data when the FD closes */ typedef CommCbMemFunT Dialer; - closeHandler = asyncCall(9, 5, "httpStateData::httpStateConnClosed", - Dialer(this,&HttpStateData::httpStateConnClosed)); + closeHandler = JobCallback(9, 5, + Dialer, this, HttpStateData::httpStateConnClosed); comm_add_close_handler(fd, closeHandler); } @@ -1403,8 +1403,7 @@ HttpStateData::maybeReadVirginBody() flags.do_next_read = 0; typedef CommCbMemFunT Dialer; entry->delayAwareRead(fd, readBuf->space(read_size), read_size, - asyncCall(11, 5, "HttpStateData::readReply", - Dialer(this, &HttpStateData::readReply))); + JobCallback(11, 5, Dialer, this, HttpStateData::readReply)); } } @@ -1447,8 +1446,8 @@ HttpStateData::sendComplete(const CommIoCbParams &io) * request bodies. */ typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(11, 5, "HttpStateData::httpTimeout", - TimeoutDialer(this,&HttpStateData::httpTimeout)); + AsyncCall::Pointer timeoutCall = JobCallback(11, 5, + TimeoutDialer, this, HttpStateData::httpTimeout); commSetTimeout(fd, Config.Timeout.read, timeoutCall); @@ -1989,8 +1988,8 @@ HttpStateData::sendRequest() } typedef CommCbMemFunT TimeoutDialer; - AsyncCall::Pointer timeoutCall = asyncCall(11, 5, "HttpStateData::httpTimeout", - TimeoutDialer(this,&HttpStateData::httpTimeout)); + AsyncCall::Pointer timeoutCall = JobCallback(11, 5, + TimeoutDialer, this, HttpStateData::httpTimeout); commSetTimeout(fd, Config.Timeout.lifetime, timeoutCall); flags.do_next_read = 1; maybeReadVirginBody(); @@ -1999,13 +1998,13 @@ HttpStateData::sendRequest() if (!startRequestBodyFlow()) // register to receive body data return false; typedef CommCbMemFunT Dialer; - Dialer dialer(this, &HttpStateData::sentRequestBody); - requestSender = asyncCall(11,5, "HttpStateData::sentRequestBody", dialer); + requestSender = JobCallback(11,5, + Dialer, this, HttpStateData::sentRequestBody); } else { assert(!requestBodySource); typedef CommCbMemFunT Dialer; - Dialer dialer(this, &HttpStateData::sendComplete); - requestSender = asyncCall(11,5, "HttpStateData::SendComplete", dialer); + requestSender = JobCallback(11,5, + Dialer, this, HttpStateData::sendComplete); } if (_peer != NULL) { @@ -2099,8 +2098,8 @@ HttpStateData::doneSendingRequestBody() } typedef CommCbMemFunT Dialer; - Dialer dialer(this, &HttpStateData::sendComplete); - AsyncCall::Pointer call= asyncCall(11,5, "HttpStateData::SendComplete", dialer); + AsyncCall::Pointer call = JobCallback(11,5, + Dialer, this, HttpStateData::sendComplete); comm_write(fd, "\r\n", 2, call); } return; diff --git a/src/ipc/Port.cc b/src/ipc/Port.cc index 328229e247..c3075800d1 100644 --- a/src/ipc/Port.cc +++ b/src/ipc/Port.cc @@ -30,8 +30,9 @@ void Ipc::Port::listen() { debugs(54, 6, HERE); buf.prepForReading(); - AsyncCall::Pointer readHandler = asyncCall(54, 6, "Ipc::Port::noteRead", - CommCbMemFunT(this, &Port::noteRead)); + typedef CommCbMemFunT Dialer; + AsyncCall::Pointer readHandler = JobCallback(54, 6, + Dialer, this, Port::noteRead); comm_read(fd(), buf.raw(), buf.size(), readHandler); } diff --git a/src/ipc/UdsOp.cc b/src/ipc/UdsOp.cc index f1d05e3852..1c148b02fe 100644 --- a/src/ipc/UdsOp.cc +++ b/src/ipc/UdsOp.cc @@ -47,9 +47,9 @@ int Ipc::UdsOp::fd() void Ipc::UdsOp::setTimeout(int seconds, const char *handlerName) { + typedef CommCbMemFunT Dialer; AsyncCall::Pointer handler = asyncCall(54,5, handlerName, - CommCbMemFunT(this, - &UdsOp::noteTimeout)); + Dialer(CbcPointer(this), &UdsOp::noteTimeout)); commSetTimeout(fd(), seconds, handler); } @@ -103,8 +103,9 @@ bool Ipc::UdsSender::doneAll() const void Ipc::UdsSender::write() { debugs(54, 5, HERE); - AsyncCall::Pointer writeHandler = asyncCall(54, 5, "Ipc::UdsSender::wrote", - CommCbMemFunT(this, &UdsSender::wrote)); + typedef CommCbMemFunT Dialer; + AsyncCall::Pointer writeHandler = JobCallback(54, 5, + Dialer, this, UdsSender::wrote); comm_write(fd(), message.raw(), message.size(), writeHandler); writing = true; } @@ -128,5 +129,5 @@ void Ipc::UdsSender::timedout() void Ipc::SendMessage(const String& toAddress, const TypedMsgHdr &message) { - AsyncJob::AsyncStart(new UdsSender(toAddress, message)); + AsyncJob::Start(new UdsSender(toAddress, message)); } diff --git a/src/main.cc b/src/main.cc index ebe88f3598..239621b000 100644 --- a/src/main.cc +++ b/src/main.cc @@ -1460,9 +1460,9 @@ SquidMain(int argc, char **argv) mainLoop.setTimeService(&time_engine); if (IamCoordinatorProcess()) - AsyncJob::AsyncStart(Ipc::Coordinator::Instance()); + AsyncJob::Start(Ipc::Coordinator::Instance()); else if (UsingSmp() && IamWorkerProcess()) - AsyncJob::AsyncStart(new Ipc::Strand); + AsyncJob::Start(new Ipc::Strand); /* at this point we are finished the synchronous startup. */ starting_up = 0;