--------------------- PatchSet 6717 Date: 2008/01/24 21:29:15 Author: rousskov Branch: async-calls Tag: (none) Log: Do not store call Pointer in CallDialer because the dialer is stored in the call and the call does not go away until the last reference to it is gone. We wanted to store a call pointer to simplify canDial() and dial(). This change makes them [slightly] more complex by passing a reference to the "owner" call object as a parameter and avoiding any storage issues. We could store a raw pointer to the call instead, but it is scary to do that when calls are refcounted. There is still one place that uses raw call pointers -- AsyncJob::inCall. There, it is used mostly for debugging and only during a call handler execution (at least that's the theory). We will probably eliminate it eventually. Members: src/AsyncCall.h:1.3.22.21->1.3.22.22 src/BodyPipe.cc:1.7.4.10->1.7.4.11 src/CommCalls.h:1.1.2.5->1.1.2.6 src/event.cc:1.16.4.3->1.16.4.4 src/ICAP/AsyncJob.cc:1.3.4.11->1.3.4.12 src/ICAP/AsyncJob.h:1.3.14.9->1.3.14.10 Index: squid3/src/AsyncCall.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/AsyncCall.h,v retrieving revision 1.3.22.21 retrieving revision 1.3.22.22 diff -u -r1.3.22.21 -r1.3.22.22 --- squid3/src/AsyncCall.h 29 Dec 2007 15:24:31 -0000 1.3.22.21 +++ squid3/src/AsyncCall.h 24 Jan 2008 21:29:15 -0000 1.3.22.22 @@ -1,6 +1,6 @@ /* - * $Id: AsyncCall.h,v 1.3.22.21 2007/12/29 15:24:31 chtsanti Exp $ + * $Id: AsyncCall.h,v 1.3.22.22 2008/01/24 21:29:15 rousskov Exp $ */ #ifndef SQUID_ASYNCCALL_H @@ -88,13 +88,10 @@ class CallDialer { public: - CallDialer(): call(NULL) {} + CallDialer() {} virtual ~CallDialer() {} virtual void print(std::ostream &os) const = 0; - -public: - AsyncCall::Pointer call; }; // This template implements an AsyncCall using a specified Dialer class @@ -104,13 +101,14 @@ public: AsyncCallT(int aDebugSection, int aDebugLevel, const char *aName, const Dialer &aDialer): AsyncCall(aDebugSection, aDebugLevel, aName), - dialer(aDialer) { dialer.call = this; } + dialer(aDialer) {} CallDialer *getDialer() { return &dialer; } protected: - virtual bool canFire() { return AsyncCall::canFire() && dialer.canDial(); } - virtual void fire() { dialer.dial(); } + virtual bool canFire() { return AsyncCall::canFire() && + dialer.canDial(*this); } + virtual void fire() { dialer.dial(*this); } Dialer dialer; }; Index: squid3/src/BodyPipe.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/BodyPipe.cc,v retrieving revision 1.7.4.10 retrieving revision 1.7.4.11 diff -u -r1.7.4.10 -r1.7.4.11 --- squid3/src/BodyPipe.cc 14 Jan 2008 22:36:32 -0000 1.7.4.10 +++ squid3/src/BodyPipe.cc 24 Jan 2008 21:29:15 -0000 1.7.4.11 @@ -41,7 +41,7 @@ BodyProducerDialer(BodyProducer *aProducer, Parent::Method aHandler, BodyPipe::Pointer bp): Parent(aProducer, aHandler, bp) {} - virtual bool canDial(); + virtual bool canDial(AsyncCall &call); }; // The BodyConsumerDialer is an AsyncCall class which used to schedule BodyConsumer calls. @@ -55,36 +55,36 @@ BodyConsumerDialer(BodyConsumer *aConsumer, Parent::Method aHandler, BodyPipe::Pointer bp): Parent(aConsumer, aHandler, bp) {} - virtual bool canDial(); + virtual bool canDial(AsyncCall &call); }; bool -BodyProducerDialer::canDial() { - if (!Parent::canDial()) +BodyProducerDialer::canDial(AsyncCall &call) { + if (!Parent::canDial(call)) return false; BodyProducer *producer = object; BodyPipe::Pointer pipe = arg1; if (!pipe->stillProducing(producer)) { - debugs(call->debugSection, call->debugLevel, HERE << producer << + debugs(call.debugSection, call.debugLevel, HERE << producer << " no longer producing for " << pipe->status()); - return call->cancel("no longer producing"); + return call.cancel("no longer producing"); } return true; } bool -BodyConsumerDialer::canDial() { - if (!Parent::canDial()) +BodyConsumerDialer::canDial(AsyncCall &call) { + if (!Parent::canDial(call)) return false; BodyConsumer *consumer = object; BodyPipe::Pointer pipe = arg1; if (!pipe->stillConsuming(consumer)) { - debugs(call->debugSection, call->debugLevel, HERE << consumer << + debugs(call.debugSection, call.debugLevel, HERE << consumer << " no longer consuming from " << pipe->status()); - return call->cancel("no longer consuming"); + return call.cancel("no longer consuming"); } return true; Index: squid3/src/CommCalls.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/Attic/CommCalls.h,v retrieving revision 1.1.2.5 retrieving revision 1.1.2.6 diff -u -r1.1.2.5 -r1.1.2.6 --- squid3/src/CommCalls.h 19 Jan 2008 19:15:57 -0000 1.1.2.5 +++ squid3/src/CommCalls.h 24 Jan 2008 21:29:15 -0000 1.1.2.6 @@ -1,6 +1,6 @@ /* - * $Id: CommCalls.h,v 1.1.2.5 2008/01/19 19:15:57 chtsanti Exp $ + * $Id: CommCalls.h,v 1.1.2.6 2008/01/24 21:29:15 rousskov Exp $ */ #ifndef SQUID_COMMCALLS_H @@ -265,7 +265,6 @@ AsyncCall(debugSection, debugLevel, callName), dialer(aDialer) { - dialer.call = this; } Index: squid3/src/event.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/event.cc,v retrieving revision 1.16.4.3 retrieving revision 1.16.4.4 diff -u -r1.16.4.3 -r1.16.4.4 --- squid3/src/event.cc 29 Dec 2007 15:24:40 -0000 1.16.4.3 +++ squid3/src/event.cc 24 Jan 2008 21:29:15 -0000 1.16.4.4 @@ -1,5 +1,5 @@ /* - * $Id: event.cc,v 1.16.4.3 2007/12/29 15:24:40 chtsanti Exp $ + * $Id: event.cc,v 1.16.4.4 2008/01/24 21:29:15 rousskov Exp $ * * DEBUG: section 41 Event Processing * AUTHOR: Henrik Nordstrom @@ -55,9 +55,9 @@ virtual ~EventDialer(); virtual void print(std::ostream &os) const; - virtual bool canDial(); + virtual bool canDial(AsyncCall &call); - void dial() { theHandler(theArg); } + void dial(AsyncCall &) { theHandler(theArg); } private: EVH *theHandler; @@ -86,13 +86,13 @@ } bool -EventDialer::canDial() { +EventDialer::canDial(AsyncCall &call) { // TODO: add Parent::canDial() that always returns true //if (!Parent::canDial()) // return false; if (isLockedArg && !cbdataReferenceValid(theArg)) - return call->cancel("stale handler data"); + return call.cancel("stale handler data"); return true; } Index: squid3/src/ICAP/AsyncJob.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/AsyncJob.cc,v retrieving revision 1.3.4.11 retrieving revision 1.3.4.12 diff -u -r1.3.4.11 -r1.3.4.12 --- squid3/src/ICAP/AsyncJob.cc 21 Dec 2007 15:01:15 -0000 1.3.4.11 +++ squid3/src/ICAP/AsyncJob.cc 24 Jan 2008 21:29:15 -0000 1.3.4.12 @@ -90,27 +90,27 @@ return true; // so that it is safe for kids to use } -bool AsyncJob::canBeCalled(AsyncCall::Pointer &call) const +bool AsyncJob::canBeCalled(AsyncCall &call) const { if (inCall != NULL) { // This may happen when we have bugs or some module is not calling // us asynchronously (comm used to do that). debugs(93, 5, HERE << inCall << " is in progress; " << - *call << " canot reenter the job."); - return call->cancel("reentrant job call"); + call << " canot reenter the job."); + return call.cancel("reentrant job call"); } return true; } -void AsyncJob::callStart(AsyncCall::Pointer &call) +void AsyncJob::callStart(AsyncCall &call) { // we must be called asynchronously and hence, the caller must lock us Must(cbdataReferenceValid(toCbdata())); Must(!inCall); // see AsyncJob::canBeCalled - inCall = call; + inCall = &call; // XXX: ugly, but safe if callStart/callEnd,Ex are paired debugs(inCall->debugSection, inCall->debugLevel, typeName << " status in:" << status()); } @@ -173,29 +173,25 @@ JobDialer::JobDialer(const JobDialer &d): CallDialer(d), job(d.job), lock(d.lock) { - assert(!call); // otherwise, two dialers will share the same call cbdataReference(lock); } JobDialer::~JobDialer(){ - // TODO: assert(!call) and set dialer.call to NULL in AsyncCall kids - // TODO: move this->call manipulation logic from AsyncCall kids to call? cbdataReferenceDone(lock); } bool -JobDialer::canDial() +JobDialer::canDial(AsyncCall &call) { - assert(call != NULL); if (!cbdataReferenceValid(lock)) - return call->cancel("job is gone"); + return call.cancel("job is gone"); return job->canBeCalled(call); } void -JobDialer::dial() +JobDialer::dial(AsyncCall &call) { assert(cbdataReferenceValid(lock)); // canDial() checks for this @@ -205,8 +201,8 @@ doDial(); } catch (const TextException &e) { - debugs(call->debugSection, 3, - HERE << call->name << " threw exception: " << e.message); + debugs(call.debugSection, 3, + HERE << call.name << " threw exception: " << e.message); job->callException(e); } Index: squid3/src/ICAP/AsyncJob.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/AsyncJob.h,v retrieving revision 1.3.14.9 retrieving revision 1.3.14.10 diff -u -r1.3.14.9 -r1.3.14.10 --- squid3/src/ICAP/AsyncJob.h 21 Dec 2007 15:01:15 -0000 1.3.14.9 +++ squid3/src/ICAP/AsyncJob.h 24 Jan 2008 21:29:15 -0000 1.3.14.10 @@ -3,7 +3,7 @@ /* - * $Id: AsyncJob.h,v 1.3.14.9 2007/12/21 15:01:15 chtsanti Exp $ + * $Id: AsyncJob.h,v 1.3.14.10 2008/01/24 21:29:15 rousskov Exp $ */ #ifndef SQUID_ASYNC_JOB_H @@ -59,8 +59,8 @@ public: // asynchronous call maintenance - bool canBeCalled(AsyncCall::Pointer &call) const; - void callStart(AsyncCall::Pointer &call); + bool canBeCalled(AsyncCall &call) const; + void callStart(AsyncCall &call); virtual void callException(const TextException &e); virtual void callEnd(); @@ -90,8 +90,8 @@ JobDialer(const JobDialer &d); virtual ~JobDialer(); - virtual bool canDial(); - void dial(); + virtual bool canDial(AsyncCall &call); + void dial(AsyncCall &call); AsyncJob *job; void *lock; // job's cbdata