--------------------- PatchSet 6133 Date: 2007/11/15 19:49:58 Author: chtsanti Branch: async-calls Tag: (none) Log: - Moving the async calls to the ICAPInitiator::noteIcapQueryAbort and ICAPInitiator::noteIcapAnswer to the new async calls interface - Calls to AsyncCallEnter/AsyncCallExit macros must removed from the childs implementations of noteIcapQueryAbort and noteIcapAnswer methods - The classes ICAPAnswerCall and ICAPQueryAbortCall does not needed eny more. I keep their code but put it inside the OLD_ASYNC_CALLS ifdefs - The method I selected to implement the asynchronous call to ICAPInitiator::noteIcapAnswer requires the addition of an HTTPMSGUNLOCK at the end of ServerStateData::noteIcapAnswer and ClientHttpRequest::noteIcapAnswer Members: src/Server.cc:1.20.4.2->1.20.4.3 src/client_side_request.cc:1.79.4.1->1.79.4.2 src/ICAP/ICAPInitiate.cc:1.2->1.2.30.1 src/ICAP/ICAPLauncher.cc:1.2.30.1->1.2.30.2 Index: squid3/src/Server.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/Server.cc,v retrieving revision 1.20.4.2 retrieving revision 1.20.4.3 diff -u -r1.20.4.2 -r1.20.4.3 --- squid3/src/Server.cc 24 Oct 2007 17:20:49 -0000 1.20.4.2 +++ squid3/src/Server.cc 15 Nov 2007 19:49:58 -0000 1.20.4.3 @@ -1,5 +1,5 @@ /* - * $Id: Server.cc,v 1.20.4.2 2007/10/24 17:20:49 chtsanti Exp $ + * $Id: Server.cc,v 1.20.4.3 2007/11/15 19:49:58 chtsanti Exp $ * * DEBUG: * AUTHOR: Duane Wessels @@ -538,7 +538,8 @@ if (doneWithIcap()) // we may still be sending virgin response handleIcapCompleted(); } - + //required because of the HTTPMSGLOCK in ICAPInitiate::sendAnswer!!! + HTTPMSGUNLOCK(msg); } // will not receive adapted response headers (and, hence, body) Index: squid3/src/client_side_request.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/client_side_request.cc,v retrieving revision 1.79.4.1 retrieving revision 1.79.4.2 diff -u -r1.79.4.1 -r1.79.4.2 --- squid3/src/client_side_request.cc 23 Oct 2007 20:27:52 -0000 1.79.4.1 +++ squid3/src/client_side_request.cc 15 Nov 2007 19:49:58 -0000 1.79.4.2 @@ -1,6 +1,6 @@ /* - * $Id: client_side_request.cc,v 1.79.4.1 2007/10/23 20:27:52 chtsanti Exp $ + * $Id: client_side_request.cc,v 1.79.4.2 2007/11/15 19:49:58 chtsanti Exp $ * * DEBUG: section 85 Client-side Request Routines * AUTHOR: Robert Collins (Originally Duane Wessels in client_side.c) @@ -1144,6 +1144,8 @@ if (!request_satisfaction_mode) doCallouts(); + //required because of the HTTPMSGLOCK in ICAPInitiate::sendAnswer!!! + HTTPMSGUNLOCK(msg); } void Index: squid3/src/ICAP/ICAPInitiate.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPInitiate.cc,v retrieving revision 1.2 retrieving revision 1.2.30.1 diff -u -r1.2 -r1.2.30.1 --- squid3/src/ICAP/ICAPInitiate.cc 8 May 2007 16:51:27 -0000 1.2 +++ squid3/src/ICAP/ICAPInitiate.cc 15 Nov 2007 19:49:58 -0000 1.2.30.1 @@ -19,6 +19,7 @@ /* Event data and callback wrapper to call noteIcapAnswer with * the answer message as a parameter. */ +#ifdef OLD_ASYNC_CALLS class ICAPAnswerCall { public: // use this function to make an asynchronous call @@ -59,7 +60,7 @@ ICAPInitiatorHolder theInitiator; bool isFinal; }; - +#endif /* ICAPInitiate */ @@ -95,15 +96,66 @@ theInitiator.clear(); } +/* +typedef UnaryMemFunT MemFun_ICAPInitiatorHttpMsg ; +class ICAPInitiatorHttpMsgCall: public JobCallT< MemFun_ICAPInitiatorHttpMsg > { +public: + typedef void (ICAPInitiator::*MemFunPtr)(HttpMsg *); + ICAPInitiatorHttpMsgCall(ICAPInitiator *anInitiator, MemFunPtr aP, HttpMsg *aMessage, const char *callName): + JobCallT< MemFun_ICAPInitiatorHttpMsg >(MemFun(anInitiator,aP,aMessage),93,5,callName) + { + assert(aMessage); + theMessage = aMessage; + HTTPMSGLOCK(theMessage); + } + ~ICAPInitiatorHttpMsgCall() + { + HTTPMSGUNLOCK(theMessage); + } +protected: + HttpMsg *theMessage; +}; + +void CallICAPInitiatorHttpMsgCall(ICAPInitiator *anInitiator, + ICAPInitiatorHttpMsgCall::MemFunPtr aP, + HttpMsg *aMessage, const char *callName) +{ + AsyncCallBase *call = new ICAPInitiatorHttpMsgCall(anInitiator, aP, aMessage, callName); + //Maybe the scheduleAsyncCall should replaced by an other macro... + scheduleAsyncCall(93, 5, __FILE__, __LINE__, + call, callName, + &(AsyncCallBase::fireWrapper2), false); +} +*/ +/* + The problem scheduling the ICAPInitiator::noteIcapAnswer is that it takes as argument an HttpMsg + object which maybe is destroyed when the noteIcapAnswer called. To avoid this required to HTTPMSGLOCK this + object, but required to HTTPMSGUNLOCK this object after the noteIcapAnswer method execuded. + There are two ways for doing this: + 1) HTTPMSGLOCK the HttpMsg object just before ICAPInitiator::noteIcapAnswer scheduled and modify all + noteIcapAnswer implementations in child classes (ClientHttpRequest and ServerStateData) to HTTPMSGUNLOCK + the HttpMsg just before this method ends + 2) Implement an JobCallT child class which also HTTPMSGLOCKs the argument of the call. + + I selected the first implementation because looks simpler but also I implemented the second approach to test this case too. + If you want to experiment with this implementation uncomment the above code and use the following call in ICAPInitiate::sendAnswer + CallICAPInitiatorHttpMsgCall(theInitiator.ptr,&ICAPInitiator::noteIcapAnswer, + msg,"ICAPInitiator::noteIcapAnswer"); + instead of the CallJobHere1 call +*/ + void ICAPInitiate::sendAnswer(HttpMsg *msg) { - ICAPAnswerCall::Schedule(theInitiator, msg); + assert(msg); + //The *::noteIcapAnswer must unlocks the msg after their executions, please read the comments above. + CallJobHere1(93, 5, theInitiator.ptr, ICAPInitiator::noteIcapAnswer, HTTPMSGLOCK(msg)); clearInitiator(); } + void ICAPInitiate::tellQueryAborted(bool final) { - ICAPQueryAbortCall::Schedule(theInitiator, final); + CallJobHere1(93, 5, theInitiator.ptr, ICAPInitiator::noteIcapQueryAbort, final); clearInitiator(); } @@ -157,6 +209,7 @@ return *this; } +#ifdef OLD_ASYNC_CALLS /* ICAPAnswerCall */ ICAPAnswerCall::ICAPAnswerCall(const ICAPInitiatorHolder &anInitiator, HttpMsg *aMessage): @@ -221,7 +274,6 @@ // The call object is deleted in ICAPAnswerCall::Wrapper } - /* ICAPQueryAbortCall */ ICAPQueryAbortCall::ICAPQueryAbortCall(const ICAPInitiatorHolder &anInitiator, bool beFinal): @@ -275,3 +327,4 @@ call->schedule(); // The call object is deleted in ICAPQueryAbortCall::Wrapper } +#endif Index: squid3/src/ICAP/ICAPLauncher.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPLauncher.cc,v retrieving revision 1.2.30.1 retrieving revision 1.2.30.2 diff -u -r1.2.30.1 -r1.2.30.2 --- squid3/src/ICAP/ICAPLauncher.cc 22 Oct 2007 19:26:14 -0000 1.2.30.1 +++ squid3/src/ICAP/ICAPLauncher.cc 15 Nov 2007 19:49:58 -0000 1.2.30.2 @@ -42,13 +42,10 @@ void ICAPLauncher::noteIcapAnswer(HttpMsg *message) { - AsyncCallEnter(noteIcapAnswer); - sendAnswer(message); clearIcap(theXaction); Must(done()); - - AsyncCallExit(); + debugs(93,3, HERE << "ICAPLauncher::noteIcapAnswer exiting "); } void ICAPLauncher::noteInitiatorAborted() @@ -64,8 +61,6 @@ void ICAPLauncher::noteIcapQueryAbort(bool final) { - AsyncCallEnter(noteQueryAbort); - clearIcap(theXaction); // TODO: add more checks from FwdState::checkRetry()? @@ -77,7 +72,6 @@ Must(done()); // swanSong will notify the initiator } - AsyncCallExit(); } bool ICAPLauncher::doneAll() const {