--------------------- PatchSet 4389 Date: 2007/04/25 21:23:47 Author: rousskov Branch: squid3-icap Tag: (none) Log: Bug #1819 fix, part 2: retry the ICAP transaction when its pconn fails. Part 2 changes should enable Squid to handle race conditions of ICAP pconns. HTTP client, server, and ICAP server representative (all ICAPInitiators) used to start ICAP transactions. If a transaction fails due to a persistent connection race condition, the initiator would either terminate the HTTP transaction or bypass the ICAP failure, violating the protocol. Now, instead of starting an ICAP transaction directly, the above initiators start ICAP Launcher. The Launcher starts the ICAP transaction and retries it (i.e., starts another transaction with the same set of parameters) if needed. The Launcher is both ICAP initiator (because it starts ICAP transactions) and ICAP initiate (because it is started by other ICAP initiators). The launcher proxies ICAP communication (usually one or two messages each way) and makes retries transparent to initiators. All ICAP transactions corresponding to the same adaptation of an HTTP message are sometimes referred to as a single ICAP query. An ICAP transaction can be retried until it read data from the ICAP server or consumed virgin body. This means that a ICAP transaction may go as far as writing the entire ICAP Preview before aborting in a retriable state. When retrying, persistent connections are not used. If the first retry fails, the query fails. Thus, a query cannot contain more than two transactions. Two new things were added to support ICAPLauncher: First, AsyncJob, is a class that handles the basic logic of maintaining an asynchronous job or task such as an ICAP transaction or a launcher. All core AsyncJob functionality was moved from ICAPXaction and ICAPModXact classes. We could not use the two classes for the launcher because ICAPLauncher is not a transaction. While currently specific to ICAP, this object may eventually be moved to Squid core and serve as a base for other asynchronous jobs or logical threads. Second, a temporary hack was added to support cbdata operations by two parents of a single CBDATA class (ICAPLauncher is both ICAP Initiator and Initiate). The second parent (ICAPInitiator) defines toCbdata() method that ICAPLauncher overwrites to provide a usable cbdata pointer to code that only sees an ICAPInitiator pointer. The ICAPInitiator pointer is unusable for cbdata when it comes from the middle of the ICAPLauncher object. Also, the initiator pointer is stored along with its cbdata so that the latter can be accessed (and unlocked) even if the initiator object is already deleted. These hacks will disappear once cbdata becomes object-aware. Members: src/Makefile.am:1.60.4.26->1.60.4.27 src/ICAP/AsyncJob.cc:1.1->1.1.2.1 src/ICAP/AsyncJob.h:1.1->1.1.2.1 src/ICAP/ICAPInitiate.cc:1.1.2.1->1.1.2.2 src/ICAP/ICAPInitiate.h:1.1.2.1->1.1.2.2 src/ICAP/ICAPInitiator.cc:1.1.2.2->1.1.2.3 src/ICAP/ICAPInitiator.h:1.1.2.2->1.1.2.3 src/ICAP/ICAPLauncher.cc:1.1->1.1.2.1 src/ICAP/ICAPLauncher.h:1.1->1.1.2.1 src/ICAP/ICAPModXact.cc:1.1.2.26->1.1.2.27 src/ICAP/ICAPModXact.h:1.1.2.12->1.1.2.13 src/ICAP/ICAPOptXact.cc:1.1.2.8->1.1.2.9 src/ICAP/ICAPOptXact.h:1.1.2.5->1.1.2.6 src/ICAP/ICAPXaction.cc:1.1.2.13->1.1.2.14 src/ICAP/ICAPXaction.h:1.1.2.8->1.1.2.9 Index: squid3/src/Makefile.am =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/Makefile.am,v retrieving revision 1.60.4.26 retrieving revision 1.60.4.27 diff -u -r1.60.4.26 -r1.60.4.27 --- squid3/src/Makefile.am 23 Apr 2007 16:23:43 -0000 1.60.4.26 +++ squid3/src/Makefile.am 25 Apr 2007 21:23:52 -0000 1.60.4.27 @@ -1,7 +1,7 @@ # # Makefile for the Squid Object Cache server # -# $Id: Makefile.am,v 1.60.4.26 2007/04/23 16:23:43 rousskov Exp $ +# $Id: Makefile.am,v 1.60.4.27 2007/04/25 21:23:52 rousskov Exp $ # # Uncomment and customize the following to suit your needs: # @@ -667,6 +667,8 @@ @ICAP_LIBS@ ICAP_libicap_a_SOURCES = \ + ICAP/AsyncJob.cc \ + ICAP/AsyncJob.h \ ICAP/ChunkedCodingParser.cc \ ICAP/ChunkedCodingParser.h \ ICAP/ICAPClient.cc \ @@ -676,6 +678,8 @@ ICAP/ICAPInitiate.cc \ ICAP/ICAPInitiate.h \ ICAP/ICAPInOut.h \ + ICAP/ICAPLauncher.cc \ + ICAP/ICAPLauncher.h \ ICAP/ICAPConfig.cc \ ICAP/ICAPConfig.h \ ICAP/ICAPElements.cc \ --- /dev/null Thu May 3 00:17:57 2007 +++ squid3/src/ICAP/AsyncJob.cc Thu May 3 00:17:57 2007 @@ -0,0 +1,113 @@ +/* + * DEBUG: section 93 ICAP (RFC 3507) Client + */ + +#include "squid.h" +#include "cbdata.h" +#include "TextException.h" +#include "AsyncJob.h" + + +AsyncJob *AsyncJob::AsyncStart(AsyncJob *job) { + assert(job); + cbdataReference(job); // unlocked when done() in callEnd() + AsyncCall(93,5, job, AsyncJob::noteStart); + return job; +} + +AsyncJob::AsyncJob(const char *aTypeName): typeName(aTypeName), inCall(NULL) +{ +} + +AsyncJob::~AsyncJob() +{ +} + +void AsyncJob::noteStart() +{ + AsyncCallEnter(noteStart); + + start(); + + AsyncCallExit(); +} + +void AsyncJob::start() +{ + Must(cbdataReferenceValid(this)); // locked in AsyncStart +} + +void AsyncJob::mustStop(const char *aReason) +{ + Must(inCall); // otherwise nobody will delete us if we are done() + Must(aReason); + if (!stopReason) { + stopReason = aReason; + debugs(93, 5, typeName << " will stop, reason: " << stopReason); + } else { + debugs(93, 5, typeName << " will stop, another reason: " << aReason); + } +} + +bool AsyncJob::done() const +{ + // stopReason, set in mustStop(), overwrites all other conditions + return stopReason != NULL || doneAll(); +} + +bool AsyncJob::doneAll() const +{ + return true; // so that it is safe for kids to use +} + +bool AsyncJob::callStart(const char *method) +{ + debugs(93, 5, typeName << "::" << method << " called" << status()); + + if (inCall) { + // this may happen when we have bugs or when arguably buggy + // comm interface calls us while we are closing the connection + debugs(93, 5, HERE << typeName << "::" << inCall << + " is in progress; " << typeName << "::" << method << + " cancels reentry."); + return false; + } + + inCall = method; + return true; +} + +void AsyncJob::callException(const TextException &e) +{ + debugs(93, 2, typeName << "::" << inCall << " caught an exception: " << + e.message << ' ' << status()); + + mustStop("exception"); +} + +void AsyncJob::callEnd() +{ + if (done()) { + debugs(93, 5, typeName << "::" << inCall << " ends job " << + status()); + + const char *inCallSaved = inCall; + const char *typeNameSaved = typeName; + void *thisSaved = this; + + swanSong(); + + void *cbdata = this; + delete this; // this is the only place where the object is deleted + cbdataReferenceDone(cbdata); // locked by AsyncStart + + // careful: this object does not exist any more + debugs(93, 6, HERE << typeNameSaved << "::" << inCallSaved << + " ended " << thisSaved); + return; + } + + debugs(93, 6, typeName << "::" << inCall << " ended" << status()); + inCall = NULL; +} + --- /dev/null Thu May 3 00:17:57 2007 +++ squid3/src/ICAP/AsyncJob.h Thu May 3 00:17:57 2007 @@ -0,0 +1,115 @@ + +/* + * $Id: AsyncJob.h,v 1.1.2.1 2007/04/25 21:23:47 rousskov Exp $ + * + * + * SQUID Web Proxy Cache http://www.squid-cache.org/ + * ---------------------------------------------------------- + * + * Squid is the result of efforts by numerous individuals from + * the Internet community; see the CONTRIBUTORS file for full + * details. Many organizations have provided support for Squid's + * development; see the SPONSORS file for full details. Squid is + * Copyrighted (C) 2001 by the Regents of the University of + * California; see the COPYRIGHT file for full details. Squid + * incorporates software developed and/or copyrighted by other + * sources; see the CREDITS file for full details. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA. + * + */ + +#ifndef SQUID_ASYNC_JOB_H +#define SQUID_ASYNC_JOB_H + +#include "AsyncCall.h" + +/* + * AsyncJob is an API and a base for a class that implements a stand-alone + * "job", "task", or "logical processing thread" which receives asynchronous + * calls. + * + * Implementations should wrap each method receiving an asynchronous call in + * a pair of macros: AsyncCallEnter and AsyncCallExit. These macros: + * - provide call debugging + * - trap exceptions and terminate the task if an exception occurs + * - ensure that only one asynchronous call is active per object + * Most of the work is done by AsyncJob class methods. Macros just provide + * an enter/try/catch/exit framework. + * + * Eventually, the macros can and perhaps should be replaced with call/event + * processing code so that individual job classes do not have to wrap all + * asynchronous calls. + */ + +class TextException; + +class AsyncJob +{ + +public: + static AsyncJob *AsyncStart(AsyncJob *job); // use this to start jobs + + AsyncJob(const char *aTypeName); + virtual ~AsyncJob(); + + void noteStart(); // calls virtual start + AsyncCallWrapper(93,3, AsyncJob, noteStart); + +protected: + void mustStop(const char *aReason); // force done() for a reason + + bool done() const; // the job is destroyed in callEnd() when done() + + virtual void start() = 0; + virtual bool doneAll() const = 0; // return true when done + virtual void swanSong() = 0; // perform internal cleanup + virtual const char *status() const = 0; // for debugging + + // asynchronous call maintenance + bool callStart(const char *methodName); + void callException(const TextException &e); + virtual void callEnd(); + + const char *stopReason; // reason for forcing done() to be true + const char *typeName; // kid (leaf) class name, for debugging + const char *inCall; // name of the asynchronous call being executed, if any +}; + + +// call guards for all "asynchronous" note*() methods +// TODO: Move to core. + +// asynchronous call entry: +// - open the try clause; +// - call callStart(). +#define AsyncCallEnter(method) \ + try { \ + if (!callStart(#method)) \ + return; + +// asynchronous call exit: +// - close the try clause; +// - catch exceptions; +// - let callEnd() handle transaction termination conditions +#define AsyncCallExit() \ + } \ + catch (const TextException &e) { \ + callException(e); \ + } \ + callEnd(); + + +#endif /* SQUID_ASYNC_JOB_H */ Index: squid3/src/ICAP/ICAPInitiate.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/Attic/ICAPInitiate.cc,v retrieving revision 1.1.2.1 retrieving revision 1.1.2.2 diff -u -r1.1.2.1 -r1.1.2.2 --- squid3/src/ICAP/ICAPInitiate.cc 23 Apr 2007 16:23:52 -0000 1.1.2.1 +++ squid3/src/ICAP/ICAPInitiate.cc 25 Apr 2007 21:23:48 -0000 1.1.2.2 @@ -7,42 +7,68 @@ #include "ICAPInitiator.h" #include "ICAPInitiate.h" -/* Event data and callback wrapper to call noteIcapAnswer with - * the adapted headers as a parameter. +/* The call objects below are not cbdata-protected or refcounted because + * nobody holds a pointer to them except for the event queue. * - * The call object is not cbdata-protected or refcounted because nobody - * holds a pointer to it except for the event queue. The call does check - * the Initiator pointer to see if that is still valid. + * The calls do check the Initiator pointer to see if that is still valid. * * TODO: convert this to a generic AsyncCall1 class - * TODO: mempool this class + * TODO: mempool kids of this class. + */ + +/* Event data and callback wrapper to call noteIcapAnswer with + * the answer message as a parameter. */ class ICAPAnswerCall { public: - // use this function to make an asynchronous call: - static void Schedule(ICAPInitiator *anInitiator, HttpMsg *aMessage); + // use this function to make an asynchronous call + static void Schedule(const ICAPInitiatorHolder &anInitiator, HttpMsg *aMessage); static void Wrapper(void *data); protected: - ICAPAnswerCall(ICAPInitiator *anInitiator, HttpMsg *aMessage); + ICAPAnswerCall(const ICAPInitiatorHolder &anInitiator, HttpMsg *aMessage); ~ICAPAnswerCall(); void schedule(); void call(); - ICAPInitiator *theInitiator; + ICAPInitiatorHolder theInitiator; HttpMsg *theMessage; }; +/* Event data and callback wrapper to call noteIcapQueryAbort with + * the termination status as a parameter. + * + * XXX: This class is a clone of ICAPAnswerCall. + */ +class ICAPQueryAbortCall { +public: + // use this function to make an asynchronous call + static void Schedule(const ICAPInitiatorHolder &anInitiator, bool beFinal); + + static void Wrapper(void *data); + +protected: + ICAPQueryAbortCall(const ICAPInitiatorHolder &anInitiator, bool beFinal); + + void schedule(); + void call(); + + ICAPInitiatorHolder theInitiator; + bool isFinal; +}; + + /* ICAPInitiate */ -ICAPInitiate::ICAPInitiate(ICAPInitiator *anInitiator, ICAPServiceRep::Pointer &aService): - theInitiator(cbdataReference(anInitiator)), theService(aService) +ICAPInitiate::ICAPInitiate(const char *aTypeName, + ICAPInitiator *anInitiator, ICAPServiceRep::Pointer &aService): + AsyncJob(aTypeName), theInitiator(anInitiator), theService(aService) { assert(theService != NULL); - debugs(93,7, "ICAPInitiate initialized." << status()); + assert(theInitiator); } ICAPInitiate::~ICAPInitiate() @@ -56,8 +82,8 @@ debugs(93, 5, HERE << "swan sings" << status()); if (theInitiator) { - debugs(93, 3, HERE << "probably failed; sending abort notification"); - tellQueryAborted(); + debugs(93, 3, HERE << "fatal failure; sending abort notification"); + tellQueryAborted(true); // final by default } debugs(93, 5, HERE << "swan sang" << status()); @@ -65,7 +91,8 @@ void ICAPInitiate::clearInitiator() { - cbdataReferenceDone(theInitiator); + if (theInitiator) + theInitiator.clear(); } void ICAPInitiate::sendAnswer(HttpMsg *msg) @@ -74,9 +101,9 @@ clearInitiator(); } -void ICAPInitiate::tellQueryAborted() +void ICAPInitiate::tellQueryAborted(bool final) { - AsyncCall(93,5, theInitiator, ICAPInitiator::noteIcapQueryAborted); + ICAPQueryAbortCall::Schedule(theInitiator, final); clearInitiator(); } @@ -91,13 +118,51 @@ } +/* ICAPInitiatorHolder */ + +ICAPInitiatorHolder::ICAPInitiatorHolder(ICAPInitiator *anInitiator): + ptr(0), cbdata(0) +{ + if (anInitiator) { + cbdata = cbdataReference(anInitiator->toCbdata()); + ptr = anInitiator; + } +} + +ICAPInitiatorHolder::ICAPInitiatorHolder(const ICAPInitiatorHolder &anInitiator): + ptr(0), cbdata(0) +{ + if (anInitiator != NULL && cbdataReferenceValid(anInitiator.cbdata)) { + cbdata = cbdataReference(anInitiator.cbdata); + ptr = anInitiator.ptr; + } +} + +ICAPInitiatorHolder::~ICAPInitiatorHolder() +{ + clear(); +} + +void ICAPInitiatorHolder::clear() { + if (ptr) { + ptr = NULL; + cbdataReferenceDone(cbdata); + } +} + +// should not be used +ICAPInitiatorHolder &ICAPInitiatorHolder::operator =(const ICAPInitiatorHolder &anInitiator) +{ + assert(false); + return *this; +} + /* ICAPAnswerCall */ -ICAPAnswerCall::ICAPAnswerCall(ICAPInitiator *anInitiator, HttpMsg *aMessage): - theInitiator(0), theMessage(0) +ICAPAnswerCall::ICAPAnswerCall(const ICAPInitiatorHolder &anInitiator, HttpMsg *aMessage): + theInitiator(anInitiator), theMessage(0) { - if (anInitiator && cbdataReferenceValid(anInitiator)) { - theInitiator = cbdataReference(anInitiator); + if (theInitiator) { assert(aMessage); theMessage = HTTPMSGLOCK(aMessage); } @@ -120,10 +185,8 @@ ICAPAnswerCall::~ICAPAnswerCall() { - if (theInitiator) { - cbdataReferenceDone(theInitiator); + if (theInitiator) HTTPMSGUNLOCK(theMessage); - } } void ICAPAnswerCall::Wrapper(void *data) @@ -136,11 +199,11 @@ void ICAPAnswerCall::call() { assert(theInitiator); - if (cbdataReferenceValid(theInitiator)) { + if (cbdataReferenceValid(theInitiator.cbdata)) { debugs(93, 3, "entering " << theInitiator << "->ICAPInitiator::noteIcapAnswer(" << theMessage << ")"); - theInitiator->noteIcapAnswer(theMessage); + theInitiator.ptr->noteIcapAnswer(theMessage); debugs(93, 3, "exiting " << theInitiator << "->ICAPInitiator::noteIcapAnswer(" << theMessage << ")"); @@ -151,9 +214,64 @@ } } -void ICAPAnswerCall::Schedule(ICAPInitiator *anInitiator, HttpMsg *aMessage) +void ICAPAnswerCall::Schedule(const ICAPInitiatorHolder &anInitiator, HttpMsg *aMessage) { ICAPAnswerCall *call = new ICAPAnswerCall(anInitiator, aMessage); call->schedule(); // The call object is deleted in ICAPAnswerCall::Wrapper } + + +/* ICAPQueryAbortCall */ + +ICAPQueryAbortCall::ICAPQueryAbortCall(const ICAPInitiatorHolder &anInitiator, bool beFinal): + theInitiator(anInitiator), isFinal(beFinal) +{ +} + +void ICAPQueryAbortCall::schedule() +{ + if (theInitiator) { + debugs(93,3, __FILE__ << "(" << __LINE__ << ") will call " << + theInitiator << "->ICAPInitiator::noteIcapQueryAbort(" << + isFinal << ")"); + eventAdd("ICAPInitiator::noteIcapQueryAbort", + &ICAPQueryAbortCall::Wrapper, this, 0.0, 0, false); + } else { + debugs(93,3, __FILE__ << "(" << __LINE__ << ") will not call " << + theInitiator << "->ICAPInitiator::noteIcapQueryAbort(" << + isFinal << ")"); + } +} + +void ICAPQueryAbortCall::Wrapper(void *data) +{ + assert(data); + ICAPQueryAbortCall *c = static_cast(data); + c->call(); + delete c; +} + +void ICAPQueryAbortCall::call() { + assert(theInitiator); + if (cbdataReferenceValid(theInitiator.cbdata)) { + debugs(93, 3, "entering " << + theInitiator << "->ICAPInitiator::noteIcapQueryAbort(" << + isFinal << ")"); + theInitiator.ptr->noteIcapQueryAbort(isFinal); + debugs(93, 3, "exiting " << + theInitiator << "->ICAPInitiator::noteIcapQueryAbort(" << + isFinal << ")"); + } else { + debugs(93, 3, "ignoring " << + theInitiator << "->ICAPInitiator::noteIcapQueryAbort(" << + isFinal << ")"); + } +} + +void ICAPQueryAbortCall::Schedule(const ICAPInitiatorHolder &anInitiator, bool beFinal) +{ + ICAPQueryAbortCall *call = new ICAPQueryAbortCall(anInitiator, beFinal); + call->schedule(); + // The call object is deleted in ICAPQueryAbortCall::Wrapper +} Index: squid3/src/ICAP/ICAPInitiate.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/Attic/ICAPInitiate.h,v retrieving revision 1.1.2.1 retrieving revision 1.1.2.2 diff -u -r1.1.2.1 -r1.1.2.2 --- squid3/src/ICAP/ICAPInitiate.h 23 Apr 2007 16:23:52 -0000 1.1.2.1 +++ squid3/src/ICAP/ICAPInitiate.h 25 Apr 2007 21:23:48 -0000 1.1.2.2 @@ -1,6 +1,6 @@ /* - * $Id: ICAPInitiate.h,v 1.1.2.1 2007/04/23 16:23:52 rousskov Exp $ + * $Id: ICAPInitiate.h,v 1.1.2.2 2007/04/25 21:23:48 rousskov Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -37,38 +37,53 @@ #include "comm.h" #include "MemBuf.h" #include "ICAPServiceRep.h" -#include "AsyncCall.h" +#include "AsyncJob.h" + +class HttpMsg; +class ICAPInitiator; + +/* Initiator holder associtates an initiator with its cbdata. It is used as + * a temporary hack to make cbdata work with multiple inheritance */ +class ICAPInitiatorHolder { +public: + ICAPInitiatorHolder(ICAPInitiator *anInitiator); + ICAPInitiatorHolder(const ICAPInitiatorHolder &anInitiator); + ~ICAPInitiatorHolder(); + + + void clear(); + + // to make comparison with NULL possible + operator void*() { return ptr; } + bool operator == (void *) const { return ptr == NULL; } + bool operator != (void *) const { return ptr != NULL; } + bool operator !() const { return !ptr; } + + ICAPInitiator *ptr; + void *cbdata; + +private: + ICAPInitiatorHolder &operator =(const ICAPInitiatorHolder &anInitiator); +}; /* * The ICAP Initiate is a common base for ICAP queries or transactions * initiated by an ICAPInitiator. This interface exists to allow an ICAP - * initiator to signal its queries or transactions that it is aborting and - * no longer expecting an ICAP answer. The class is also handy for - * implementing common initiate actions such as maintaining and notifying - * the initiator. + * initiator to signal its "initiatees" that it is aborting and no longer + * expecting an answer. The class is also handy for implementing common + * initiate actions such as maintaining and notifying the initiator. * * ICAPInitiate implementations must cbdata-protect themselves. * - * Currently, only ICAPXaction is using this class, but this will change - * when we start supporting multiple ICAP transactions per ICAP query. - * * This class could have been named ICAPInitiatee. */ - -class HttpMsg; -class ICAPInitiator; - -class ICAPInitiate +class ICAPInitiate: public AsyncJob { public: - ICAPInitiate(ICAPInitiator *anInitiator, ICAPServiceRep::Pointer &aService); + ICAPInitiate(const char *aTypeName, ICAPInitiator *anInitiator, ICAPServiceRep::Pointer &aService); virtual ~ICAPInitiate(); - // start handler, treat as protected - virtual void start() = 0; - AsyncCallWrapper(93,3, ICAPInitiate, start); - // communication with the initiator virtual void noteInitiatorAborted() = 0; AsyncCallWrapper(93,3, ICAPInitiate, noteInitiatorAborted) @@ -77,14 +92,14 @@ ICAPServiceRep &service(); void sendAnswer(HttpMsg *msg); // send to the initiator - void tellQueryAborted(); // tell initiator + void tellQueryAborted(bool final); // tell initiator void clearInitiator(); // used by noteInitiatorAborted; TODO: make private virtual void swanSong(); // internal cleanup virtual const char *status() const; // for debugging - ICAPInitiator *theInitiator; + ICAPInitiatorHolder theInitiator; ICAPServiceRep::Pointer theService; }; Index: squid3/src/ICAP/ICAPInitiator.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPInitiator.cc,v retrieving revision 1.1.2.2 retrieving revision 1.1.2.3 diff -u -r1.1.2.2 -r1.1.2.3 --- squid3/src/ICAP/ICAPInitiator.cc 23 Apr 2007 16:23:52 -0000 1.1.2.2 +++ squid3/src/ICAP/ICAPInitiator.cc 25 Apr 2007 21:23:49 -0000 1.1.2.3 @@ -6,21 +6,20 @@ #include "ICAPXaction.h" #include "ICAPInitiator.h" -ICAPXaction *ICAPInitiator::startIcapXaction(ICAPXaction *x) { - assert(x); +ICAPInitiate *ICAPInitiator::initiateIcap(ICAPInitiate *x) { cbdataReference(x); - return ICAPXaction::AsyncStart(x); + return dynamic_cast(ICAPInitiate::AsyncStart(x)); } -void ICAPInitiator::clearIcapXaction(ICAPXaction *&x) { +void ICAPInitiator::clearIcap(ICAPInitiate *&x) { assert(x); cbdataReferenceDone(x); } -void ICAPInitiator::announceInitiatorAbort(ICAPXaction *&x) +void ICAPInitiator::announceInitiatorAbort(ICAPInitiate *&x) { if (x) { - AsyncCall(93,5, x, ICAPXaction::noteInitiatorAborted); - clearIcapXaction(x); + AsyncCall(93,5, x, ICAPInitiate::noteInitiatorAborted); + clearIcap(x); } } Index: squid3/src/ICAP/ICAPInitiator.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPInitiator.h,v retrieving revision 1.1.2.2 retrieving revision 1.1.2.3 diff -u -r1.1.2.2 -r1.1.2.3 --- squid3/src/ICAP/ICAPInitiator.h 23 Apr 2007 16:23:53 -0000 1.1.2.2 +++ squid3/src/ICAP/ICAPInitiator.h 25 Apr 2007 21:23:49 -0000 1.1.2.3 @@ -1,6 +1,6 @@ /* - * $Id: ICAPInitiator.h,v 1.1.2.2 2007/04/23 16:23:53 rousskov Exp $ + * $Id: ICAPInitiator.h,v 1.1.2.3 2007/04/25 21:23:49 rousskov Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -34,8 +34,6 @@ #ifndef SQUID_ICAPINITIATOR_H #define SQUID_ICAPINITIATOR_H -#include "AsyncCall.h" - /* * The ICAP Initiator is an ICAP vectoring point that initates ICAP * transactions. This interface exists to allow ICAP transactions to @@ -46,7 +44,7 @@ */ class HttpMsg; -class ICAPXaction; +class ICAPInitiate; class ICAPInitiator { @@ -57,17 +55,20 @@ virtual void noteIcapAnswer(HttpMsg *message) = 0; // called when valid ICAP response headers are no longer expected - virtual void noteIcapQueryAborted() = 0; - AsyncCallWrapper(93,3, ICAPInitiator, noteIcapQueryAborted); + // the final parameter is set to disable bypass or retries + virtual void noteIcapQueryAbort(bool final) = 0; + + // a temporary cbdata-for-multiple inheritance hack, see ICAPInitiator.cc + virtual void *toCbdata() { return this; } protected: - ICAPXaction *startIcapXaction(ICAPXaction *x); // locks and returns x + ICAPInitiate *initiateIcap(ICAPInitiate *x); // locks and returns x // done with x (and not calling announceInitiatorAbort) - void clearIcapXaction(ICAPXaction *&x); // unlocks x + void clearIcap(ICAPInitiate *&x); // unlocks x // inform the transaction about abnormal termination and clear it - void announceInitiatorAbort(ICAPXaction *&x); // unlocks x + void announceInitiatorAbort(ICAPInitiate *&x); // unlocks x }; #endif /* SQUID_ICAPINITIATOR_H */ --- /dev/null Thu May 3 00:17:57 2007 +++ squid3/src/ICAP/ICAPLauncher.cc Thu May 3 00:17:57 2007 @@ -0,0 +1,96 @@ +/* + * DEBUG: section 93 ICAP (RFC 3507) Client + */ + +#include "squid.h" +#include "TextException.h" +#include "HttpMsg.h" +#include "ICAPLauncher.h" +#include "ICAPXaction.h" + + +ICAPLauncher::ICAPLauncher(const char *aTypeName, ICAPInitiator *anInitiator, ICAPServiceRep::Pointer &aService): + ICAPInitiate(aTypeName, anInitiator, aService), + theXaction(0), theLaunches(0) +{ +} + +ICAPLauncher::~ICAPLauncher() +{ + assert(!theXaction); +} + +void ICAPLauncher::start() +{ + ICAPInitiate::start(); + + Must(theInitiator); + launchXaction(false); +} + +void ICAPLauncher::launchXaction(bool final) +{ + Must(!theXaction); + ++theLaunches; + debugs(93,4, HERE << "launching xaction #" << theLaunches); + ICAPXaction *x = createXaction(); + if (final) + x->disableRetries(); + theXaction = initiateIcap(x); + Must(theXaction); +} + +void ICAPLauncher::noteIcapAnswer(HttpMsg *message) +{ + AsyncCallEnter(noteIcapAnswer); + + sendAnswer(message); + clearIcap(theXaction); + Must(done()); + + AsyncCallExit(); +} + +void ICAPLauncher::noteInitiatorAborted() +{ + AsyncCallEnter(noteInitiatorAborted); + + announceInitiatorAbort(theXaction); // propogate to the transaction + clearInitiator(); + Must(done()); // should be nothing else to do + + AsyncCallExit(); +} + +void ICAPLauncher::noteIcapQueryAbort(bool final) +{ + AsyncCallEnter(noteQueryAbort); + + clearIcap(theXaction); + + // TODO: add more checks from FwdState::checkRetry()? + if (!final && theLaunches < 2 && !shutting_down) { + launchXaction(true); + } else { + debugs(93,3, HERE << "cannot retry the failed ICAP xaction; tries: " << + theLaunches << "; final: " << final); + Must(done()); // swanSong will notify the initiator + } + + AsyncCallExit(); +} + +bool ICAPLauncher::doneAll() const { + return (!theInitiator || !theXaction) && ICAPInitiate::doneAll(); +} + +void ICAPLauncher::swanSong() +{ + if (theInitiator) + tellQueryAborted(!service().bypass); + + if (theXaction) + clearIcap(theXaction); + + ICAPInitiate::swanSong(); +} --- /dev/null Thu May 3 00:17:57 2007 +++ squid3/src/ICAP/ICAPLauncher.h Thu May 3 00:17:57 2007 @@ -0,0 +1,96 @@ + +/* + * $Id: ICAPLauncher.h,v 1.1.2.1 2007/04/25 21:23:48 rousskov Exp $ + * + * + * SQUID Web Proxy Cache http://www.squid-cache.org/ + * ---------------------------------------------------------- + * + * Squid is the result of efforts by numerous individuals from + * the Internet community; see the CONTRIBUTORS file for full + * details. Many organizations have provided support for Squid's + * development; see the SPONSORS file for full details. Squid is + * Copyrighted (C) 2001 by the Regents of the University of + * California; see the COPYRIGHT file for full details. Squid + * incorporates software developed and/or copyrighted by other + * sources; see the CREDITS file for full details. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program; if not, write to the Free Software + * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111, USA. + * + */ + +#ifndef SQUID_ICAPLAUNCHER_H +#define SQUID_ICAPLAUNCHER_H + +#include "ICAP/ICAPInitiator.h" +#include "ICAP/ICAPInitiate.h" + +/* + * The ICAP Launcher starts an ICAP transaction. If the transaction fails + * due to what looks like a persistent connection race condition, the launcher + * starts a new ICAP transaction using a freshly opened connection. + * + * ICAPLauncher and one or more ICAP transactions initiated by it form an + * ICAP "query". + * + * An ICAP Initiator deals with the ICAP Launcher and not an individual ICAP + * transaction because the latter may disappear and be replaced by another + * transaction. + * + * Specific ICAP launchers implement the createXaction() method to create + * REQMOD, RESPMOD, or OPTIONS transaction from initiator-supplied data. + * + * TODO: This class might be the right place to initiate ICAP ACL checks or + * implement more sophisticated ICAP transaction handling like chaining of + * ICAP transactions. + */ + +class ICAPXaction; + +// Note: ICAPInitiate must be the first parent for cbdata to work. We use +// a temporary ICAPInitaitorHolder/toCbdata hacks and do not call cbdata +// operations on the initiator directly. +class ICAPLauncher: public ICAPInitiate, public ICAPInitiator +{ +public: + ICAPLauncher(const char *aTypeName, ICAPInitiator *anInitiator, ICAPServiceRep::Pointer &aService); + virtual ~ICAPLauncher(); + + // ICAPInitiate: asynchronous communication with the initiator + void noteInitiatorAborted(); + + // ICAPInitiator: asynchronous communication with the current transaction + virtual void noteIcapAnswer(HttpMsg *message); + virtual void noteIcapQueryAbort(bool final); + + // a temporary cbdata-for-multiple inheritance hack, see ICAPInitiator.cc + virtual void *toCbdata() { return this; } + +protected: + // ICAPInitiate API implementation + virtual void start(); + virtual bool doneAll() const; + virtual void swanSong(); + + // creates the right ICAP transaction using stored configuration params + virtual ICAPXaction *createXaction() = 0; + + void launchXaction(bool final); + + ICAPInitiate *theXaction; // current ICAP transaction + int theLaunches; // the number of transaction launches +}; + +#endif /* SQUID_ICAPLAUNCHER_H */ Index: squid3/src/ICAP/ICAPModXact.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPModXact.cc,v retrieving revision 1.1.2.26 retrieving revision 1.1.2.27 diff -u -r1.1.2.26 -r1.1.2.27 --- squid3/src/ICAP/ICAPModXact.cc 23 Apr 2007 16:23:53 -0000 1.1.2.26 +++ squid3/src/ICAP/ICAPModXact.cc 25 Apr 2007 21:23:49 -0000 1.1.2.27 @@ -25,6 +25,7 @@ // TODO: replace gotEncapsulated() with something faster; we call it often CBDATA_CLASS_INIT(ICAPModXact); +CBDATA_CLASS_INIT(ICAPModXactLauncher); static const size_t TheBackupLimit = BodyPipe::MaxCapacity; @@ -65,8 +66,6 @@ // initiator wants us to start void ICAPModXact::start() { - ICAPXaction_Enter(start); - ICAPXaction::start(); estimateVirginBody(); // before virgin disappears! @@ -78,11 +77,9 @@ else waitForService(); - // XXX: but this has to be here to catch other errors. Thus, if - // commConnectStart in startWriting fails, we may get here + // XXX: If commConnectStart in startWriting fails, we may get here //_after_ the object got destroyed. Somebody please fix commConnectStart! - // XXX: Is the above comment still valid? - ICAPXaction_Exit(); + // TODO: Does re-entrance protection in callStart() solve the above? } static @@ -108,9 +105,12 @@ Must(state.serviceWaiting); state.serviceWaiting = false; - Must(service().up()); - - startWriting(); + if (service().up()) { + startWriting(); + } else { + disableRetries(); + mustStop("ICAP service unusable"); + } ICAPXaction_Exit(); } @@ -365,6 +365,7 @@ " virgin body bytes"); bp.consume(size); virginConsumed += size; + disableRetries(); } } @@ -949,15 +950,13 @@ debugs(93, 5, HERE << "swan sings" << status()); stopWriting(false); - stopBackup(); + stopSending(false); if (icapReply) { delete icapReply; icapReply = NULL; } - stopSending(false); - ICAPXaction::swanSong(); } @@ -1387,3 +1386,18 @@ return true; } + + +/* ICAPModXactLauncher */ + +ICAPModXactLauncher::ICAPModXactLauncher(ICAPInitiator *anInitiator, HttpMsg *virginHeader, HttpRequest *virginCause, ICAPServiceRep::Pointer &aService): + ICAPLauncher("ICAPModXactLauncher", anInitiator, aService) +{ + virgin.setHeader(virginHeader); + virgin.setCause(virginCause); +} + +ICAPXaction *ICAPModXactLauncher::createXaction() +{ + return new ICAPModXact(this, virgin.header, virgin.cause, theService); +} Index: squid3/src/ICAP/ICAPModXact.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPModXact.h,v retrieving revision 1.1.2.12 retrieving revision 1.1.2.13 diff -u -r1.1.2.12 -r1.1.2.13 --- squid3/src/ICAP/ICAPModXact.h 23 Apr 2007 16:23:53 -0000 1.1.2.12 +++ squid3/src/ICAP/ICAPModXact.h 25 Apr 2007 21:23:50 -0000 1.1.2.13 @@ -1,6 +1,6 @@ /* - * $Id: ICAPModXact.h,v 1.1.2.12 2007/04/23 16:23:53 rousskov Exp $ + * $Id: ICAPModXact.h,v 1.1.2.13 2007/04/25 21:23:50 rousskov Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -34,9 +34,10 @@ #ifndef SQUID_ICAPMODXACT_H #define SQUID_ICAPMODXACT_H +#include "BodyPipe.h" #include "ICAPXaction.h" #include "ICAPInOut.h" -#include "BodyPipe.h" +#include "ICAPLauncher.h" /* * ICAPModXact implements ICAP REQMOD and RESPMOD transaction using @@ -127,9 +128,6 @@ { public: - typedef RefCount Pointer; - -public: ICAPModXact(ICAPInitiator *anInitiator, HttpMsg *virginHeader, HttpRequest *virginCause, ICAPServiceRep::Pointer &s); // BodyProducer methods @@ -289,7 +287,20 @@ CBDATA_CLASS2(ICAPModXact); }; -// destroys the transaction; implemented in ICAPClient.cc (ick?) -extern void ICAPNoteXactionDone(ICAPModXact::Pointer x); +// An ICAPLauncher that stores ICAPModXact construction info and +// creates ICAPModXact when needed +class ICAPModXactLauncher: public ICAPLauncher +{ +public: + ICAPModXactLauncher(ICAPInitiator *anInitiator, HttpMsg *virginHeader, HttpRequest *virginCause, ICAPServiceRep::Pointer &s); + +protected: + virtual ICAPXaction *createXaction(); + + ICAPInOut virgin; + +private: + CBDATA_CLASS2(ICAPModXactLauncher); +}; #endif /* SQUID_ICAPMOD_XACT_H */ Index: squid3/src/ICAP/ICAPOptXact.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPOptXact.cc,v retrieving revision 1.1.2.8 retrieving revision 1.1.2.9 diff -u -r1.1.2.8 -r1.1.2.9 --- squid3/src/ICAP/ICAPOptXact.cc 23 Apr 2007 16:23:54 -0000 1.1.2.8 +++ squid3/src/ICAP/ICAPOptXact.cc 25 Apr 2007 21:23:50 -0000 1.1.2.9 @@ -11,6 +11,8 @@ #include "TextException.h" CBDATA_CLASS_INIT(ICAPOptXact); +CBDATA_CLASS_INIT(ICAPOptXactLauncher); + ICAPOptXact::ICAPOptXact(ICAPInitiator *anInitiator, ICAPServiceRep::Pointer &aService): ICAPXaction("ICAPOptXact", anInitiator, aService) @@ -19,15 +21,9 @@ void ICAPOptXact::start() { - ICAPXaction_Enter(start); - ICAPXaction::start(); - Must(cbdataReferenceValid(this)); // set by AsyncStart - openConnection(); - - ICAPXaction_Exit(); } void ICAPOptXact::handleCommConnected() @@ -88,3 +84,15 @@ return r; } + +/* ICAPOptXactLauncher */ + +ICAPOptXactLauncher::ICAPOptXactLauncher(ICAPInitiator *anInitiator, ICAPServiceRep::Pointer &aService): + ICAPLauncher("ICAPOptXactLauncher", anInitiator, aService) +{ +} + +ICAPXaction *ICAPOptXactLauncher::createXaction() +{ + return new ICAPOptXact(this, theService); +} Index: squid3/src/ICAP/ICAPOptXact.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPOptXact.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/ICAP/ICAPOptXact.h 23 Apr 2007 16:23:54 -0000 1.1.2.5 +++ squid3/src/ICAP/ICAPOptXact.h 25 Apr 2007 21:23:50 -0000 1.1.2.6 @@ -1,5 +1,5 @@ /* - * $Id: ICAPOptXact.h,v 1.1.2.5 2007/04/23 16:23:54 rousskov Exp $ + * $Id: ICAPOptXact.h,v 1.1.2.6 2007/04/25 21:23:50 rousskov Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -34,9 +34,10 @@ #define SQUID_ICAPOPTXACT_H #include "ICAPXaction.h" +#include "ICAPLauncher.h" class ICAPOptions; -class HttpMsg; + /* ICAPOptXact sends an ICAP OPTIONS request to the ICAP service, * parses the ICAP response, and sends it to the initiator. A NULL response @@ -64,4 +65,18 @@ CBDATA_CLASS2(ICAPOptXact); }; +// An ICAPLauncher that stores ICAPOptXact construction info and +// creates ICAPOptXact when needed +class ICAPOptXactLauncher: public ICAPLauncher +{ +public: + ICAPOptXactLauncher(ICAPInitiator *anInitiator, ICAPServiceRep::Pointer &aService); + +protected: + virtual ICAPXaction *createXaction(); + +private: + CBDATA_CLASS2(ICAPOptXactLauncher); +}; + #endif /* SQUID_ICAPOPTXACT_H */ Index: squid3/src/ICAP/ICAPXaction.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPXaction.cc,v retrieving revision 1.1.2.13 retrieving revision 1.1.2.14 diff -u -r1.1.2.13 -r1.1.2.14 --- squid3/src/ICAP/ICAPXaction.cc 23 Apr 2007 16:23:55 -0000 1.1.2.13 +++ squid3/src/ICAP/ICAPXaction.cc 25 Apr 2007 21:23:50 -0000 1.1.2.14 @@ -59,23 +59,15 @@ ICAPXaction_fromData(data).noteCommRead(status, size); } -ICAPXaction *ICAPXaction::AsyncStart(ICAPXaction *x) { - assert(x != NULL); - cbdataReference(x); // unlocked when done() in callEnd() - AsyncCall(93,5, x, ICAPXaction::start); - return x; -} - ICAPXaction::ICAPXaction(const char *aTypeName, ICAPInitiator *anInitiator, ICAPServiceRep::Pointer &aService): - ICAPInitiate(anInitiator, aService), + ICAPInitiate(aTypeName, anInitiator, aService), id(++TheLastId), connection(-1), commBuf(NULL), commBufSize(0), commEof(false), reuseConnection(true), - connector(NULL), reader(NULL), writer(NULL), closer(NULL), - typeName(aTypeName), - inCall(NULL) + isRetriable(true), + connector(NULL), reader(NULL), writer(NULL), closer(NULL) { debugs(93,3, typeName << " constructed, this=" << this << " [icapx" << id << ']'); // we should not call virtual status() here @@ -87,11 +79,15 @@ " [icapx" << id << ']'); // we should not call virtual status() here } +void ICAPXaction::disableRetries() { + debugs(93,5, typeName << (isRetriable ? "becomes" : "remains") << + " final" << status()); + isRetriable = false; +} + void ICAPXaction::start() { - debugs(93,3, HERE << typeName << " starts" << status()); - - Must(cbdataReferenceValid(this)); // locked by AsyncStart + ICAPInitiate::start(); readBuf.init(SQUID_TCP_SO_RCVBUF, SQUID_TCP_SO_RCVBUF); commBuf = (char*)memAllocBuf(SQUID_TCP_SO_RCVBUF, &commBufSize); @@ -102,29 +98,33 @@ // TODO: obey service-specific, OPTIONS-reported connection limit void ICAPXaction::openConnection() { + Must(connection < 0); + const ICAPServiceRep &s = service(); - // TODO: check whether NULL domain is appropriate here - connection = icapPconnPool->pop(s.host.buf(), s.port, NULL, NULL); - if (connection >= 0) { - debugs(93,3, HERE << "reused pconn FD " << connection); - connector = &ICAPXaction_noteCommConnected; // make doneAll() false - eventAdd("ICAPXaction::reusedConnection", + // if we cannot retry, we must not reuse pconns because of race conditions + if (isRetriable) { + // TODO: check whether NULL domain is appropriate here + connection = icapPconnPool->pop(s.host.buf(), s.port, NULL, NULL); + + if (connection >= 0) { + debugs(93,3, HERE << "reused pconn FD " << connection); + connector = &ICAPXaction_noteCommConnected; // make doneAll() false + eventAdd("ICAPXaction::reusedConnection", reusedConnection, this, 0.0, 0, true); - return; + return; + } } - if (connection < 0) { - connection = comm_open(SOCK_STREAM, 0, getOutgoingAddr(NULL), 0, - COMM_NONBLOCKING, s.uri.buf()); + connection = comm_open(SOCK_STREAM, 0, getOutgoingAddr(NULL), 0, + COMM_NONBLOCKING, s.uri.buf()); - if (connection < 0) - dieOnConnectionFailure(); // throws - } + if (connection < 0) + dieOnConnectionFailure(); // throws debugs(93,3, typeName << " opens connection to " << s.host.buf() << ":" << s.port); @@ -170,6 +170,7 @@ debugs(93,3, HERE << "pushing pconn" << status()); commSetTimeout(connection, -1, NULL, NULL); icapPconnPool->push(connection, theService->host.buf(), theService->port, NULL, NULL); + disableRetries(); } else { debugs(93,3, HERE << "closing pconn" << status()); // comm_close will clear timeout @@ -272,15 +273,18 @@ mustStop("ICAP service connection externally closed"); } -bool ICAPXaction::done() const +void ICAPXaction::callEnd() { - // stopReason, set in mustStop(), overwrites all other conditions - return stopReason != NULL || doneAll(); + if (doneWithIo()) { + debugs(93, 5, HERE << typeName << " done with I/O" << status()); + closeConnection(); + } + ICAPInitiate::callEnd(); // may destroy us } bool ICAPXaction::doneAll() const { - return !connector && !reader && !writer; + return !connector && !reader && !writer && ICAPInitiate::doneAll(); } void ICAPXaction::updateTimeout() { @@ -332,10 +336,13 @@ * here instead of reading directly into readBuf.buf. */ - if (sz > 0) + if (sz > 0) { readBuf.append(commBuf, sz); - else + disableRetries(); + } else { + reuseConnection = false; commEof = true; + } handleCommRead(sz); @@ -412,18 +419,6 @@ ICAPXaction_Exit(); } -void ICAPXaction::mustStop(const char *aReason) -{ - Must(inCall); // otherwise nobody will delete us if we are done() - Must(aReason); - if (!stopReason) { - stopReason = aReason; - debugs(93, 5, typeName << " will stop, reason: " << stopReason); - } else { - debugs(93, 5, typeName << " will stop, another reason: " << aReason); - } -} - // This 'last chance' method is called before a 'done' transaction is deleted. // It is wrong to call virtual methods from a destructor. Besides, this call // indicates that the transaction will terminate as planned. @@ -439,61 +434,10 @@ if (commBuf) memFreeBuf(commBufSize, commBuf); - ICAPInitiate::swanSong(); -} - -bool ICAPXaction::callStart(const char *method) -{ - debugs(93, 5, typeName << "::" << method << " called" << status()); - - if (inCall) { - // this may happen when we have bugs or when arguably buggy - // comm interface calls us while we are closing the connection - debugs(93, 5, HERE << typeName << "::" << inCall << - " is in progress; " << typeName << "::" << method << - " cancels reentry."); - return false; - } - - inCall = method; - return true; -} - -void ICAPXaction::callException(const TextException &e) -{ - debugs(93, 2, typeName << "::" << inCall << " caught an exception: " << - e.message << ' ' << status()); - - reuseConnection = false; // be conservative - mustStop("exception"); -} + if (theInitiator) + tellQueryAborted(!isRetriable); -void ICAPXaction::callEnd() -{ - if (done()) { - debugs(93, 5, typeName << "::" << inCall << " ends xaction " << - status()); - swanSong(); - const char *inCallSaved = inCall; - const char *typeNameSaved = typeName; - void *object = this; - inCall = NULL; - - delete this; - cbdataReferenceDone(object); // locked by AsyncStart - - // careful: this object does not exist any more - debugs(93, 6, HERE << typeNameSaved << "::" << inCallSaved << - " ended " << object); - return; - } else - if (doneWithIo()) { - debugs(93, 5, HERE << typeName << " done with I/O" << status()); - closeConnection(); - } - - debugs(93, 6, typeName << "::" << inCall << " ended" << status()); - inCall = NULL; + ICAPInitiate::swanSong(); } // returns a temporary string depicting transaction status, for debugging Index: squid3/src/ICAP/ICAPXaction.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPXaction.h,v retrieving revision 1.1.2.8 retrieving revision 1.1.2.9 diff -u -r1.1.2.8 -r1.1.2.9 --- squid3/src/ICAP/ICAPXaction.h 23 Apr 2007 16:23:55 -0000 1.1.2.8 +++ squid3/src/ICAP/ICAPXaction.h 25 Apr 2007 21:23:51 -0000 1.1.2.9 @@ -1,6 +1,6 @@ /* - * $Id: ICAPXaction.h,v 1.1.2.8 2007/04/23 16:23:55 rousskov Exp $ + * $Id: ICAPXaction.h,v 1.1.2.9 2007/04/25 21:23:51 rousskov Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -38,10 +38,8 @@ #include "MemBuf.h" #include "ICAPServiceRep.h" #include "ICAPInitiate.h" -#include "AsyncCall.h" class HttpMsg; -class TextException; /* * The ICAP Xaction implements common tasks for ICAP OPTIONS, REQMOD, and @@ -57,14 +55,11 @@ { public: - // Use this to start ICAP transactions because - // the start routine may result in failures/callbacks. - static ICAPXaction *AsyncStart(ICAPXaction *x); - -public: ICAPXaction(const char *aTypeName, ICAPInitiator *anInitiator, ICAPServiceRep::Pointer &aService); virtual ~ICAPXaction(); + void disableRetries(); + // comm handler wrappers, treat as private void noteCommConnected(comm_err_t status); void noteCommWrote(comm_err_t status, size_t sz); @@ -73,8 +68,7 @@ void noteCommClosed(); protected: - virtual void start() = 0; // has body - + virtual void start(); virtual void noteInitiatorAborted(); // TODO: move to ICAPInitiate // comm hanndlers; called by comm handler wrappers @@ -96,25 +90,26 @@ bool parseHttpMsg(HttpMsg *msg); // true=success; false=needMore; throw=err bool mayReadMore() const; + virtual bool doneReading() const; virtual bool doneWriting() const; bool doneWithIo() const; - - bool done() const; virtual bool doneAll() const; - void mustStop(const char *reason); // called just before the 'done' transaction is deleted virtual void swanSong(); // returns a temporary string depicting transaction status, for debugging - const char *status() const; + virtual const char *status() const; virtual void fillPendingStatus(MemBuf &buf) const; virtual void fillDoneStatus(MemBuf &buf) const; // useful for debugging virtual bool fillVirginHttpHeader(MemBuf&) const; + // custom end-of-call checks + virtual void callEnd(); + protected: const int id; // transaction ID for debugging, unique across ICAP xactions @@ -136,14 +131,10 @@ size_t commBufSize; bool commEof; bool reuseConnection; + bool isRetriable; const char *stopReason; - // asynchronous call maintenance - bool callStart(const char *method); - void callException(const TextException &e); - void callEnd(); - // active (pending) comm callbacks for the ICAP server connection CNCB *connector; IOCB *reader; @@ -163,25 +154,8 @@ }; // call guards for all "asynchronous" note*() methods - -// asynchronous call entry: -// - open the try clause; -// - call callStart(). -#define ICAPXaction_Enter(method) \ - try { \ - if (!callStart(#method)) \ - return; - -// asynchronous call exit: -// - close the try clause; -// - catch exceptions; -// - let callEnd() handle transaction termination conditions -#define ICAPXaction_Exit() \ - } \ - catch (const TextException &e) { \ - callException(e); \ - } \ - callEnd(); - +// If we move ICAPXaction_* macros to core, they can use these generic names: +#define ICAPXaction_Enter(method) AsyncCallEnter(method) +#define ICAPXaction_Exit() AsyncCallExit() #endif /* SQUID_ICAPXACTION_H */