From: wessels <> Date: Wed, 1 Nov 2006 06:30:55 +0000 (+0000) Subject: - Many ICAP fixes from Alex Rousskov accumulated on the X-Git-Tag: SQUID_3_0_PRE5~11 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=c99de60701b56be31c01be2045d204ed411e33ca;p=thirdparty%2Fsquid.git - Many ICAP fixes from Alex Rousskov accumulated on the sourceforge squid3-icap branch since 2006/10, including: - Polished ICAP service selection code and implemented bypass of optional services. The code implements icap_class configuration directive which is currently used as a "set of interchangeable ICAP services". Squid2 and current squid.conf may imply otherwise. - Support Transfer-* ICAP OPTIONS response header. If Squid knows that a service does not want the URL, Squid will not use the service, even if it is an essential service with bypass=0. Note that we may make this decision before we know what the service wants. Eventually, ACLs should initiate and wait for the OPTIONS transaction for yet-unprobed services. - When ICAP transactions fail to connect to the service many times, the service is suspended until the next OPTIONS update. The limit is currently hard-coded to 10. Suspended service is a down service and will be skipped by the ACL service selection algorithm. - Rewrote the code updating ICAP service options. We no longer mark the service being updated as "down". Only presence of valid and fresh options is important. We also try to update the options before they expire to avoid any service downtime or use of stale options. - Report interesting changes in the ICAP service state, some with debugging level one to alert the cache administrator. - When cloning a request during an ICAP 204 "No Content" REQMOD response, preserve the client address so that the rest of the code has access to it. This change appears to fix Squid Bug #1712. - After ICAP 100 Continue, expect new ICAP headers instead of HTTP headers. Reset ICAP message object to be ready to parse ICAP headers again. (Tsantilas Christos ) - The ieof HTTP chunk-extension was written after chunk-data instead of being written after the chunk-size. (Tsantilas Christos ) - Merged common code from the ICAPClientReqmodPrecache and ICAPClientReqmodPrecache classes into the newly added ICAPClientVector class. The specific vectors do not have a common owner (yet?) because ServerStateData and ClientHttpRequest do not have a common base class. Thus, ICAPClientVector has to rely on its kids to communicate with their owners. However, at least 50% of the logic was common and has been moved. Eventually, we may want to create a simple ICAPOwner API that ServerStateData and ClientHttpRequest can implement and ICAPClientVector can rely on. This will make the code simpler and more efficient. The big merge was motivated by a couple of bugs that were found in one vector class but that did not exist or behaved differently in the other vector, mostly likely due to natural diversion of used-to-be identical code. - Rewrote communication between a server-side ICAPClient*mod* vector and its owner. When a server-side ICAPClient*mod* vector was notifying its owner of more adapted data, the owner could delete the vector (by calling icap->ownerAbort) if the store entry was not willing to accept the data. The same deletion could happen when a vector was notifying the owner of a successful termination. In all those cases, the vector did not expect to be deleted and could continue to do something, causing segmentation faults. Now, when more data is available, the vector calls its owner and checks the return value of the call. If it is false, the vector knows it has been deleted and quits. When vector terminates, it calls its owner and trusts the owner to always delete the vector. The "check return value and quit" design is not perfect, but we are paying the price for isolating the vectors from their owners while using direct calls between them (instead of MsgPipe or a similar less efficient indirect approach we use elsewhere). - Renamed doIcap to startIcap and moved more common code there. Changed its return type to bool. We now handle three cases when ICAP ACLs call back: 1) No service was selected (because there was no applicable service or because all applicable services were broken and optional). We proceed as if ICAP was not configured. 2) The selected essential service is broken. This is a fatal transaction error and we return an "ICAP protocol error" HTTP error response. We could proceed with the ICAP stuff, but it saves a lot of cycles to abort early. 3) The selected service is not broken. We proceed with the ICAP stuff. The old code did not detect case #2, even though there was code to handle that case (with dangerous XXX suggestions that are now gone). The code should probably be polished further to move common ftp/http logic from icapAclCheckDone()s into ServerStateData. - Make sure there is an accept callback when we are accepting. If there is no callback and we accept, we will silently leak the accepted FD. When we are running out of FDs, there is often no accept callback. The old code, when running out of FDs, would create lots of "orphaned" or "forgotten" FDs that will eventually get into a CLOSED_WAIT state and remain there until Squid quits. The new code does not call accept() if there is no accept callback and does not register the accept FD for reading if the AcceptLimiter is deferring, because when the AcceptLimiter kicks in, it will register the accept FD for reading. There are most likely other places/cases where accept FD should not be registered for reading. - When an exception is caught, mark the ICAP connection as non-reusable so that it is not recycled while a write is pending but simply closed instead. Our write callback will still be called, unfortunately, because there is no way to clear the callback without invalidating its data (i.e., the transaction pointer). This change prevents pconn.cc:253: "!comm_has_incomplete_write(fd)" assertion from firing when things go wrong (e.g., the ICAP server cannot be contacted to retrieve OPTIONS). Not all exceptions caught by the ICAP xaction should lead to the ICAP connection termination, but it is very difficult if not impossible to reliably detect exceptional conditions when it is safe to reuse the ICAP connection, and it is probably not worth it anyway. - Added Tsantilas Christos to CONTRIBUTORS for fixing ICAP bugs. - Polished debugging. --- diff --git a/CONTRIBUTORS b/CONTRIBUTORS index ee72abfe57..b93ceb5068 100644 --- a/CONTRIBUTORS +++ b/CONTRIBUTORS @@ -104,5 +104,6 @@ and ideas to make this software available. Felix Meschberger Mark Bergsma Tim Starling + Tsantilas Christos Duane Wessels diff --git a/src/HttpReply.cc b/src/HttpReply.cc index 783ab97187..22f7b64ef6 100644 --- a/src/HttpReply.cc +++ b/src/HttpReply.cc @@ -1,6 +1,6 @@ /* - * $Id: HttpReply.cc,v 1.89 2006/06/07 22:39:33 hno Exp $ + * $Id: HttpReply.cc,v 1.90 2006/10/31 23:30:56 wessels Exp $ * * DEBUG: section 58 HTTP Reply (Response) * AUTHOR: Alex Rousskov @@ -94,6 +94,7 @@ HttpReply::init() httpBodyInit(&body); hdrCacheInit(); httpStatusLineInit(&sline); + pstate = psReadyToParseStartLine; do_clean = true; } diff --git a/src/HttpRequest.cc b/src/HttpRequest.cc index b8d23f2234..c2f740c81c 100644 --- a/src/HttpRequest.cc +++ b/src/HttpRequest.cc @@ -1,6 +1,6 @@ /* - * $Id: HttpRequest.cc,v 1.68 2006/09/26 13:30:09 adrian Exp $ + * $Id: HttpRequest.cc,v 1.69 2006/10/31 23:30:56 wessels Exp $ * * DEBUG: section 73 HTTP Request * AUTHOR: Duane Wessels @@ -96,6 +96,7 @@ HttpRequest::init() extacl_user = null_string; extacl_passwd = null_string; extacl_log = null_string; + pstate = psReadyToParseStartLine; } void diff --git a/src/ICAP/ChunkedCodingParser.cc b/src/ICAP/ChunkedCodingParser.cc index 06e0e8435c..6be2a2ca4b 100644 --- a/src/ICAP/ChunkedCodingParser.cc +++ b/src/ICAP/ChunkedCodingParser.cc @@ -65,7 +65,7 @@ void ChunkedCodingParser::parseChunkBeg() size_t crlfEnd = 0; if (findCrlf(crlfBeg, crlfEnd)) { - debugs(99,5, "found chunk-size end: " << crlfBeg << "-" << crlfEnd); + debugs(93,7, "found chunk-size end: " << crlfBeg << "-" << crlfEnd); int size = -1; const char *p = 0; @@ -89,7 +89,7 @@ void ChunkedCodingParser::parseChunkBeg() theIn->consume(crlfEnd); theChunkSize = theLeftBodySize = size; - debugs(99,5, "found chunk: " << theChunkSize); + debugs(93,7, "found chunk: " << theChunkSize); theStep = theChunkSize == 0 ? psTrailer : psChunkBody; return; } diff --git a/src/ICAP/ICAPClientReqmodPrecache.cc b/src/ICAP/ICAPClientReqmodPrecache.cc index 42853b6a71..b66e34cf61 100644 --- a/src/ICAP/ICAPClientReqmodPrecache.cc +++ b/src/ICAP/ICAPClientReqmodPrecache.cc @@ -1,10 +1,7 @@ #include "squid.h" #include "client_side_request.h" #include "ClientRequestContext.h" -#include "MsgPipe.h" #include "MsgPipeData.h" -#include "MsgPipeSource.h" -#include "MsgPipeSink.h" #include "HttpRequest.h" #include "ICAPClientReqmodPrecache.h" #include "ICAPServiceRep.h" @@ -12,106 +9,19 @@ CBDATA_CLASS_INIT(ICAPClientReqmodPrecache); -ICAPClientReqmodPrecache::ICAPClientReqmodPrecache(ICAPServiceRep::Pointer aService): service(aService), http(NULL), virgin(NULL), adapted(NULL) +ICAPClientReqmodPrecache::ICAPClientReqmodPrecache(ICAPServiceRep::Pointer aService): + ICAPClientVector(aService, "ICAPClientReqmodPrecache"), http(NULL) { - debug(93,3)("ICAPClientReqmodPrecache constructed, this=%p\n", this); -} - -ICAPClientReqmodPrecache::~ICAPClientReqmodPrecache() -{ - stop(notifyNone); - cbdataReferenceDone(http); - debug(93,3)("ICAPClientReqmodPrecache destructed, this=%p\n", this); - - if (virgin != NULL) - freeVirgin(); - - if (adapted != NULL) { - /* - * adapted->sink is equal to this. Remove the pointer since - * we are deleting this. - */ - - if (adapted->sink) - adapted->sink = NULL; - - freeAdapted(); - } } void ICAPClientReqmodPrecache::startReqMod(ClientHttpRequest *aHttp, HttpRequest *request) { - debug(93,3)("ICAPClientReqmodPrecache::startReqMod() called\n"); http = cbdataReference(aHttp); - - virgin = new MsgPipe("virgin"); // this is the place to create a refcount ptr - virgin->source = this; - virgin->data = new MsgPipeData; - virgin->data->cause = NULL; - virgin->data->setHeader(request); - virgin->data->body = new MemBuf; - virgin->data->body->init(ICAP::MsgPipeBufSizeMin, ICAP::MsgPipeBufSizeMax); - - adapted = new MsgPipe("adapted"); - adapted->sink = this; - - ICAPInitXaction(service, virgin, adapted); - - virgin->sendSourceStart(); // we may have virgin data to provide - adapted->sendSinkNeed(); // we want adapted response, eventially -} - -void ICAPClientReqmodPrecache::sendMoreData(StoreIOBuffer buf) -{ - debug(93,3)("ICAPClientReqmodPrecache::sendMoreData() called\n"); - //buf.dump(); - /* - * The caller is responsible for not giving us more data - * than will fit in body MemBuf. Caller should use - * potentialSpaceSize() to find out how much we can hold. - */ - virgin->data->body->append(buf.data, buf.length); - virgin->sendSourceProgress(); + startMod(http, NULL, request); } -int -ICAPClientReqmodPrecache::potentialSpaceSize() -{ - if (virgin == NULL) - return 0; - - return (int) virgin->data->body->potentialSpaceSize(); -} - -// ClientHttpRequest says we have the entire HTTP message -void ICAPClientReqmodPrecache::doneSending() -{ - debug(93,3)("ICAPClientReqmodPrecache::doneSending() called\n"); - - virgin->sendSourceFinish(); -} - -// ClientHttpRequest tells us to abort -void ICAPClientReqmodPrecache::ownerAbort() -{ - debug(93,3)("ICAPClientReqmodPrecache::ownerAbort() called\n"); - stop(notifyIcap); -} - -// ICAP client needs more virgin response data -void ICAPClientReqmodPrecache::noteSinkNeed(MsgPipe *p) -{ - debug(93,3)("ICAPClientReqmodPrecache::noteSinkNeed() called\n"); - - if (virgin->data->body->potentialSpaceSize()) - http->icapSpaceAvailable(); -} - -// ICAP client aborting -void ICAPClientReqmodPrecache::noteSinkAbort(MsgPipe *p) -{ - debug(93,3)("ICAPClientReqmodPrecache::noteSinkAbort() called\n"); - stop(notifyOwner); +void ICAPClientReqmodPrecache::tellSpaceAvailable() { + http->icapSpaceAvailable(); } // ICAP client starts sending adapted response @@ -157,7 +67,7 @@ void ICAPClientReqmodPrecache::noteSourceProgress(MsgPipe *p) HttpRequest *req = dynamic_cast(adapted->data->header); if (req) { - debugs(32,3,HERE << "notifying body_reader, contentSize() = " << p->data->body->contentSize()); + debugs(93,3,HERE << "notifying body_reader, contentSize() = " << p->data->body->contentSize()); req->body_reader->notify(p->data->body->contentSize()); } else { http->takeAdaptedBody(adapted->data->body); @@ -165,71 +75,41 @@ void ICAPClientReqmodPrecache::noteSourceProgress(MsgPipe *p) } } -// ICAP client is done sending adapted response -void ICAPClientReqmodPrecache::noteSourceFinish(MsgPipe *p) +void ICAPClientReqmodPrecache::tellDoneAdapting() { - debug(93,3)("ICAPClientReqmodPrecache::noteSourceFinish() called\n"); + debug(93,3)("ICAPClientReqmodPrecache::tellDoneAdapting() called\n"); //tell ClientHttpRequest that we expect no more response data - http->doneAdapting(); + http->doneAdapting(); // does not delete us (yet?) stop(notifyNone); + // we should be eventually deleted by owner in ~ClientHttpRequest() } -// ICAP client is aborting -void ICAPClientReqmodPrecache::noteSourceAbort(MsgPipe *p) +void ICAPClientReqmodPrecache::tellAbortAdapting() { - debug(93,3)("ICAPClientReqmodPrecache::noteSourceAbort() called\n"); - stop(notifyOwner); + debug(93,3)("ICAPClientReqmodPrecache::tellAbortAdapting() called\n"); + // tell ClientHttpRequest that we are aborting ICAP processing prematurely + http->abortAdapting(); } // internal cleanup void ICAPClientReqmodPrecache::stop(Notify notify) { - if (virgin != NULL) { - if (notify == notifyIcap) - virgin->sendSourceAbort(); - else - virgin->source = NULL; - - freeVirgin(); - } - -#if DONT_FREE_ADAPTED /* * NOTE: We do not clean up "adapted->sink" here because it may * have an HTTP message body that needs to stay around a little * while longer so that the HTTP server-side can forward it on. */ - if (adapted != NULL) { - if (notify == notifyIcap) - adapted->sendSinkAbort(); - else - adapted->sink = NULL; - - freeAdapted(); - } - -#endif - - if (http) { - if (notify == notifyOwner) - // tell ClientHttpRequest that we are aborting prematurely - http->abortAdapting(); - cbdataReferenceDone(http); + // XXX: who will clean up the "adapted->sink" then? Does it happen + // when the owner deletes us? Is that why we are deleted when the + // owner is destroyed and not when ICAP adaptation is done, like + // in http.cc case? - // http is now NULL, will not call it any more - } -} - -void ICAPClientReqmodPrecache::freeVirgin() -{ - // virgin->data->cause should be NULL; - virgin = NULL; // refcounted -} + // XXX: "adapted->sink" does not really have an "HTTP message body", + // In fact, it simply points to "this". Should the above comment + // refer to adapted and adapted->data->body? -void ICAPClientReqmodPrecache::freeAdapted() -{ - adapted = NULL; // refcounted + ICAPClientVector::clean(notify, false); } /* @@ -247,13 +127,13 @@ ICAPClientReqmodPrecache::readBody(void *data, MemBuf &mb, size_t size) assert(icap->adapted->data != NULL); MemBuf *bodybuf = icap->adapted->data->body; assert(bodybuf != NULL); - debugs(32,3,HERE << "readBody requested size " << size); - debugs(32,3,HERE << "readBody bodybuf size " << bodybuf->contentSize()); + debugs(93,3,HERE << "readBody requested size " << size); + debugs(93,3,HERE << "readBody bodybuf size " << bodybuf->contentSize()); if ((mb_size_t) size > bodybuf->contentSize()) size = bodybuf->contentSize(); - debugs(32,3,HERE << "readBody actual size " << size); + debugs(93,3,HERE << "readBody actual size " << size); assert(size); @@ -268,7 +148,7 @@ void ICAPClientReqmodPrecache::abortBody(void *data, size_t remaining) { if (remaining >= 0) { - debugs(0,0,HERE << "ICAPClientReqmodPrecache::abortBody size " << remaining); + debugs(93,1,HERE << "ICAPClientReqmodPrecache::abortBody size " << remaining); // more? } @@ -283,7 +163,7 @@ ICAPClientReqmodPrecache::abortBody(void *data, size_t remaining) void ICAPClientReqmodPrecache::kickBody(void *data) { - debugs(32,3,HERE << "ICAPClientReqmodPrecache::kickBody"); + debugs(93,3,HERE << "ICAPClientReqmodPrecache::kickBody"); ICAPClientReqmodPrecache *icap = static_cast(data); assert(icap->adapted != NULL); icap->adapted->sendSinkNeed(); diff --git a/src/ICAP/ICAPClientReqmodPrecache.h b/src/ICAP/ICAPClientReqmodPrecache.h index bf5d527357..d0285a5ea6 100644 --- a/src/ICAP/ICAPClientReqmodPrecache.h +++ b/src/ICAP/ICAPClientReqmodPrecache.h @@ -1,6 +1,6 @@ /* - * $Id: ICAPClientReqmodPrecache.h,v 1.3 2006/04/27 19:27:37 wessels Exp $ + * $Id: ICAPClientReqmodPrecache.h,v 1.4 2006/10/31 23:30:58 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -31,68 +31,54 @@ * */ -#ifndef SQUID_ICAPCLIENTSIDEHOOK_H -#define SQUID_ICAPCLIENTSIDEHOOK_H +#ifndef SQUID_ICAPCLIENTREQMODPRECACHE_H +#define SQUID_ICAPCLIENTREQMODPRECACHE_H -#include "MsgPipe.h" -#include "MsgPipeSource.h" -#include "MsgPipeSink.h" +#include "ICAPClientVector.h" -/* The ICAP ClientReqmodPrecache implements message pipe sink and source interfaces. It - * helps client-side to marshall the incoming/virgin HTTP message (being - * recieved from the HTTP client) to Squid's ICAP client module, using the - * MsgPipe interface. The same interface is used to get the adapted HTTP - * message back from the ICAP client. client-side is the "owner" of the - * ICAPClientReqmodPrecache. +/* + * ICAPClientReqmodPrecache implements the ICAP client-side pre-cache + * vectoring point using ICAPClientVector as a parent. + * ClientHttpRequest is the Owner of this vectoring point. */ -class HttpRequest; - class ClientRequestContext; -class ICAPClientReqmodPrecache: public MsgPipeSource, public MsgPipeSink +class ICAPClientReqmodPrecache: public ICAPClientVector { public: ICAPClientReqmodPrecache(ICAPServiceRep::Pointer); - virtual ~ICAPClientReqmodPrecache(); // synchronous calls called by ClientHttpRequest void startReqMod(ClientHttpRequest *, HttpRequest *); - void sendMoreData(StoreIOBuffer buf); - void doneSending(); - void ownerAbort(); - int potentialSpaceSize(); /* how much data can we accept? */ // pipe source methods; called by ICAP while receiving the virgin message - virtual void noteSinkNeed(MsgPipe *p); - virtual void noteSinkAbort(MsgPipe *p); + // pipe sink methods; called by ICAP while sending the adapted message virtual void noteSourceStart(MsgPipe *p); virtual void noteSourceProgress(MsgPipe *p); - virtual void noteSourceFinish(MsgPipe *p); - virtual void noteSourceAbort(MsgPipe *p); + +protected: + // used by ICAPClientVector because it does not know Owner type + virtual void tellSpaceAvailable(); + virtual void tellDoneAdapting(); + virtual void tellAbortAdapting(); + virtual void stop(Notify notify); public: - ICAPServiceRep::Pointer service; ClientHttpRequest *http; - MsgPipe::Pointer virgin; - MsgPipe::Pointer adapted; BodyReader::Pointer body_reader; private: - typedef enum { notifyNone, notifyOwner, notifyIcap } Notify; - void stop(Notify notify); - void freeVirgin(); - void freeAdapted(); - CBDATA_CLASS2(ICAPClientReqmodPrecache); - // Hooks to BodyReader so HttpStateData can get the // adapted request body static BodyReadFunc readBody; static BodyAbortFunc abortBody; static BodyKickFunc kickBody; + + CBDATA_CLASS2(ICAPClientReqmodPrecache); }; #endif /* SQUID_ICAPCLIENTSIDEHOOK_H */ diff --git a/src/ICAP/ICAPClientRespmodPrecache.cc b/src/ICAP/ICAPClientRespmodPrecache.cc index fcc6672c69..7df4d6757e 100644 --- a/src/ICAP/ICAPClientRespmodPrecache.cc +++ b/src/ICAP/ICAPClientRespmodPrecache.cc @@ -1,9 +1,6 @@ #include "squid.h" #include "http.h" -#include "MsgPipe.h" #include "MsgPipeData.h" -#include "MsgPipeSource.h" -#include "MsgPipeSink.h" #include "HttpRequest.h" #include "HttpReply.h" #include "ICAPClientRespmodPrecache.h" @@ -12,128 +9,28 @@ CBDATA_CLASS_INIT(ICAPClientRespmodPrecache); -ICAPClientRespmodPrecache::ICAPClientRespmodPrecache(ICAPServiceRep::Pointer aService): service(aService), serverState(NULL), virgin(NULL), adapted(NULL) +ICAPClientRespmodPrecache::ICAPClientRespmodPrecache(ICAPServiceRep::Pointer aService): + ICAPClientVector(aService, "ICAPClientRespmodPrecache"), serverState(NULL) { - debug(93,5)("ICAPClientRespmodPrecache constructed, this=%p\n", this); } -ICAPClientRespmodPrecache::~ICAPClientRespmodPrecache() +void ICAPClientRespmodPrecache::startRespMod(ServerStateData *aServerState, HttpRequest *request, HttpReply *reply) { - stop(notifyNone); - cbdataReferenceDone(serverState); - debug(93,5)("ICAPClientRespmodPrecache destructed, this=%p\n", this); - - if (virgin != NULL) - freeVirgin(); - - if (adapted != NULL) - freeAdapted(); - - service = NULL; -} - -void ICAPClientRespmodPrecache::startRespMod(ServerStateData *anServerState, HttpRequest *request, HttpReply *reply) -{ - serverState = cbdataReference(anServerState); - - virgin = new MsgPipe("virgin"); // this is the place to create a refcount ptr - virgin->source = this; - virgin->data = new MsgPipeData; - virgin->data->setCause(request); - virgin->data->setHeader(reply); - virgin->data->body = new MemBuf; - virgin->data->body->init(ICAP::MsgPipeBufSizeMin, ICAP::MsgPipeBufSizeMax); - - adapted = new MsgPipe("adapted"); - adapted->sink = this; -#if ICAP_ANCHOR_LOOPBACK - - adapted->data = new MsgPipeData; - adapted->data->setCause(request); // should not hurt -#else - - ICAPInitXaction(service, virgin, adapted); -#endif - - virgin->sendSourceStart(); // we may have virgin data to provide - adapted->sendSinkNeed(); // we want adapted response, eventially -} - -void ICAPClientRespmodPrecache::sendMoreData(StoreIOBuffer buf) -{ - debug(93,5)("ICAPClientRespmodPrecache::sendMoreData() called\n"); - //debugs(93,0,HERE << "appending " << buf.length << " bytes"); - //debugs(93,0,HERE << "body.contentSize = " << virgin->data->body->contentSize()); - //buf.dump(); - /* - * The caller is responsible for not giving us more data - * than will fit in body MemBuf. Caller should use - * potentialSpaceSize() to find out how much we can hold. - */ - virgin->data->body->append(buf.data, buf.length); - virgin->sendSourceProgress(); -} - -int -ICAPClientRespmodPrecache::potentialSpaceSize() -{ - if (virgin == NULL) - return 0; - - return (int) virgin->data->body->potentialSpaceSize(); -} - -// ServerStateData says we have the entire HTTP message -void ICAPClientRespmodPrecache::doneSending() -{ - debug(93,5)("ICAPClientRespmodPrecache::doneSending() called\n"); - -#if ICAP_ANCHOR_LOOPBACK - /* simple assignments are not the right way to do this */ - adapted->data->setHeader(virgin->data->header); - adapted->data->body = virgin->data->body; - noteSourceFinish(adapted); - return; -#else - - virgin->sendSourceFinish(); -#endif -} - -// ServerStateData tells us to abort -void ICAPClientRespmodPrecache::ownerAbort() -{ - debug(93,5)("ICAPClientRespmodPrecache::ownerAbort() called\n"); - stop(notifyIcap); -} - -// ICAP client needs more virgin response data -void ICAPClientRespmodPrecache::noteSinkNeed(MsgPipe *p) -{ - debug(93,5)("ICAPClientRespmodPrecache::noteSinkNeed() called\n"); - - if (virgin->data->body->potentialSpaceSize()) - serverState->icapSpaceAvailable(); -} - -// ICAP client aborting -void ICAPClientRespmodPrecache::noteSinkAbort(MsgPipe *p) -{ - debug(93,5)("ICAPClientRespmodPrecache::noteSinkAbort() called\n"); - stop(notifyOwner); + serverState = cbdataReference(aServerState); + startMod(serverState, request, reply); } // ICAP client starts sending adapted response // ICAP client has received new HTTP headers (if any) at this point void ICAPClientRespmodPrecache::noteSourceStart(MsgPipe *p) { - debugs(93,5, HERE << "ICAPClientRespmodPrecache::noteSourceStart() called"); + debugs(93,3, HERE << "ICAPClientRespmodPrecache::noteSourceStart() called"); HttpReply *reply = dynamic_cast(adapted->data->header); /* - * The ICAP reply MUST have a new HTTP reply header, or else - * it is an invalid ICAP message. Invalid ICAP messages should - * be handled prior to this point. + * The ICAP reply MUST have a new HTTP reply header, or else + * it is an invalid ICAP message. Invalid ICAP messages should + * be handled prior to this point. */ assert(reply); // check that ICAP xaction created the right object assert(reply == adapted->data->header); @@ -146,7 +43,8 @@ void ICAPClientRespmodPrecache::noteSourceStart(MsgPipe *p) ssize_t dummy; bool expect_body = reply->expectingBody(virgin->data->cause->method, dummy); - serverState->takeAdaptedHeaders(reply); + if (!serverState->takeAdaptedHeaders(reply)) // deletes us + return; if (expect_body) noteSourceProgress(p); @@ -157,70 +55,39 @@ void ICAPClientRespmodPrecache::noteSourceStart(MsgPipe *p) // ICAP client sends more data void ICAPClientRespmodPrecache::noteSourceProgress(MsgPipe *p) { - debug(93,5)("ICAPClientRespmodPrecache::noteSourceProgress() called\n"); + debug(93,3)("ICAPClientRespmodPrecache::noteSourceProgress() called\n"); //tell ServerStateData to store a fresh portion of the adapted response assert(serverState); if (p->data->body->hasContent()) { - serverState->takeAdaptedBody(p->data->body); - } -} + if (!serverState->takeAdaptedBody(p->data->body)) + return; -// ICAP client is done sending adapted response -void ICAPClientRespmodPrecache::noteSourceFinish(MsgPipe *p) -{ - debug(93,5)("ICAPClientRespmodPrecache::noteSourceFinish() called\n"); - //tell ServerStateData that we expect no more response data - serverState->doneAdapting(); - stop(notifyNone); + // HttpStateData::takeAdaptedBody does not detect when we have enough, + // so we always notify source that there more buffer space is available + if (p->data->body->hasPotentialSpace()) + adapted->sendSinkNeed(); + } } -// ICAP client is aborting -void ICAPClientRespmodPrecache::noteSourceAbort(MsgPipe *p) +void +ICAPClientRespmodPrecache::tellSpaceAvailable() { - debug(93,5)("ICAPClientRespmodPrecache::noteSourceAbort() called\n"); - stop(notifyOwner); + serverState->icapSpaceAvailable(); } -// internal cleanup -void ICAPClientRespmodPrecache::stop(Notify notify) +void +ICAPClientRespmodPrecache::tellDoneAdapting() { - if (virgin != NULL) { - if (notify == notifyIcap) - virgin->sendSourceAbort(); - else - virgin->source = NULL; - - freeVirgin(); - } - - if (adapted != NULL) { - if (notify == notifyIcap) - adapted->sendSinkAbort(); - else - adapted->sink = NULL; - - freeAdapted(); - } - - if (serverState) { - if (notify == notifyOwner) - // tell ServerStateData that we are aborting prematurely - serverState->abortAdapting(); - - cbdataReferenceDone(serverState); - - // serverState is now NULL, will not call it any more - } + serverState->finishAdapting(); // deletes us } -void ICAPClientRespmodPrecache::freeVirgin() +void +ICAPClientRespmodPrecache::tellAbortAdapting() { - virgin = NULL; // refcounted + debug(93,3)("ICAPClientReqmodPrecache::tellAbortAdapting() called\n"); + // tell ClientHttpRequest that we are aborting ICAP processing prematurely + serverState->abortAdapting(); // deletes us } -void ICAPClientRespmodPrecache::freeAdapted() -{ - adapted = NULL; // refcounted -} diff --git a/src/ICAP/ICAPClientRespmodPrecache.h b/src/ICAP/ICAPClientRespmodPrecache.h index e13324ddb0..db4e10d63b 100644 --- a/src/ICAP/ICAPClientRespmodPrecache.h +++ b/src/ICAP/ICAPClientRespmodPrecache.h @@ -1,6 +1,6 @@ /* - * $Id: ICAPClientRespmodPrecache.h,v 1.3 2006/01/25 17:41:23 wessels Exp $ + * $Id: ICAPClientRespmodPrecache.h,v 1.4 2006/10/31 23:30:58 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -31,62 +31,44 @@ * */ -#ifndef SQUID_ICAPANCHOR_H -#define SQUID_ICAPANCHOR_H +#ifndef SQUID_ICAPCLIENTRESPMODPRECACHE_H +#define SQUID_ICAPCLIENTRESPMODPRECACHE_H -#include "MsgPipe.h" -#include "MsgPipeSource.h" -#include "MsgPipeSink.h" -#include "ICAPServiceRep.h" +#include "ICAPClientVector.h" -/* The ICAP Anchor implements message pipe sink and source interfaces. It - * helps ServerStateData to marshall the incoming/virgin HTTP message (being - * recieved from the HTTP server) to Squid's ICAP client module, using the - * MsgPipe interface. The same interface is used to get the adapted HTTP - * message back from the ICAP client. ServerStateData is the "owner" of the - * ICAPClientRespmodPrecache. +/* + * ICAPClientRespmodPrecache implements the server-side pre-cache ICAP + * vectoring point using ICAPClientVector as a parent. + * ServerStateData is the Owner of this vectoring point. */ -class HttpRequest; - -class HttpReply; +class ServerStateData; -class ICAPClientRespmodPrecache: public MsgPipeSource, public MsgPipeSink +class ICAPClientRespmodPrecache: public ICAPClientVector { public: ICAPClientRespmodPrecache(ICAPServiceRep::Pointer); - virtual ~ICAPClientRespmodPrecache(); // synchronous calls called by ServerStateData void startRespMod(ServerStateData *anServerState, HttpRequest *request, HttpReply *reply); - void sendMoreData(StoreIOBuffer buf); - void doneSending(); - void ownerAbort(); - int potentialSpaceSize(); /* how much data can we accept? */ // pipe source methods; called by ICAP while receiving the virgin message - virtual void noteSinkNeed(MsgPipe *p); - virtual void noteSinkAbort(MsgPipe *p); // pipe sink methods; called by ICAP while sending the adapted message virtual void noteSourceStart(MsgPipe *p); virtual void noteSourceProgress(MsgPipe *p); - virtual void noteSourceFinish(MsgPipe *p); - virtual void noteSourceAbort(MsgPipe *p); + +protected: + virtual void tellSpaceAvailable(); + virtual void tellDoneAdapting(); // deletes us + virtual void tellAbortAdapting(); // deletes us public: - ICAPServiceRep::Pointer service; ServerStateData *serverState; - MsgPipe::Pointer virgin; - MsgPipe::Pointer adapted; private: - typedef enum { notifyNone, notifyOwner, notifyIcap } Notify; - void stop(Notify notify); - void freeVirgin(); - void freeAdapted(); CBDATA_CLASS2(ICAPClientRespmodPrecache); }; -#endif /* SQUID_ICAPANCHOR_H */ +#endif /* SQUID_ICAPCLIENTRESPMODPRECACHE_H */ diff --git a/src/ICAP/ICAPClientVector.cc b/src/ICAP/ICAPClientVector.cc new file mode 100644 index 0000000000..16e34bfc5e --- /dev/null +++ b/src/ICAP/ICAPClientVector.cc @@ -0,0 +1,172 @@ +#include "squid.h" +#include "MsgPipe.h" +#include "MsgPipeData.h" +#include "MsgPipeSource.h" +#include "MsgPipeSink.h" +#include "HttpRequest.h" +#include "ICAPClientVector.h" +#include "ICAPClient.h" + +ICAPClientVector::ICAPClientVector(ICAPServiceRep::Pointer aService, const char *aPoint): + theOwner(0), vPoint(aPoint), + service(aService), virgin(NULL), adapted(NULL) +{ + debug(93,3)("%s constructed, this=%p\n", vPoint, this); +} + +ICAPClientVector::~ICAPClientVector() +{ + stop(notifyNone); + debug(93,3)("%s destructed, this=%p\n", vPoint, this); +} + +void ICAPClientVector::startMod(void *anOwner, HttpRequest *cause, HttpMsg *header) +{ + debug(93,5)("%s starting, this=%p\n", vPoint, this); + + theOwner = anOwner; + + virgin = new MsgPipe("virgin"); // this is the place to create a refcount ptr + virgin->source = this; + virgin->data = new MsgPipeData; + virgin->data->setCause(cause); + virgin->data->setHeader(header); + virgin->data->body = new MemBuf; + virgin->data->body->init(ICAP::MsgPipeBufSizeMin, ICAP::MsgPipeBufSizeMax); + + adapted = new MsgPipe("adapted"); + adapted->sink = this; + +#if ICAP_ANCHOR_LOOPBACK + adapted->data = new MsgPipeData; + adapted->data->setCause(request); // should not hurt +#else + ICAPInitXaction(service, virgin, adapted); +#endif + + virgin->sendSourceStart(); // we may have virgin data to provide + adapted->sendSinkNeed(); // we want adapted response, eventially +} + +void ICAPClientVector::sendMoreData(StoreIOBuffer buf) +{ + debug(93,7)("%s::sendMoreData(%p)\n", vPoint, this); + //debugs(93,0,HERE << "appending " << buf.length << " bytes"); + //debugs(93,0,HERE << "body.contentSize = " << virgin->data->body->contentSize()); + //buf.dump(); + /* + * The caller is responsible for not giving us more data + * than will fit in body MemBuf. Caller should use + * potentialSpaceSize() to find out how much we can hold. + */ + virgin->data->body->append(buf.data, buf.length); + virgin->sendSourceProgress(); +} + +int +ICAPClientVector::potentialSpaceSize() +{ + if (virgin == NULL) + return 0; + + return (int) virgin->data->body->potentialSpaceSize(); +} + +// Owner says we have the entire HTTP message +void ICAPClientVector::doneSending() +{ + debug(93,3)("%s::doneSending(%p)\n", vPoint, this); + +#if ICAP_ANCHOR_LOOPBACK + /* simple assignments are not the right way to do this */ + adapted->data->setHeader(virgin->data->header); + adapted->data->body = virgin->data->body; + noteSourceFinish(adapted); + // checkDoneAdapting() does not support loopback mode + return; +#else + virgin->sendSourceFinish(); + checkDoneAdapting(); // may call the owner back, unfortunately +#endif +} + +// Owner tells us to abort +void ICAPClientVector::ownerAbort() +{ + debug(93,3)("%s::ownerAbort(%p)\n", vPoint, this); + stop(notifyIcap); +} + +// ICAP client needs more virgin response data +void ICAPClientVector::noteSinkNeed(MsgPipe *p) +{ + debug(93,3)("%s::noteSinkNeed(%p)\n", vPoint, this); + + if (virgin->data->body->potentialSpaceSize()) + tellSpaceAvailable(); +} + +// ICAP client aborting +void ICAPClientVector::noteSinkAbort(MsgPipe *p) +{ + debug(93,3)("%s::noteSinkAbort(%p)\n", vPoint, this); + stop(notifyOwner); // deletes us +} + +// ICAP client is done sending adapted response +void ICAPClientVector::noteSourceFinish(MsgPipe *p) +{ + debug(93,3)("%s::noteSourceFinish(%p)\n", vPoint, this); + checkDoneAdapting(); // may delete us +} + +void ICAPClientVector::checkDoneAdapting() { + debug(93,5)("%s::checkDoneAdapting(%p): %d & %d\n", vPoint, this, + (int)!virgin->source, (int)!adapted->source); + // done if we are not sending and are not receiving + if (!virgin->source && !adapted->source) + tellDoneAdapting(); // deletes us +} + +// ICAP client is aborting +void ICAPClientVector::noteSourceAbort(MsgPipe *p) +{ + debug(93,3)("%s::noteSourceAbort(%p)\n", vPoint, this); + stop(notifyOwner); // deletes us +} + +void ICAPClientVector::stop(Notify notify) +{ + debug(93,3)("%s::stop(%p, %d)\n", vPoint, this, (int)notify); + clean(notify, true); +} + +void ICAPClientVector::clean(Notify notify, bool cleanAdapted) +{ + if (virgin != NULL) { + if (notify == notifyIcap) + virgin->sendSourceAbort(); + else + virgin->source = NULL; + virgin = NULL; // refcounted + } + + if (cleanAdapted && adapted != NULL) { + if (notify == notifyIcap) + adapted->sendSinkAbort(); + else + adapted->sink = NULL; + adapted = NULL; // refcounted + } + + service = NULL; + + if (theOwner) { + if (notify == notifyOwner) + tellAbortAdapting(); // deletes us + else + cbdataReferenceDone(theOwner); + } + + // not safe to do anything here because we may have been deleted. +} diff --git a/src/ICAP/ICAPClientVector.h b/src/ICAP/ICAPClientVector.h new file mode 100644 index 0000000000..462a53e406 --- /dev/null +++ b/src/ICAP/ICAPClientVector.h @@ -0,0 +1,101 @@ + +/* + * $Id: ICAPClientVector.h,v 1.1 2006/10/31 23:30:58 wessels 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_ICAPVECTOR_H +#define SQUID_ICAPVECTOR_H + +#include "MsgPipe.h" +#include "MsgPipeSource.h" +#include "MsgPipeSink.h" +#include "ICAPServiceRep.h" + +/* + * The ICAP Vector helps its Owner to talk to the ICAP transaction, which + * implements asynchronous communication with the ICAP server. The Owner + * is either the HTTP client side (ClientHttpRequest) or the HTTP server + * side (ServerStateData). The Vector marshals the incoming/virgin HTTP + * message to the ICAP transaction, via the MsgPipe interface. The same + * interface is used to get the adapted HTTP message back. + * + * ICAPClientReqmodPrecache and ICAPClientRespmodPrecache classes use + * ICAPVector as a base and cover specifics of their vectoring point. + */ + +class ICAPClientVector: public MsgPipeSource, public MsgPipeSink +{ + +public: + ICAPClientVector(ICAPServiceRep::Pointer, const char *aPoint); + virtual ~ICAPClientVector(); + + // synchronous calls called by Owner + void sendMoreData(StoreIOBuffer buf); + void doneSending(); + void ownerAbort(); + int potentialSpaceSize(); /* how much data can we accept? */ + + // pipe source methods; called by ICAP while receiving the virgin message + virtual void noteSinkNeed(MsgPipe *p); + virtual void noteSinkAbort(MsgPipe *p); + + // pipe sink methods; called by ICAP while sending the adapted message + virtual void noteSourceStart(MsgPipe *p) = 0; + virtual void noteSourceProgress(MsgPipe *p) = 0; + virtual void noteSourceFinish(MsgPipe *p); + virtual void noteSourceAbort(MsgPipe *p); + +protected: + typedef enum { notifyNone, notifyOwner, notifyIcap } Notify; + + // implemented by kids because we do not have a common Owner parent + virtual void tellSpaceAvailable() = 0; + virtual void tellDoneAdapting() = 0; // may delete us + virtual void tellAbortAdapting() = 0; // may delete us + virtual void stop(Notify notify); // may delete us + + void startMod(void *anOwner, HttpRequest *cause, HttpMsg *header); + void clean(Notify notify, bool cleanAdapted = true); + +private: + void checkDoneAdapting(); + +public: + void *theOwner; + const char *vPoint; // unmanaged vectoring point name for debugging + + ICAPServiceRep::Pointer service; + MsgPipe::Pointer virgin; + MsgPipe::Pointer adapted; +}; + +#endif /* SQUID_ICAPVECTOR_H */ diff --git a/src/ICAP/ICAPConfig.cc b/src/ICAP/ICAPConfig.cc index 06ec2ba00d..d0480c0448 100644 --- a/src/ICAP/ICAPConfig.cc +++ b/src/ICAP/ICAPConfig.cc @@ -1,6 +1,6 @@ /* - * $Id: ICAPConfig.cc,v 1.11 2006/05/11 23:53:13 wessels Exp $ + * $Id: ICAPConfig.cc,v 1.12 2006/10/31 23:30:58 wessels Exp $ * * SQUID Web Proxy Cache http://www.squid-cache.org/ * ---------------------------------------------------------- @@ -152,31 +152,17 @@ ICAPAccessCheck::check() for (ci = TheICAPConfig.classes.begin(); ci != TheICAPConfig.classes.end(); ++ci) { - ICAPClass *theClass = *ci; - - Vector::iterator si; - - for (si = theClass->services.begin(); si != theClass->services.end(); ++si) { - ICAPServiceRep *theService = si->getRaw(); - - if (method != theService->method) - continue; - - if (point != theService->point) - continue; - - debug(93,3)("ICAPAccessCheck::check: class '%s' has candidate service '%s'\n", theClass->key.buf(), theService->key.buf()); - - candidateClasses += theClass->key; - - /* - * Break here because we only need one matching service - * to justify ACL-checking a class. We might use other - * services belonging to the class if the first service - * is unavailable, etc. - */ - break; - + /* + * We only find the first matching service because we only need + * one matching service to justify ACL-checking a class. We might + * use other services belonging to the class if the first service + * turns out to be unusable for some reason. + */ + ICAPClass *c = *ci; + ICAPServiceRep::Pointer service = findBestService(c, false); + if (service.getRaw()) { + debug(93,3)("ICAPAccessCheck::check: class '%s' has candidate service '%s'\n", c->key.buf(), service->key.buf()); + candidateClasses += c->key; } } @@ -277,23 +263,72 @@ ICAPAccessCheck::do_callback() return; } - Vector::iterator i; + const ICAPServiceRep::Pointer service = findBestService(theClass, true); + if (!service) + callback(NULL, validated_cbdata); + else + callback(service, validated_cbdata); +} - for (i = theClass->services.begin(); i != theClass->services.end(); ++i) { - ICAPServiceRep *theService = i->getRaw(); +ICAPServiceRep::Pointer +ICAPAccessCheck::findBestService(ICAPClass *c, bool preferUp) { - if (method != theService->method) + const char *what = preferUp ? "up " : ""; + debugs(93,7,HERE << "looking for the first matching " << + what << "service in class " << c->key); + + ICAPServiceRep::Pointer secondBest; + + Vector::iterator si; + for (si = c->services.begin(); si != c->services.end(); ++si) { + ICAPServiceRep::Pointer service = *si; + + if (method != service->method) continue; - if (point != theService->point) + if (point != service->point) continue; - callback(*i, validated_cbdata); + // sending a message to a broken service is likely to cause errors + if (service->bypass && service->broken()) + continue; - return; + if (service->up()) { + // sending a message to a service that does not want it is useless + // note that we cannot check wantsUrl for service that is not "up" + // note that even essential services are skipped on unwanted URLs! + if (!service->wantsUrl(req->urlpath)) + continue; + } else { + if (!secondBest) + secondBest = service; + if (preferUp) { + // the caller asked for an "up" service and we can bypass this one + if (service->bypass) + continue; + debugs(93,5,HERE << "cannot skip an essential down service"); + what = "down-but-essential "; + } + } + + debugs(93,5,HERE << "found first matching " << + what << "service in class " << c->key << + ": " << service->key); + + return service; + } + + if (secondBest.getRaw()) { + what = "down "; + debugs(93,5,HERE << "found first matching " << + what << "service in class " << c->key << + ": " << secondBest->key); + return secondBest; } - callback(NULL, callback_data); + debugs(93,5,HERE << "found no matching " << + what << "services in class " << c->key); + return ICAPServiceRep::Pointer(); } // ================================================================================ // diff --git a/src/ICAP/ICAPConfig.h b/src/ICAP/ICAPConfig.h index 3fb81d7009..34a11d4b8f 100644 --- a/src/ICAP/ICAPConfig.h +++ b/src/ICAP/ICAPConfig.h @@ -1,6 +1,6 @@ /* - * $Id: ICAPConfig.h,v 1.9 2006/08/07 02:28:24 robertc Exp $ + * $Id: ICAPConfig.h,v 1.10 2006/10/31 23:30:58 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -78,6 +78,7 @@ private: Vector candidateClasses; String matchedClass; void do_callback(); + ICAPServiceRep::Pointer findBestService(ICAPClass *c, bool preferUp); public: void check(); diff --git a/src/ICAP/ICAPModXact.cc b/src/ICAP/ICAPModXact.cc index 565ce9ff24..253d60da25 100644 --- a/src/ICAP/ICAPModXact.cc +++ b/src/ICAP/ICAPModXact.cc @@ -124,15 +124,16 @@ void ICAPModXact::noteServiceReady() Must(state.serviceWaiting); state.serviceWaiting = false; - startWriting(); // will throw if service is not up + + Must(service().up()); + + startWriting(); ICAPXaction_Exit(); } void ICAPModXact::startWriting() { - Must(service().up()); - state.writing = State::writingConnect; openConnection(); // put nothing here as openConnection calls commConnectStart @@ -179,7 +180,7 @@ void ICAPModXact::handleCommWroteHeaders() virginWriteClaim.protectAll(); writeMore(); } else { - stopWriting(); + stopWriting(true); } } @@ -199,7 +200,11 @@ void ICAPModXact::writeMore() case State::writingPaused: // waiting for the ICAP server response - case State::writingDone: // nothing more to write + case State::writingReallyDone: // nothing more to write + return; + + case State::writingAlmostDone: // was waiting for the last write + stopWriting(false); return; case State::writingPreview: @@ -230,7 +235,7 @@ void ICAPModXact::writePriviewBody() debugs(93, 7, "ICAPModXact wrote entire Preview body " << status()); if (preview.ieof()) - stopWriting(); + stopWriting(true); else state.writing = State::writingPaused; } @@ -245,15 +250,15 @@ void ICAPModXact::writePrimeBody() const size_t size = body->contentSize(); writeSomeBody("prime virgin body", size); - if (state.doneReceiving) { - debugs(98, 5, HERE << "state.doneReceiving is set"); - stopWriting(); + if (state.doneReceiving && claimSize(virginWriteClaim) <= 0) { + debugs(93, 5, HERE << "state.doneReceiving is set and wrote all"); + stopWriting(true); } } void ICAPModXact::writeSomeBody(const char *label, size_t size) { - Must(!writer && !state.doneWriting()); + Must(!writer && state.writing < state.writingAlmostDone); debugs(93, 8, HERE << "will write up to " << size << " bytes of " << label); @@ -261,14 +266,14 @@ void ICAPModXact::writeSomeBody(const char *label, size_t size) writeBuf.init(); // note: we assume that last-chunk will fit - const size_t writeableSize = claimSize(virginWriteClaim); - const size_t chunkSize = XMIN(writeableSize, size); + const size_t writableSize = claimSize(virginWriteClaim); + const size_t chunkSize = XMIN(writableSize, size); if (chunkSize) { debugs(93, 7, HERE << "will write " << chunkSize << "-byte chunk of " << label); } else { - debugs(93, 7, "ICAPModXact has no writeable " << label << " content"); + debugs(93, 7, "ICAPModXact has no writable " << label << " content"); } moveRequestChunk(writeBuf, chunkSize); @@ -295,34 +300,36 @@ void ICAPModXact::writeSomeBody(const char *label, size_t size) void ICAPModXact::moveRequestChunk(MemBuf &buf, size_t chunkSize) { if (chunkSize > 0) { - openChunk(buf, chunkSize); + openChunk(buf, chunkSize, false); buf.append(claimContent(virginWriteClaim), chunkSize); - closeChunk(buf, false); + closeChunk(buf); virginWriteClaim.release(chunkSize); virginConsume(); } - if (state.writing == State::writingPreview) - preview.wrote(chunkSize, state.doneReceiving); // even if wrote nothing + if (state.writing == State::writingPreview) { + // even if we are doneReceiving, we may not have written everything + const bool wroteEof = state.doneReceiving && + claimSize(virginWriteClaim) <= 0; + preview.wrote(chunkSize, wroteEof); // even if wrote nothing + } } void ICAPModXact::addLastRequestChunk(MemBuf &buf) { - openChunk(buf, 0); - closeChunk(buf, state.writing == State::writingPreview && preview.ieof()); + const bool ieof = state.writing == State::writingPreview && preview.ieof(); + openChunk(buf, 0, ieof); + closeChunk(buf); } -void ICAPModXact::openChunk(MemBuf &buf, size_t chunkSize) +void ICAPModXact::openChunk(MemBuf &buf, size_t chunkSize, bool ieof) { - buf.Printf("%x\r\n", (int) chunkSize); + buf.Printf((ieof ? "%x; ieof\r\n" : "%x\r\n"), (int) chunkSize); } -void ICAPModXact::closeChunk(MemBuf &buf, bool ieof) +void ICAPModXact::closeChunk(MemBuf &buf) { - if (ieof) - buf.append("; ieof", 6); - buf.append(ICAP::crlf, 2); // chunk-terminating CRLF } @@ -374,22 +381,36 @@ void ICAPModXact::handleCommWroteBody() writeMore(); } -void ICAPModXact::stopWriting() +// Called when we do not expect to call comm_write anymore. +// We may have a pending write though. +// If stopping nicely, we will just wait for that pending write, if any. +void ICAPModXact::stopWriting(bool nicely) { - if (state.writing == State::writingDone) + if (state.writing == State::writingReallyDone) return; - debugs(93, 7, HERE << "will no longer write " << status()); + if (writer) { + if (nicely) { + debugs(93, 7, HERE << "will wait for the last write " << status()); + state.writing = State::writingAlmostDone; // may already be set + return; + } + debugs(93, 2, HERE << "will NOT wait for the last write " << status()); - state.writing = State::writingDone; + // Comm does not have an interface to clear the writer callback nicely, + // but without clearing the writer we cannot recycle the connection. + // We prevent connection reuse and hope that we can handle a callback + // call at any time. Somebody should either fix this code or add + // comm_remove_write_handler() to comm API. + reuseConnection = false; + } + + debugs(93, 7, HERE << "will no longer write " << status()); + state.writing = State::writingReallyDone; virginWriteClaim.disable(); virginConsume(); - - // Comm does not have an interface to clear the writer, but - // writeMore() will not write if our write callback is called - // when state.writing == State::writingDone; } void ICAPModXact::stopBackup() @@ -426,20 +447,20 @@ void ICAPModXact::startReading() void ICAPModXact::readMore() { if (reader || doneReading()) { - debugs(32,3,HERE << "returning from readMore because reader or doneReading()"); + debugs(93,3,HERE << "returning from readMore because reader or doneReading()"); return; } // do not fill readBuf if we have no space to store the result if (!adapted->data->body->hasPotentialSpace()) { - debugs(93,1,HERE << "Not reading because ICAP reply buffer is full"); + debugs(93,3,HERE << "not reading because ICAP reply buffer is full"); return; } if (readBuf.hasSpace()) scheduleRead(); else - debugs(93,1,HERE << "nothing to do because !readBuf.hasSpace()"); + debugs(93,3,HERE << "nothing to do because !readBuf.hasSpace()"); } // comm module read a portion of the ICAP response for us @@ -471,7 +492,7 @@ void ICAPModXact::echoMore() adapted->sendSourceProgress(); } - if (!from.hasContent() && state.doneReceiving) { + if (state.doneReceiving && claimSize(virginSendClaim) <= 0) { debugs(93, 5, "ICAPModXact echoed all " << status()); stopSending(true); } else { @@ -620,7 +641,7 @@ void ICAPModXact::parseIcapHead() // handle100Continue() manages state.writing on its own. // Non-100 status means the server needs no postPreview data from us. if (state.writing == State::writingPaused) - stopWriting(); + stopWriting(true); // TODO: Consider applying a Squid 2.5 patch to recognize 201 responses } @@ -653,7 +674,8 @@ void ICAPModXact::handle100Continue() if (virginSendClaim.limited()) // preview only stopBackup(); - state.parsing = State::psHttpHeader; // eventually + state.parsing = State::psIcapHeader; // eventually + icapReply->reset(); state.writing = State::writingPrime; @@ -688,16 +710,16 @@ void ICAPModXact::handle204NoContent() httpBuf.init(); packHead(httpBuf, oldHead); - // allocate the adapted message + // allocate the adapted message and copy metainfo Must(!adapted->data->header); HttpMsg *newHead = NULL; - - if (dynamic_cast(oldHead)) - newHead = new HttpRequest; - else - if (dynamic_cast(oldHead)) - newHead = new HttpReply; - + if (const HttpRequest *oldR = dynamic_cast(oldHead)) { + HttpRequest *newR = new HttpRequest; + newR->client_addr = oldR->client_addr; + newHead = newR; + } else + if (dynamic_cast(oldHead)) + newHead = new HttpReply; Must(newHead); adapted->data->setHeader(newHead); @@ -730,18 +752,16 @@ void ICAPModXact::parseHttpHead() maybeAllocateHttpMsg(); if (!parseHead(adapted->data->header)) - return; // need more header data + return; // need more header data } state.parsing = State::psBody; } -/* - * Common routine used to parse both HTTP and ICAP headers - */ +// parses both HTTP and ICAP headers bool ICAPModXact::parseHead(HttpMsg *head) { - assert(head); + Must(head); debugs(93, 5, HERE << "have " << readBuf.contentSize() << " head bytes to parse" << "; state: " << state.parsing); @@ -749,7 +769,7 @@ bool ICAPModXact::parseHead(HttpMsg *head) const bool parsed = head->parse(&readBuf, commEof, &error); Must(parsed || !error); // success or need more data - if (!parsed) { // need more data + if (!parsed) { // need more data debugs(93, 5, HERE << "parse failed, need more data, return false"); head->reset(); return false; @@ -794,10 +814,10 @@ bool ICAPModXact::parsePresentBody() if (parsed) return true; - debugs(32,3,HERE << this << " needsMoreData = " << bodyParser->needsMoreData()); + debugs(93,3,HERE << this << " needsMoreData = " << bodyParser->needsMoreData()); if (bodyParser->needsMoreData()) { - debugs(32,3,HERE << this); + debugs(93,3,HERE << this); Must(mayReadMore()); readMore(); } @@ -896,10 +916,10 @@ void ICAPModXact::noteSinkAbort(MsgPipe *p) // internal cleanup void ICAPModXact::doStop() { - debugs(98, 5, HERE << "doStop() called"); + debugs(93, 5, HERE << "doStop() called"); ICAPXaction::doStop(); - stopWriting(); + stopWriting(false); stopBackup(); if (icapReply) { @@ -950,10 +970,21 @@ void ICAPModXact::makeRequestHeaders(MemBuf &buf) // build HTTP request header, if any ICAP::Method m = s.method; - if (ICAP::methodRespmod == m && virgin->data->cause) - encapsulateHead(buf, "req-hdr", httpBuf, virgin->data->cause); - else if (ICAP::methodReqmod == m) - encapsulateHead(buf, "req-hdr", httpBuf, virgin->data->header); + const HttpRequest *request = virgin->data->cause ? + virgin->data->cause : + dynamic_cast(virgin->data->header); + + // to simplify, we could we assume that request is always available + + String urlPath; + if (request) { + urlPath = request->urlpath; + if (ICAP::methodRespmod == m) + encapsulateHead(buf, "req-hdr", httpBuf, request); + else + if (ICAP::methodReqmod == m) + encapsulateHead(buf, "req-hdr", httpBuf, virgin->data->header); + } if (ICAP::methodRespmod == m) if (const MsgPipeData::Header *prime = virgin->data->header) @@ -968,7 +999,7 @@ void ICAPModXact::makeRequestHeaders(MemBuf &buf) buf.append(ICAP::crlf, 2); // terminate Encapsulated line - if (shouldPreview()) { + if (shouldPreview(urlPath)) { buf.Printf("Preview: %d\r\n", (int)preview.ad()); virginSendClaim.protectUpTo(preview.ad()); } @@ -979,15 +1010,12 @@ void ICAPModXact::makeRequestHeaders(MemBuf &buf) virginSendClaim.protectAll(); } - const HttpRequest *request = virgin->data->cause ? - virgin->data->cause : - dynamic_cast(virgin->data->header); - - if (TheICAPConfig.send_client_ip) - if (request->client_addr.s_addr != any_addr.s_addr) + if (TheICAPConfig.send_client_ip && request) + if (request->client_addr.s_addr != any_addr.s_addr && + request->client_addr.s_addr != no_addr.s_addr) buf.Printf("X-Client-IP: %s\r\n", inet_ntoa(request->client_addr)); - if (TheICAPConfig.send_client_username) + if (TheICAPConfig.send_client_username && request) if (request->auth_user_request) if (request->auth_user_request->username()) buf.Printf("X-Client-Username: %s\r\n", request->auth_user_request->username()); @@ -1020,7 +1048,7 @@ void ICAPModXact::packHead(MemBuf &httpBuf, const HttpMsg *head) } // decides whether to offer a preview and calculates its size -bool ICAPModXact::shouldPreview() +bool ICAPModXact::shouldPreview(const String &urlPath) { size_t wantedSize; @@ -1029,8 +1057,8 @@ bool ICAPModXact::shouldPreview() return false; } - if (!service().wantsPreview(wantedSize)) { - debugs(93, 5, "ICAPModXact should not offer preview"); + if (!service().wantsPreview(urlPath, wantedSize)) { + debugs(93, 5, "ICAPModXact should not offer preview for " << urlPath); return false; } @@ -1071,12 +1099,19 @@ bool ICAPModXact::shouldAllow204() return virginBody.size() < TheBackupLimit; } -// returns a temporary string depicting transaction status, for debugging void ICAPModXact::fillPendingStatus(MemBuf &buf) const { + ICAPXaction::fillPendingStatus(buf); + if (state.serviceWaiting) buf.append("U", 1); + if (!state.doneReceiving) + buf.append("R", 1); + + if (!doneReading()) + buf.append("r", 1); + if (!state.doneWriting() && state.writing != State::writingInit) buf.Printf("w(%d)", state.writing); @@ -1097,6 +1132,8 @@ void ICAPModXact::fillPendingStatus(MemBuf &buf) const void ICAPModXact::fillDoneStatus(MemBuf &buf) const { + ICAPXaction::fillDoneStatus(buf); + if (state.doneReceiving) buf.append("R", 1); @@ -1275,15 +1312,15 @@ size_t ICAPPreview::debt() const return done() ? 0 : (theAd - theWritten); } -void ICAPPreview::wrote(size_t size, bool sawEof) +void ICAPPreview::wrote(size_t size, bool wroteEof) { Must(enabled()); theWritten += size; if (theWritten >= theAd) - theState = stDone; // sawEof is irrelevant + theState = stDone; // wroteEof is irrelevant else - if (sawEof) + if (wroteEof) theState = stIeof; } diff --git a/src/ICAP/ICAPModXact.h b/src/ICAP/ICAPModXact.h index 6873883ad3..35867fd475 100644 --- a/src/ICAP/ICAPModXact.h +++ b/src/ICAP/ICAPModXact.h @@ -1,6 +1,6 @@ /* - * $Id: ICAPModXact.h,v 1.5 2006/01/09 20:38:44 wessels Exp $ + * $Id: ICAPModXact.h,v 1.6 2006/10/31 23:30:58 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -110,7 +110,7 @@ public: bool done() const; // wrote everything bool ieof() const; // premature EOF - void wrote(size_t size, bool sawEof); + void wrote(size_t size, bool wroteEof); private: size_t theWritten; @@ -167,17 +167,18 @@ private: void startReading(); void readMore(); virtual bool doneReading() const { return commEof || state.doneParsing(); } + virtual bool doneWriting() const { return state.doneWriting(); } size_t claimSize(const MemBufClaim &claim) const; const char *claimContent(const MemBufClaim &claim) const; void makeRequestHeaders(MemBuf &buf); void moveRequestChunk(MemBuf &buf, size_t chunkSize); void addLastRequestChunk(MemBuf &buf); - void openChunk(MemBuf &buf, size_t chunkSize); - void closeChunk(MemBuf &buf, bool ieof); + void openChunk(MemBuf &buf, size_t chunkSize, bool ieof); + void closeChunk(MemBuf &buf); void virginConsume(); - bool shouldPreview(); + bool shouldPreview(const String &urlPath); bool shouldAllow204(); void prepBackup(size_t expectedSize); void backup(const MemBuf &buf); @@ -206,7 +207,7 @@ private: virtual void doStop(); void stopReceiving(); void stopSending(bool nicely); - void stopWriting(); + void stopWriting(bool nicely); void stopParsing(); void stopBackup(); @@ -248,7 +249,7 @@ private: 1; // expect no new virgin info (from the virgin pipe) // will not write anything [else] to the ICAP server connection - bool doneWriting() const { return writing == writingDone; } + bool doneWriting() const { return writing == writingReallyDone; } // parsed entire ICAP response from the ICAP server bool doneParsing() const { return parsing == psDone; } @@ -264,7 +265,9 @@ private: // measures ICAP request writing progress enum Writing { writingInit, writingConnect, writingHeaders, - writingPreview, writingPaused, writingPrime, writingDone } writing; + writingPreview, writingPaused, writingPrime, + writingAlmostDone, // waiting for the last write() call to finish + writingReallyDone } writing; enum Sending { sendingUndecided, sendingVirgin, sendingAdapted, sendingDone } sending; diff --git a/src/ICAP/ICAPOptXact.cc b/src/ICAP/ICAPOptXact.cc index a426c77bc9..911d3c840b 100644 --- a/src/ICAP/ICAPOptXact.cc +++ b/src/ICAP/ICAPOptXact.cc @@ -16,17 +16,16 @@ ICAPOptXact::ICAPOptXact(): ICAPXaction("ICAPOptXact"), options(NULL), cb(NULL), cbData(NULL) { - debug(93,9)("ICAPOptXact constructed, this=%p\n", this); } ICAPOptXact::~ICAPOptXact() { Must(!options); // the caller must set to NULL - debug(93,9)("ICAPOptXact destructed, this=%p\n", this); } void ICAPOptXact::start(ICAPServiceRep::Pointer &aService, Callback *aCb, void *aCbData) { + ICAPXaction_Enter(start); service(aService); Must(!cb && aCb && aCbData); @@ -34,6 +33,8 @@ void ICAPOptXact::start(ICAPServiceRep::Pointer &aService, Callback *aCb, void * cbData = cbdataReference(aCbData); openConnection(); + + ICAPXaction_Exit(); } void ICAPOptXact::handleCommConnected() @@ -49,6 +50,12 @@ void ICAPOptXact::handleCommConnected() scheduleWrite(requestBuf); } +bool ICAPOptXact::doneAll() const +{ + return options && ICAPXaction::doneAll(); +} + + void ICAPOptXact::doStop() { ICAPXaction::doStop(); @@ -63,7 +70,7 @@ void ICAPOptXact::doStop() } } - // get rid of options if we did call the callback + // get rid of options if we did not call the callback delete options; options = NULL; @@ -94,6 +101,10 @@ void ICAPOptXact::handleCommRead(size_t) bool ICAPOptXact::parseResponse() { + debugs(93, 5, HERE << "have " << readBuf.contentSize() << " bytes to parse" << + status()); + debugs(93, 5, HERE << "\n" << readBuf.content()); + HttpReply *r = new HttpReply; r->protoPrefix = "ICAP/"; // TODO: make an IcapReply class? diff --git a/src/ICAP/ICAPOptXact.h b/src/ICAP/ICAPOptXact.h index 48ad1aa94a..8fce3de9a6 100644 --- a/src/ICAP/ICAPOptXact.h +++ b/src/ICAP/ICAPOptXact.h @@ -1,5 +1,5 @@ /* - * $Id: ICAPOptXact.h,v 1.3 2005/12/22 22:26:31 wessels Exp $ + * $Id: ICAPOptXact.h,v 1.4 2006/10/31 23:30:58 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -59,6 +59,7 @@ protected: virtual void handleCommConnected(); virtual void handleCommWrote(size_t size); virtual void handleCommRead(size_t size); + virtual bool doneAll() const; void makeRequest(MemBuf &buf); bool parseResponse(); diff --git a/src/ICAP/ICAPOptions.cc b/src/ICAP/ICAPOptions.cc index 0ffae1b28c..41cffef587 100644 --- a/src/ICAP/ICAPOptions.cc +++ b/src/ICAP/ICAPOptions.cc @@ -1,4 +1,5 @@ #include "squid.h" +#include "wordlist.h" #include "HttpReply.h" #include "ICAPOptions.h" #include "TextException.h" @@ -9,104 +10,41 @@ extern ICAPConfig TheICAPConfig; ICAPOptions::ICAPOptions(): error("unconfigured"), max_connections(-1), allow204(false), - preview(-1), theTTL(-1), transfer_ext(NULL) + preview(-1), theTTL(-1) { - transfers.preview = transfers.ignore = transfers.complete = NULL; - transfers.other = TRANSFER_NONE; -}; + theTransfers.preview.name = "Transfer-Preview"; + theTransfers.preview.kind = xferPreview; + theTransfers.ignore.name = "Transfer-Ignore"; + theTransfers.ignore.kind = xferIgnore; + theTransfers.complete.name = "Transfer-Complete"; + theTransfers.complete.kind = xferComplete; + + // Section 4.10.2 of RFC 3507 says that default is no Preview + // TODO: provide a squid.conf option to overwrite the default + theTransfers.byDefault = &theTransfers.complete; +} ICAPOptions::~ICAPOptions() { - delete transfers.preview; - delete transfers.ignore; - delete transfers.complete; - delete transfer_ext; -}; - -ICAPOptions::transfer_type ICAPOptions::getTransferExt(const char *s) -{ - - if (transfer_ext) { - List *data = transfer_ext; - - while (data) { - if (*(data->element.ext) == *s) { - return data->element.type; - } - - data = data->next; - } - } - - return TRANSFER_NONE; } -void ICAPOptions::insertTransferExt(const char *t, transfer_type t_type) +// future optimization note: this method is called by ICAP ACL code at least +// twice for each HTTP message to see if the message should be ignored. For any +// non-ignored HTTP message, ICAP calls to check whether a preview is needed. +ICAPOptions::TransferKind ICAPOptions::transferKind(const String &urlPath) const { - List **Tail; - TransferPair t_ext; - - if (t == "*") { - transfers.other = t_type; - return; - } + if (theTransfers.preview.matches(urlPath)) + return xferPreview; - for (Tail = &transfer_ext; *Tail; Tail = &((*Tail)->next)) { - if (*(*Tail)->element.ext == *t) { - (*Tail)->element.type = t_type; - return; - } - } + if (theTransfers.complete.matches(urlPath)) + return xferComplete; - t_ext.ext = xstrdup(t); - t_ext.type = t_type; - List *q = new List(t_ext); - *(Tail) = q; + if (theTransfers.ignore.matches(urlPath)) + return xferIgnore; -}; - -void ICAPOptions::cfgTransferListHeader(const HttpHeader *h, const char *fname, transfer_type t_type) -{ - const String s = h->getByName(fname); - - if (!s.size()) - return; - - if (t_type == TRANSFER_PREVIEW) - transfers.preview = parseExtFileList(s.buf(), s.buf() + s.size(), t_type); - else if (t_type == TRANSFER_IGNORE) - transfers.ignore = parseExtFileList(s.buf(), s.buf() + s.size(), t_type); - else if (t_type == TRANSFER_COMPLETE) - transfers.complete = parseExtFileList(s.buf(), s.buf() + s.size(), t_type); - else - fatalf("Unexpected transfer_type at %s:%d", __FILE__,__LINE__); -} - -List *ICAPOptions::parseExtFileList(const char *start, const char *end, transfer_type t_type) -{ - const String s = xstrndup(start, end - start + 1); - const char *item; - const char *pos = NULL; - char *fext = NULL; - int ilen; - String t = NULL; - - List **Tail = NULL; - List *H = NULL; - - for (Tail = &H; *Tail; Tail = &((*Tail)->next)) - - ; - while (strListGetItem(&s, ',', &item, &ilen, &pos)) { - fext = xstrndup(item, ilen + 1); - t = fext; - List *q = new List (t); - *(Tail) = q; - Tail = &q->next; - insertTransferExt(fext, t_type); - } - - return H; + debugs(93,7, "ICAPOptions url " << urlPath << " matches no extensions; " << + "using default: " << theTransfers.byDefault->name); + return theTransfers.byDefault->kind; } bool ICAPOptions::valid() const @@ -167,11 +105,9 @@ void ICAPOptions::configure(const HttpReply *reply) cfgIntHeader(h, "Preview", preview); - cfgTransferListHeader(h, "Transfer-Preview", TRANSFER_PREVIEW); - - cfgTransferListHeader(h, "Transfer-Ignore", TRANSFER_IGNORE); - - cfgTransferListHeader(h, "Transfer-Complete", TRANSFER_COMPLETE); + cfgTransferList(h, theTransfers.preview); + cfgTransferList(h, theTransfers.ignore); + cfgTransferList(h, theTransfers.complete); } void ICAPOptions::cfgMethod(ICAP::Method m) @@ -189,4 +125,81 @@ void ICAPOptions::cfgIntHeader(const HttpHeader *h, const char *fname, int &valu value = atoi(s.buf()); else value = -1; + + debugs(93,5, "ICAPOptions::cfgIntHeader " << fname << ": " << value); +} + +void ICAPOptions::cfgTransferList(const HttpHeader *h, TransferList &list) +{ + const String buf = h->getByName(list.name); + bool foundStar = false; + list.parse(buf, foundStar); + + if (foundStar) { + theTransfers.byDefault = &list; + debugs(93,5, "ICAPOptions::cfgTransferList: " << + "set default transfer to " << list.name); + } + + list.report(5, "ICAPOptions::cfgTransferList: "); +} + + +/* ICAPOptions::TransferList */ + +ICAPOptions::TransferList::TransferList(): extensions(NULL), name(NULL), + kind(xferNone) { +}; + +ICAPOptions::TransferList::~TransferList() { + wordlistDestroy(&extensions); +}; + +void ICAPOptions::TransferList::add(const char *extension) { + wordlistAdd(&extensions, extension); +}; + +bool ICAPOptions::TransferList::matches(const String &urlPath) const { + const int urlLen = urlPath.size(); + for (wordlist *e = extensions; e; e = e->next) { + // optimize: store extension lengths + const int eLen = strlen(e->key); + + // assume URL contains at least '/' before the extension + if (eLen < urlLen) { + const int eOff = urlLen - eLen; + // RFC 3507 examples imply that extensions come without leading '.' + if (urlPath.buf()[eOff-1] == '.' && + strcmp(urlPath.buf() + eOff, e->key) == 0) { + debugs(93,7, "ICAPOptions url " << urlPath << " matches " << + name << " extension " << e->key); + return true; + } + } + } + debugs(93,8, "ICAPOptions url " << urlPath << " matches no " << name << " extensions"); + return false; +} + +void ICAPOptions::TransferList::parse(const String &buf, bool &foundStar) { + foundStar = false; + + const char *item; + const char *pos = NULL; + int ilen; + while (strListGetItem(&buf, ',', &item, &ilen, &pos)) { + if (ilen == 1 && *item == '*') + foundStar = true; + else + add(xstrndup(item, ilen+1)); + } +} + +void ICAPOptions::TransferList::report(int level, const char *prefix) const { + if (extensions) { + for (wordlist *e = extensions; e; e = e->next) + debugs(93,level, prefix << name << ": " << e->key); + } else { + debugs(93,level, prefix << "no " << name << " extensions"); + } } diff --git a/src/ICAP/ICAPOptions.h b/src/ICAP/ICAPOptions.h index ca5bca9baa..58c3f9a632 100644 --- a/src/ICAP/ICAPOptions.h +++ b/src/ICAP/ICAPOptions.h @@ -1,6 +1,6 @@ /* - * $Id: ICAPOptions.h,v 1.7 2006/02/16 20:44:07 wessels Exp $ + * $Id: ICAPOptions.h,v 1.8 2006/10/31 23:30:58 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -35,9 +35,10 @@ #define SQUID_ICAPOPTIONS_H #include "squid.h" -#include "List.h" #include "ICAPClient.h" +class wordlist; + /* Maintains options supported by a given ICAP service. * See RFC 3507, Section "4.10.2 OPTIONS Response". */ @@ -61,8 +62,8 @@ public: int ttl() const { return theTTL; }; - typedef enum { TRANSFER_NONE, TRANSFER_PREVIEW, TRANSFER_IGNORE, TRANSFER_COMPLETE } transfer_type; - transfer_type getTransferExt(const char *); + typedef enum { xferNone, xferPreview, xferIgnore, xferComplete } TransferKind; + TransferKind transferKind(const String &urlPath) const; public: const char *error; // human-readable information; set iff !valid() @@ -78,38 +79,42 @@ public: bool allow204; int preview; - // varios Transfer-* lists +protected: + // Transfer-* extension list representation + // maintains wordlist and does parsing/matching + class TransferList { + public: + TransferList(); + ~TransferList(); + + bool matches(const String &urlPath) const; + + void parse(const String &buf, bool &foundStar); + void add(const char *extension); + void report(int level, const char *prefix) const; + + public: + wordlist *extensions; // TODO: optimize with a hash of some sort + const char *name; // header name, mostly for debugging + TransferKind kind; // to simplify caller's life + }; + // varios Transfer-* lists struct Transfers { - List *preview; - List *ignore; - List *complete; - transfer_type other; // default X from Transfer-X: * - } - - transfers; + TransferList preview; + TransferList ignore; + TransferList complete; + TransferList *byDefault; // Transfer-X that has '*' + } theTransfers; -protected: int theTTL; time_t theTimestamp; - // The list of pairs "file extension <-> transfer type" - - struct TransferPair - { - char *ext; - transfer_type type; - }; - - List *transfer_ext; - private: void cfgMethod(ICAP::Method m); void cfgIntHeader(const HttpHeader *h, const char *fname, int &value); - void insertTransferExt(const char *t, transfer_type t_type); - void cfgTransferListHeader(const HttpHeader *h, const char *fname, transfer_type type); - List *parseExtFileList(const char *start, const char *end, transfer_type t_type); + void cfgTransferList(const HttpHeader *h, TransferList &l); }; diff --git a/src/ICAP/ICAPServiceRep.cc b/src/ICAP/ICAPServiceRep.cc index cbd18e76bd..6c9c6ff630 100644 --- a/src/ICAP/ICAPServiceRep.cc +++ b/src/ICAP/ICAPServiceRep.cc @@ -12,14 +12,21 @@ CBDATA_CLASS_INIT(ICAPServiceRep); +// XXX: move to squid.conf +const int ICAPServiceRep::TheSessionFailureLimit = 10; + ICAPServiceRep::ICAPServiceRep(): method(ICAP::methodNone), - point(ICAP::pointNone), port(-1), bypass(false), unreachable(false), - theOptions(NULL), theState(stateInit), notifying(false), self(NULL) + point(ICAP::pointNone), port(-1), bypass(false), + theOptions(NULL), theLastUpdate(0), + theSessionFailures(0), isSuspended(0), + waiting(false), notifying(false), + updateScheduled(false), self(NULL), + wasAnnouncedUp(true) // do not announce an "up" service at startup {} ICAPServiceRep::~ICAPServiceRep() { - Must(!waiting()); + Must(!waiting); changeOptions(0); } @@ -158,22 +165,74 @@ ICAPServiceRep::configure(Pointer &aSelf) void ICAPServiceRep::invalidate() { assert(self != NULL); - self = NULL; // may destroy us and, hence, invalidate cbdata(this) + Pointer savedSelf = self; // to prevent destruction when we nullify self + self = NULL; + + announceStatusChange("invalidated by reconfigure", false); + + savedSelf = NULL; // may destroy us and, hence, invalidate cbdata(this) // TODO: it would be nice to invalidate cbdata(this) when not destroyed } +void ICAPServiceRep::noteFailure() { + ++theSessionFailures; + debugs(93,4, "ICAPService failure " << theSessionFailures << + ", out of " << TheSessionFailureLimit << " allowed"); + + if (theSessionFailures > TheSessionFailureLimit) + suspend("too many failures"); + + // TODO: Should bypass setting affect how much Squid tries to talk to + // the ICAP service that is currently unusable and is likely to remain + // so for some time? The current code says "no". Perhaps the answer + // should be configurable. +} + +void ICAPServiceRep::suspend(const char *reason) { + if (isSuspended) { + debugs(93,4, "keeping ICAPService suspended, also for " << reason); + } else { + isSuspended = reason; + debugs(93,1, "suspending ICAPService for " << reason); + announceStatusChange("suspended", true); + } +} + +bool ICAPServiceRep::probed() const +{ + return theLastUpdate != 0; +} + +bool ICAPServiceRep::hasOptions() const { + return theOptions && theOptions->valid() && theOptions->fresh(); +} + bool ICAPServiceRep::up() const { - return self != NULL && theState == stateUp; + return self != NULL && !isSuspended && hasOptions(); +} + +bool ICAPServiceRep::broken() const +{ + return probed() && !up(); +} + +bool ICAPServiceRep::wantsUrl(const String &urlPath) const +{ + Must(hasOptions()); + return theOptions->transferKind(urlPath) != ICAPOptions::xferIgnore; } -bool ICAPServiceRep::wantsPreview(size_t &wantedSize) const +bool ICAPServiceRep::wantsPreview(const String &urlPath, size_t &wantedSize) const { - Must(up()); + Must(hasOptions()); if (theOptions->preview < 0) return false; + if (theOptions->transferKind(urlPath) != ICAPOptions::xferPreview) + return false; + wantedSize = theOptions->preview; return true; @@ -181,7 +240,7 @@ bool ICAPServiceRep::wantsPreview(size_t &wantedSize) const bool ICAPServiceRep::allows204() const { - Must(up()); + Must(hasOptions()); return true; // in the future, we may have ACLs to prevent 204s } @@ -196,7 +255,10 @@ void ICAPServiceRep_noteTimeToUpdate(void *data) void ICAPServiceRep::noteTimeToUpdate() { - if (!self || waiting()) { + if (self != NULL) + updateScheduled = false; + + if (!self || waiting) { debugs(93,5, "ICAPService ignores options update " << status()); return; } @@ -241,6 +303,7 @@ void ICAPServiceRep::callWhenReady(Callback *cb, void *data) { Must(cb); Must(self != NULL); + Must(!broken()); // we do not wait for a broken service Client i; i.service = self; @@ -248,7 +311,7 @@ void ICAPServiceRep::callWhenReady(Callback *cb, void *data) i.data = cbdataReference(data); theClients.push_back(i); - if (waiting() || notifying) + if (waiting || notifying) return; // do nothing, we will be picked up in noteTimeToNotify() if (needNewOptions()) @@ -263,23 +326,28 @@ void ICAPServiceRep::scheduleNotification() eventAdd("ICAPServiceRep::noteTimeToNotify", &ICAPServiceRep_noteTimeToNotify, this, 0, 0, true); } -bool ICAPServiceRep::waiting() const -{ - return theState == stateWait; -} - bool ICAPServiceRep::needNewOptions() const { - return !theOptions || !theOptions->fresh(); + return self != NULL && !up(); } void ICAPServiceRep::changeOptions(ICAPOptions *newOptions) { debugs(93,9, "ICAPService changes options from " << theOptions << " to " << newOptions); + delete theOptions; theOptions = newOptions; + theSessionFailures = 0; + isSuspended = 0; + theLastUpdate = squid_curtime; + + checkOptions(); + announceStatusChange("down after an options fetch failure", true); +} +void ICAPServiceRep::checkOptions() +{ if (theOptions == NULL) return; @@ -292,10 +360,8 @@ void ICAPServiceRep::changeOptions(ICAPOptions *newOptions) bool method_found = false; String method_list; Vector ::iterator iter = theOptions->methods.begin(); - debugs(0,0,HERE); while (iter != theOptions->methods.end()) { - debugs(0,0,HERE); if (*iter == method) { method_found = true; @@ -305,11 +371,8 @@ void ICAPServiceRep::changeOptions(ICAPOptions *newOptions) method_list.append(ICAP::methodStr(*iter)); method_list.append(" ", 1); iter++; - debugs(0,0,HERE); } - debugs(0,0,HERE); - if (!method_found) { debugs(93,1, "WARNING: Squid is configured to use ICAP method " << ICAP::methodStr(method) << @@ -323,24 +386,21 @@ void ICAPServiceRep::changeOptions(ICAPOptions *newOptions) * Check the ICAP server's date header for clock skew */ int skew = abs((int)(theOptions->timestamp() - squid_curtime)); - if (skew > theOptions->ttl()) debugs(93, 1, host.buf() << "'s clock is skewed by " << skew << " seconds!"); +} -#if 0 - - List *tmp; - - for (tmp = theOptions->transfers.preview; tmp; tmp=tmp->next) - debugs(93,1,"Transfer-Preview: " << tmp->element.buf()); - - for (tmp = theOptions->transfers.ignore; tmp; tmp=tmp->next) - debugs(93,1,"Transfer-Ignore: " << tmp->element.buf()); +void ICAPServiceRep::announceStatusChange(const char *downPhrase, bool important) const +{ + if (wasAnnouncedUp == up()) // no significant changes to announce + return; - for (tmp = theOptions->transfers.complete; tmp; tmp=tmp->next) - debugs(93,1,"Transfer-Complete: " << tmp->element.buf()); + const char *what = bypass ? "optional" : "essential"; + const char *state = wasAnnouncedUp ? downPhrase : "up"; + const int level = important ? 1 : 2; + debugs(93,level, what << " ICAP service is " << state << ": " << uri); -#endif + wasAnnouncedUp = !wasAnnouncedUp; } static @@ -354,91 +414,106 @@ void ICAPServiceRep_noteNewOptions(ICAPOptXact *x, void *data) void ICAPServiceRep::noteNewOptions(ICAPOptXact *x) { Must(x); - Must(waiting()); - - theState = stateDown; // default in case we fail to set new options + Must(waiting); + waiting = false; changeOptions(x->options); x->options = NULL; delete x; - if (theOptions && theOptions->valid()) - theState = stateUp; - - debugs(93,6, "ICAPService got new options and is now " << - (up() ? "up" : "down")); + debugs(93,3, "ICAPService got new options and is now " << status()); scheduleUpdate(); - scheduleNotification(); } void ICAPServiceRep::startGettingOptions() { + Must(!waiting); debugs(93,6, "ICAPService will get new options " << status()); - theState = stateWait; + waiting = true; ICAPOptXact *x = new ICAPOptXact; x->start(self, &ICAPServiceRep_noteNewOptions, this); - // TODO: timeout incase ICAPOptXact never calls us back? + // TODO: timeout in case ICAPOptXact never calls us back? } void ICAPServiceRep::scheduleUpdate() { - int delay = -1; + if (updateScheduled) + return; // already scheduled + + // XXX: move hard-coded constants from here to TheICAPConfig + + // conservative estimate of how long the OPTIONS transaction will take + const int expectedWait = 20; // seconds + + time_t when = 0; if (theOptions && theOptions->valid()) { const time_t expire = theOptions->expire(); + debugs(93,7, "ICAPService options expire on " << expire << " >= " << squid_curtime); - if (expire > squid_curtime) - delay = expire - squid_curtime; + if (expire < 0) // unknown expiration time + when = squid_curtime + 60*60; else - if (expire >= 0) - delay = 1; // delay for expired or 'expiring now' options - else - delay = 60*60; // default for options w/o known expiration time + if (expire < expectedWait) // invalid expiration time + when = squid_curtime + 60*60; + else + when = expire - expectedWait; // before the current options expire } else { - delay = 5*60; // delay for a down service + when = squid_curtime + 3*60; // delay for a down service } - if (delay <= 0) { - debugs(93,0, "internal error: ICAPServiceRep failed to compute options update schedule"); - delay = 5*60; // delay for an internal error - } + debugs(93,7, "ICAPService options raw update on " << when << " or " << (when - squid_curtime)); + if (when < squid_curtime) + when = squid_curtime; + + const int minUpdateGap = 1*60; // seconds + if (when < theLastUpdate + minUpdateGap) + when = theLastUpdate + minUpdateGap; - // with zero delay, the state changes to stateWait before - // notifications are sent out to clients - assert(delay > 0); + // TODO: keep the time of the last update to prevet too-frequent updates - debugs(93,7, "ICAPService will update options in " << delay << " sec"); + const int delay = when - squid_curtime; + + debugs(93,5, "ICAPService will update options in " << delay << " sec"); eventAdd("ICAPServiceRep::noteTimeToUpdate", &ICAPServiceRep_noteTimeToUpdate, this, delay, 0, true); - - // XXX: prompt updates of valid options should not disable concurrent ICAP - // xactions. 'Wait' state should not mark the service 'down'! This will - // also remove 'delay == 0' as a special case above. + updateScheduled = true; } +// returns a temporary string depicting service status, for debugging const char *ICAPServiceRep::status() const { + static MemBuf buf; + + buf.reset(); + buf.append("[", 1); + + if (up()) + buf.append("up", 2); + else + buf.append("down", 4); + if (!self) - return "[invalidated]"; + buf.append(",gone", 5); - switch (theState) { + if (waiting) + buf.append(",wait", 5); - case stateInit: - return "[init]"; + if (notifying) + buf.append(",notif", 6); - case stateWait: - return "[wait]"; + if (theSessionFailures > 0) + buf.Printf(",F%d", theSessionFailures); - case stateUp: - return "[up]"; + if (isSuspended) + buf.append(",susp", 5); - case stateDown: - return "[down]"; - } + buf.append("]", 1); + buf.terminate(); - return "[unknown]"; + return buf.content(); } diff --git a/src/ICAP/ICAPServiceRep.h b/src/ICAP/ICAPServiceRep.h index 115f89e24a..4f9af4a1c8 100644 --- a/src/ICAP/ICAPServiceRep.h +++ b/src/ICAP/ICAPServiceRep.h @@ -1,6 +1,6 @@ /* - * $Id: ICAPServiceRep.h,v 1.4 2006/08/21 00:50:45 robertc Exp $ + * $Id: ICAPServiceRep.h,v 1.5 2006/10/31 23:30:58 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -44,7 +44,28 @@ class ICAPOptXact; /* The ICAP service representative maintains information about a single ICAP service that Squid communicates with. The representative initiates OPTIONS requests to the service to keep cached options fresh. One ICAP server may - host many ICAP services */ + host many ICAP services. */ + +/* + * A service is "up" if there is a fresh cached OPTIONS response and is + * "down" otherwise. A service is "probed" if we tried to get an OPTIONS + * response from it and succeeded or failed. A probed down service is + * called "broken". + * + * As a bootstrapping mechanism, ICAP transactions wait for an unprobed + * service to get a fresh OPTIONS response (see the callWhenReady method). + * The waiting callback is called when the OPTIONS transaction completes, + * even if the service is now broken. + * + * We do not initiate ICAP transactions with a broken service, but will + * eventually retry to fetch its options in hope to bring the service up. + * + * A service that should no longer be used after Squid reconfiguration is + * treated as if it does not have a fresh cached OPTIONS response. We do + * not try to fetch fresh options for such a service. It should be + * auto-destroyed by refcounting when no longer used. + */ + class ICAPServiceRep : public RefCountable { @@ -62,22 +83,20 @@ public: const char *methodStr() const; const char *vectPointStr() const; - bool up() const; + bool probed() const; // see comments above + bool broken() const; // see comments above + bool up() const; // see comments above - /* Service is "up" iff there is a fresh cached OPTIONS response. To - get an OPTIONS response, ICAPServiceRep does an OPTIONS - transaction. Failed transaction results in a "down" service. The - Callback is called if/once the service is in a steady ("up" or - "down") state. */ typedef void Callback(void *data, Pointer &service); void callWhenReady(Callback *cb, void *data); - // the methods below can only be called on an up() service - - bool wantsPreview(size_t &wantedSize) const; + bool wantsUrl(const String &urlPath) const; + bool wantsPreview(const String &urlPath, size_t &wantedSize) const; bool allows204() const; + void noteFailure(); // called by transactions to report service failure + public: String key; ICAP::Method method; @@ -89,9 +108,8 @@ public: int port; String resource; - // non-options flags; TODO: check that both are used. + // XXX: use it when selecting a service and handling ICAP errors! bool bypass; - bool unreachable; public: // treat these as private, they are for callbacks only void noteTimeToUpdate(); @@ -112,26 +130,38 @@ private: Clients theClients; // all clients waiting for a call back ICAPOptions *theOptions; + time_t theLastUpdate; // time the options were last updated + + static const int TheSessionFailureLimit; + int theSessionFailures; + const char *isSuspended; // also stores suspension reason for debugging - typedef enum { stateInit, stateWait, stateUp, stateDown } State; - State theState; + bool waiting; // for an OPTIONS transaction to finish bool notifying; // may be true in any state except for the initial + bool updateScheduled; // time-based options update has been scheduled private: ICAP::Method parseMethod(const char *) const; ICAP::VectPoint parseVectPoint(const char *) const; - bool waiting() const; + void suspend(const char *reason); + + bool hasOptions() const; bool needNewOptions() const; + void scheduleUpdate(); void scheduleNotification(); - void changeOptions(ICAPOptions *newOptions); + void startGettingOptions(); - void scheduleUpdate(); + void changeOptions(ICAPOptions *newOptions); + void checkOptions(); + + void announceStatusChange(const char *downPhrase, bool important) const; const char *status() const; Pointer self; + mutable bool wasAnnouncedUp; // prevent sequential same-state announcements CBDATA_CLASS2(ICAPServiceRep); }; diff --git a/src/ICAP/ICAPXaction.cc b/src/ICAP/ICAPXaction.cc index ab8bbd2bfd..c87aeaad7d 100644 --- a/src/ICAP/ICAPXaction.cc +++ b/src/ICAP/ICAPXaction.cc @@ -46,7 +46,7 @@ void ICAPXaction_noteCommConnected(int, comm_err_t status, int xerrno, void *dat } static -void ICAPXaction_noteCommWrote(int, char *, size_t size, comm_err_t status, void *data) +void ICAPXaction_noteCommWrote(int, char *, size_t size, comm_err_t status, int xerrno, void *data) { ICAPXaction_fromData(data).noteCommWrote(status, size); } @@ -68,6 +68,7 @@ ICAPXaction::ICAPXaction(const char *aTypeName): theService(NULL), inCall(NULL) { + debug(93,3)("%s constructed, this=%p\n", typeName, this); readBuf.init(SQUID_TCP_SO_RCVBUF, SQUID_TCP_SO_RCVBUF); commBuf = (char*)memAllocBuf(SQUID_TCP_SO_RCVBUF, &commBufSize); // make sure maximum readBuf space does not exceed commBuf size @@ -76,6 +77,7 @@ ICAPXaction::ICAPXaction(const char *aTypeName): ICAPXaction::~ICAPXaction() { + debug(93,3)("%s destructing, this=%p\n", typeName, this); doStop(); readBuf.clean(); memFreeBuf(commBufSize, commBuf); @@ -104,7 +106,7 @@ void ICAPXaction::openConnection() COMM_NONBLOCKING, s.uri.buf()); if (connection < 0) - throw TexcHere("cannot connect to ICAP service " /* + uri */); + dieOnConnectionFailure(); // throws } debugs(93,3, typeName << " opens connection to " << s.host.buf() << ":" << s.port); @@ -138,23 +140,31 @@ ICAPXaction::reusedConnection(void *data) void ICAPXaction::closeConnection() { if (connection >= 0) { - commSetTimeout(connection, -1, NULL, NULL); if (closer) { comm_remove_close_handler(connection, closer, this); closer = NULL; } - cancelRead(); + cancelRead(); // may not work + + if (reuseConnection && (writer || reader)) { + debugs(93,5, HERE << "not reusing pconn due to pending I/O " << status()); + reuseConnection = false; + } if (reuseConnection) { - debugs(93,3, HERE << "pushing pconn " << connection); + debugs(93,3, HERE << "pushing pconn " << status()); + commSetTimeout(connection, -1, NULL, NULL); icapPconnPool->push(connection, theService->host.buf(), theService->port, NULL); } else { - debugs(93,3, HERE << "closing pconn " << connection); + debugs(93,3, HERE << "closing pconn " << status()); + // comm_close will clear timeout comm_close(connection); } + writer = NULL; + reader = NULL; connector = NULL; connection = -1; } @@ -167,20 +177,30 @@ void ICAPXaction::noteCommConnected(comm_err_t commStatus) Must(connector); connector = NULL; - Must(commStatus == COMM_OK); + + if (commStatus != COMM_OK) + dieOnConnectionFailure(); // throws + + fd_table[connection].noteUse(icapPconnPool); handleCommConnected(); ICAPXaction_Exit(); } +void ICAPXaction::dieOnConnectionFailure() { + theService->noteFailure(); + debugs(93,3, typeName << " failed to connect to the ICAP service at " << + service().uri); + throw TexcHere("cannot connect to the ICAP service"); +} + void ICAPXaction::scheduleWrite(MemBuf &buf) { // comm module will free the buffer - writer = (IOCB *)&ICAPXaction_noteCommWrote; + writer = &ICAPXaction_noteCommWrote; comm_write_mbuf(connection, &buf, writer, this); - fd_table[connection].noteUse(icapPconnPool); - commSetTimeout(connection, 61, &ICAPXaction_noteCommTimedout, this); + updateTimeout(); } void ICAPXaction::noteCommWrote(comm_err_t commStatus, size_t size) @@ -192,6 +212,8 @@ void ICAPXaction::noteCommWrote(comm_err_t commStatus, size_t size) Must(commStatus == COMM_OK); + updateTimeout(); + handleCommWrote(size); ICAPXaction_Exit(); @@ -239,12 +261,8 @@ void ICAPXaction::handleCommClosed() bool ICAPXaction::done() const { - if (stopReason != NULL) { // mustStop() has been called - debugs(93,1,HERE << "ICAPXaction is done() because " << stopReason); - return true; - } - - return doneAll(); + // stopReason, set in mustStop(), overwrites all other conditions + return stopReason != NULL || doneAll(); } bool ICAPXaction::doneAll() const @@ -252,6 +270,19 @@ bool ICAPXaction::doneAll() const return !connector && !reader && !writer; } +void ICAPXaction::updateTimeout() { + if (reader || writer) { + // restart the timeout before each I/O + // XXX: why does Config.Timeout lacks a write timeout? + commSetTimeout(connection, Config.Timeout.read, + &ICAPXaction_noteCommTimedout, this); + } else { + // clear timeout when there is no I/O + // Do we need a lifetime timeout? + commSetTimeout(connection, -1, NULL, NULL); + } +} + void ICAPXaction::scheduleRead() { Must(connection >= 0); @@ -265,7 +296,7 @@ void ICAPXaction::scheduleRead() */ comm_read(connection, commBuf, readBuf.spaceSize(), reader, this); - commSetTimeout(connection, 61, &ICAPXaction_noteCommTimedout, this); + updateTimeout(); } // comm module read a portion of the ICAP response for us @@ -279,6 +310,8 @@ void ICAPXaction::noteCommRead(comm_err_t commStatus, size_t sz) Must(commStatus == COMM_OK); Must(sz >= 0); + updateTimeout(); + debugs(93, 3, HERE << "read " << sz << " bytes"); /* @@ -305,10 +338,10 @@ void ICAPXaction::cancelRead() // These checks try to mimic the comm_read_cancel() assertions. if (comm_has_pending_read(connection) && - !comm_has_pending_read_callback(connection)) + !comm_has_pending_read_callback(connection)) { comm_read_cancel(connection, reader, this); - - reader = NULL; + reader = NULL; + } } } @@ -341,13 +374,28 @@ bool ICAPXaction::doneReading() const return commEof; } +bool ICAPXaction::doneWriting() const +{ + return !writer; +} + +bool ICAPXaction::doneWithIo() const +{ + return connection >= 0 && // or we could still be waiting to open it + !connector && !reader && !writer && // fast checks, some redundant + doneReading() && doneWriting(); +} + void ICAPXaction::mustStop(const char *aReason) { Must(inCall); // otherwise nobody will call doStop() - Must(!stopReason); Must(aReason); - stopReason = aReason; - debugs(93, 5, typeName << " will stop, reason: " << stopReason); + if (!stopReason) { + stopReason = aReason; + debugs(93, 5, typeName << " will stop, reason: " << stopReason); + } else { + debugs(93, 5, typeName << " will stop, another reason: " << aReason); + } } // internal cleanup @@ -392,8 +440,8 @@ void ICAPXaction::callException(const TextException &e) debugs(93, 4, typeName << "::" << inCall << " caught an exception: " << e.message << ' ' << status()); - if (!done()) - mustStop("exception"); + reuseConnection = false; // be conservative + mustStop("exception"); } void ICAPXaction::callEnd() @@ -403,6 +451,10 @@ void ICAPXaction::callEnd() status()); doStop(); // may delete us return; + } else + if (doneWithIo()) { + debugs(93, 5, HERE << typeName << " done with I/O " << status()); + closeConnection(); } debugs(93, 6, typeName << "::" << inCall << " ended " << status()); diff --git a/src/ICAP/ICAPXaction.h b/src/ICAP/ICAPXaction.h index 4999fcd13d..b4fac40b87 100644 --- a/src/ICAP/ICAPXaction.h +++ b/src/ICAP/ICAPXaction.h @@ -1,6 +1,6 @@ /* - * $Id: ICAPXaction.h,v 1.8 2006/09/19 17:17:52 serassio Exp $ + * $Id: ICAPXaction.h,v 1.9 2006/10/31 23:30:58 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -79,14 +79,19 @@ protected: void openConnection(); void closeConnection(); + void dieOnConnectionFailure(); + void scheduleRead(); void scheduleWrite(MemBuf &buf); + void updateTimeout(); void cancelRead(); 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; @@ -152,20 +157,20 @@ private: // - open the try clause; // - call callStart(). #define ICAPXaction_Enter(method) \ - try { \ - if (!callStart(#method)) \ - return; + 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(); + } \ + catch (const TextException &e) { \ + callException(e); \ + } \ + callEnd(); #endif /* SQUID_ICAPXACTION_H */ diff --git a/src/ICAP/MsgPipe.cc b/src/ICAP/MsgPipe.cc index 0c6c7e00e9..83f237c874 100644 --- a/src/ICAP/MsgPipe.cc +++ b/src/ICAP/MsgPipe.cc @@ -11,9 +11,9 @@ CBDATA_CLASS_INIT(MsgPipe); #define MsgPipe_MAKE_CALLBACK(callName, destination) \ static \ void MsgPipe_send ## callName(void *p) { \ - MsgPipe *pipe = static_cast(p); \ - if (pipe && pipe->canSend(pipe->destination, #callName, false)) \ - pipe->destination->note##callName(pipe); \ + MsgPipe *pipe = static_cast(p); \ + if (pipe && pipe->canSend(pipe->destination, #callName, false)) \ + pipe->destination->note##callName(pipe); \ } // static event callbacks @@ -90,7 +90,7 @@ bool MsgPipe::canSend(MsgPipeEnd *destination, const char *callName, bool future const char *verb = future ? (res ? "will send " : "wont send ") : (res ? "sends " : "ignores "); - debugs(99,5, "MsgPipe " << name << "(" << this << ") " << + debugs(93,5, "MsgPipe " << name << "(" << this << ") " << verb << callName << " to the " << (destination ? destination->kind() : "destination") << "(" << destination << "); " << diff --git a/src/ICAP/MsgPipeData.h b/src/ICAP/MsgPipeData.h index bc6e2bb06f..d30beb5c15 100644 --- a/src/ICAP/MsgPipeData.h +++ b/src/ICAP/MsgPipeData.h @@ -1,6 +1,6 @@ /* - * $Id: MsgPipeData.h,v 1.7 2006/02/17 18:11:00 wessels Exp $ + * $Id: MsgPipeData.h,v 1.8 2006/10/31 23:30:58 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -50,7 +50,7 @@ class MsgPipeData { public: - MsgPipeData(): header(0), body(0), cause(0) {}; + MsgPipeData(): header(0), body(0), cause(0) {} ~MsgPipeData() { @@ -61,19 +61,23 @@ public: body->clean(); delete body; } - }; + } void setCause(HttpRequest *r) { - HTTPMSGUNLOCK(cause); - cause = HTTPMSGLOCK(r); - }; + if (r) { + HTTPMSGUNLOCK(cause); + cause = HTTPMSGLOCK(r); + } else { + assert(!cause); + } + } void setHeader(HttpMsg *msg) { HTTPMSGUNLOCK(header); header = HTTPMSGLOCK(msg); - }; + } public: typedef HttpMsg Header; diff --git a/src/ICAP/TextException.h b/src/ICAP/TextException.h index 375ff16460..3a78dadf2d 100644 --- a/src/ICAP/TextException.h +++ b/src/ICAP/TextException.h @@ -27,20 +27,20 @@ protected: //inline //ostream &operator <<(ostream &os, const TextException &exx) { -// return exx.print(os); +// return exx.print(os); //} #if !defined(TexcHere) -# define TexcHere(msg) TextException((msg), __FILE__, __LINE__) +# define TexcHere(msg) TextException((msg), __FILE__, __LINE__) #endif extern void Throw(const char *message, const char *fileName, int lineNo); // Must(condition) is like assert(condition) but throws an exception instead #if !defined(Must) -# define Must(cond) ((cond) ? \ - (void)0 : \ - (void)Throw(#cond, __FILE__, __LINE__)) +# define Must(cond) ((cond) ? \ + (void)0 : \ + (void)Throw(#cond, __FILE__, __LINE__)) #endif #endif /* SQUID__TEXTEXCEPTION_H */ diff --git a/src/Makefile.am b/src/Makefile.am index b8a1d355c8..0899ef7601 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1,7 +1,7 @@ # # Makefile for the Squid Object Cache server # -# $Id: Makefile.am,v 1.171 2006/09/15 15:01:25 hno Exp $ +# $Id: Makefile.am,v 1.172 2006/10/31 23:30:56 wessels Exp $ # # Uncomment and customize the following to suit your needs: # @@ -670,6 +670,8 @@ ICAP_libicap_a_SOURCES = \ ICAP/ChunkedCodingParser.h \ ICAP/ICAPClient.cc \ ICAP/ICAPClient.h \ + ICAP/ICAPClientVector.cc \ + ICAP/ICAPClientVector.h \ ICAP/ICAPClientReqmodPrecache.cc \ ICAP/ICAPClientReqmodPrecache.h \ ICAP/ICAPClientRespmodPrecache.cc \ diff --git a/src/Server.cc b/src/Server.cc index 04a67683c1..2566208a9d 100644 --- a/src/Server.cc +++ b/src/Server.cc @@ -1,5 +1,5 @@ /* - * $Id: Server.cc,v 1.6 2006/09/20 22:26:24 hno Exp $ + * $Id: Server.cc,v 1.7 2006/10/31 23:30:56 wessels Exp $ * * DEBUG: * AUTHOR: Duane Wessels @@ -62,24 +62,35 @@ ServerStateData::~ServerStateData() fwd = NULL; // refcounted #if ICAP_CLIENT - if (icap) + if (icap) { + debug(11,5)("ServerStateData destroying icap=%p\n", icap); + icap->ownerAbort(); delete icap; + } #endif } #if ICAP_CLIENT /* - * Initiate an ICAP transaction. Return 0 if all is well, or -1 upon error. + * Initiate an ICAP transaction. Return true on success. * Caller will handle error condition by generating a Squid error message * or take other action. */ -int -ServerStateData::doIcap(ICAPServiceRep::Pointer service) +bool +ServerStateData::startIcap(ICAPServiceRep::Pointer service) { - debug(11,5)("ServerStateData::doIcap() called\n"); + debug(11,5)("ServerStateData::startIcap() called\n"); + if (!service) { + debug(11,3)("ServerStateData::startIcap fails: lack of service\n"); + return false; + } + if (service->broken()) { + debug(11,3)("ServerStateData::startIcap fails: broken service\n"); + return false; + } assert(NULL == icap); icap = new ICAPClientRespmodPrecache(service); - return 0; + return true; } #endif diff --git a/src/Server.h b/src/Server.h index 75a8d0ebf1..5fb82c160d 100644 --- a/src/Server.h +++ b/src/Server.h @@ -1,6 +1,6 @@ /* - * $Id: Server.h,v 1.1 2006/01/25 17:47:26 wessels Exp $ + * $Id: Server.h,v 1.2 2006/10/31 23:30:56 wessels Exp $ * * AUTHOR: Duane Wessels * @@ -59,10 +59,9 @@ public: virtual ~ServerStateData(); #if ICAP_CLIENT - - virtual void takeAdaptedHeaders(HttpReply *) = 0; - virtual void takeAdaptedBody(MemBuf *) = 0; - virtual void doneAdapting() = 0; + virtual bool takeAdaptedHeaders(HttpReply *) = 0; + virtual bool takeAdaptedBody(MemBuf *) = 0; + virtual void finishAdapting() = 0; virtual void abortAdapting() = 0; virtual void icapSpaceAvailable() = 0; virtual void icapAclCheckDone(ICAPServiceRep::Pointer) = 0; @@ -80,7 +79,7 @@ protected: ICAPClientRespmodPrecache *icap; bool icapAccessCheckPending; - int doIcap(ICAPServiceRep::Pointer); + bool startIcap(ICAPServiceRep::Pointer); #endif }; diff --git a/src/client_side.cc b/src/client_side.cc index 19c9d5a779..0b533cd1d6 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -1,6 +1,6 @@ /* - * $Id: client_side.cc,v 1.743 2006/10/19 01:39:40 wessels Exp $ + * $Id: client_side.cc,v 1.744 2006/10/31 23:30:56 wessels Exp $ * * DEBUG: section 33 Client-side Routines * AUTHOR: Duane Wessels @@ -2558,15 +2558,14 @@ clientReadBody(void *data, MemBuf &mb, size_t size) debugs(33,3,HERE << "clientReadBody in.notYetUsed " << conn->in.notYetUsed); if (size > conn->in.notYetUsed) - size = conn->in.notYetUsed; + size = conn->in.notYetUsed; // may make size zero debugs(33,3,HERE << "clientReadBody actual size " << size); - assert(size); - - mb.append(conn->in.buf, size); - - connNoteUseOfBuffer(conn, size); + if (size > 0) { + mb.append(conn->in.buf, size); + connNoteUseOfBuffer(conn, size); + } return size; } diff --git a/src/client_side_request.cc b/src/client_side_request.cc index fb55cc9f40..b4e388e167 100644 --- a/src/client_side_request.cc +++ b/src/client_side_request.cc @@ -1,6 +1,6 @@ /* - * $Id: client_side_request.cc,v 1.76 2006/10/19 00:35:35 wessels Exp $ + * $Id: client_side_request.cc,v 1.77 2006/10/31 23:30:57 wessels Exp $ * * DEBUG: section 85 Client-side Request Routines * AUTHOR: Robert Collins (Originally Duane Wessels in client_side.c) @@ -526,6 +526,9 @@ ClientRequestContext::icapAclCheckDone(ICAPServiceRep::Pointer service) * to the user, or keep going without ICAP. */ fatal("Fix this case in ClientRequestContext::icapAclCheckDone()"); + // And when fixed, check whether the service is down in doIcap and + // if it is, abort early, without creating ICAPClientReqmodPrecache. + // See Server::startIcap() and its use. http->doCallouts(); } @@ -1149,9 +1152,9 @@ ClientHttpRequest::icapSendRequestBody(MemBuf &mb) icap->body_reader->bytes_read += size_to_send; debugs(32,3," HTTP client body bytes_read=" << icap->body_reader->bytes_read); } else { - debugs(32,0,HERE << "cannot send body data to ICAP"); - debugs(32,0,HERE << "\tBodyReader MemBuf has " << mb.contentSize()); - debugs(32,0,HERE << "\tbut icap->potentialSpaceSize() is " << icap->potentialSpaceSize()); + debugs(32,2,HERE << "cannot send body data to ICAP"); + debugs(32,2,HERE << "\tBodyReader MemBuf has " << mb.contentSize()); + debugs(32,2,HERE << "\tbut icap->potentialSpaceSize() is " << icap->potentialSpaceSize()); return; } diff --git a/src/comm.cc b/src/comm.cc index 308a078d3f..f7830fcd58 100644 --- a/src/comm.cc +++ b/src/comm.cc @@ -1,6 +1,6 @@ /* - * $Id: comm.cc,v 1.427 2006/09/25 15:04:07 adrian Exp $ + * $Id: comm.cc,v 1.428 2006/10/31 23:30:57 wessels Exp $ * * DEBUG: section 5 Socket Functions * AUTHOR: Harvest Derived @@ -2063,6 +2063,18 @@ fdc_t::acceptCount() const { void fdc_t::acceptOne(int fd) { + // If there is no callback and we accept, we will leak the accepted FD. + // When we are running out of FDs, there is often no callback. + if (!accept.accept.callback.handler) { + debug (5,5) ("fdc_t::acceptOne orphaned: FD %d\n", fd); + // XXX: can we remove this and similar "just in case" calls and + // either listen always or listen only when there is a callback? + if (!AcceptLimiter::Instance().deferring()) + commSetSelect(fd, COMM_SELECT_READ, comm_accept_try, NULL, 0); + accept.accept.finished(true); + return; + } + /* * We don't worry about running low on FDs here. Instead, * httpAccept() will use AcceptLimiter if we reach the limit @@ -2077,6 +2089,7 @@ fdc_t::acceptOne(int fd) { if (newfd < 0) { if (newfd == COMM_NOMESSAGE) { /* register interest again */ + debug (5,5) ("fdc_t::acceptOne eof: FD %d handler: %p\n", fd, (void*)accept.accept.callback.handler); commSetSelect(fd, COMM_SELECT_READ, comm_accept_try, NULL, 0); accept.accept.finished(true); return; @@ -2092,6 +2105,9 @@ fdc_t::acceptOne(int fd) { return; } + debug (5,5) ("fdc_t::acceptOne accepted: FD %d handler: %p newfd: %d\n", fd, (void*)accept.accept.callback.handler, newfd); + + assert(accept.accept.callback.handler); accept.accept.doCallback(fd, newfd, COMM_OK, 0, &accept.connDetails); /* If we weren't re-registed, don't bother trying again! */ @@ -2136,6 +2152,7 @@ comm_accept_try(int fd, void *data) { */ void comm_accept(int fd, IOACB *handler, void *handler_data) { + debug (5,5) ("comm_accept: FD %d handler: %p\n", fd, (void*)handler); requireOpenAndActive(fd); /* make sure we're not pending! */ @@ -2209,8 +2226,14 @@ AcceptLimiter &AcceptLimiter::Instance() { return Instance_; } +bool +AcceptLimiter::deferring() const { + return deferred.size() > 0; +} + void AcceptLimiter::defer (int fd, Acceptor::AcceptorFunction *aFunc, void *data) { + debug (5,5) ("AcceptLimiter::defer: FD %d handler: %p\n", fd, (void*)aFunc); Acceptor temp; temp.theFunction = aFunc; temp.acceptFD = fd; @@ -2220,7 +2243,7 @@ AcceptLimiter::defer (int fd, Acceptor::AcceptorFunction *aFunc, void *data) { void AcceptLimiter::kick() { - if (!deferred.size()) + if (!deferring()) return; /* Yes, this means the first on is the last off.... diff --git a/src/comm.h b/src/comm.h index 317d69fb3e..ab42717262 100644 --- a/src/comm.h +++ b/src/comm.h @@ -114,6 +114,8 @@ public: void defer (int, Acceptor::AcceptorFunction *, void *); void kick(); + bool deferring() const; + private: static AcceptLimiter Instance_; Vector deferred; diff --git a/src/ftp.cc b/src/ftp.cc index e2e70ceb02..c5fb7f4497 100644 --- a/src/ftp.cc +++ b/src/ftp.cc @@ -1,6 +1,6 @@ /* - * $Id: ftp.cc,v 1.407 2006/09/19 07:56:57 adrian Exp $ + * $Id: ftp.cc,v 1.408 2006/10/31 23:30:57 wessels Exp $ * * DEBUG: section 9 File Transfer Protocol (FTP) * AUTHOR: Harvest Derived @@ -223,12 +223,15 @@ public: public: void icapAclCheckDone(ICAPServiceRep::Pointer); - void takeAdaptedHeaders(HttpReply *); - void takeAdaptedBody(MemBuf *); - void doneAdapting(); - void abortAdapting(); - void icapSpaceAvailable(); + virtual bool takeAdaptedHeaders(HttpReply *); + virtual bool takeAdaptedBody(MemBuf *); + virtual void finishAdapting(); + virtual void abortAdapting(); + virtual void icapSpaceAvailable(); bool icapAccessCheckPending; +private: + void backstabAdapter(); + void endAdapting(); #endif }; @@ -3353,18 +3356,18 @@ FtpStateData::icapAclCheckDone(ICAPServiceRep::Pointer service) { icapAccessCheckPending = false; - if (service == NULL) { - // handle case where no service is selected; + const bool startedIcap = startIcap(service); + + if (!startedIcap && (!service || service->bypass)) { + // handle ICAP start failure when no service was selected + // or where the selected service was optional entry->replaceHttpReply(reply); processReplyBody(); return; } - if (doIcap(service) < 0) { - /* - * XXX Maybe instead of an error page we should - * handle the reply normally (without ICAP). - */ + if (!startedIcap) { + // handle start failure for an essential ICAP service ErrorState *err = errorCon(ERR_ICAP_FAILURE, HTTP_INTERNAL_SERVER_ERROR, request); err->xerrno = errno; errorAppendEntry(entry, err); @@ -3386,15 +3389,15 @@ FtpStateData::icapSpaceAvailable() maybeReadData(); } -void +bool FtpStateData::takeAdaptedHeaders(HttpReply *rep) { debug(11,5)("FtpStateData::takeAdaptedHeaders() called\n"); if (!entry->isAccepting()) { debug(11,5)("\toops, entry is not Accepting!\n"); - icap->ownerAbort(); - return; + backstabAdapter(); + return false; } assert (rep); @@ -3404,9 +3407,10 @@ FtpStateData::takeAdaptedHeaders(HttpReply *rep) reply = HTTPMSGLOCK(rep); debug(11,5)("FtpStateData::takeAdaptedHeaders() finished\n"); + return true; } -void +bool FtpStateData::takeAdaptedBody(MemBuf *buf) { debug(11,5)("FtpStateData::takeAdaptedBody() called\n"); @@ -3414,37 +3418,27 @@ FtpStateData::takeAdaptedBody(MemBuf *buf) if (!entry->isAccepting()) { debug(11,5)("\toops, entry is not Accepting!\n"); - icap->ownerAbort(); - return; + backstabAdapter(); + return false; } storeAppend(entry, buf->content(), buf->contentSize()); buf->consume(buf->contentSize()); // consume everything written + return true; } void -FtpStateData::doneAdapting() +FtpStateData::finishAdapting() { debug(11,5)("FtpStateData::doneAdapting() called\n"); if (!entry->isAccepting()) { debug(11,5)("\toops, entry is not Accepting!\n"); - icap->ownerAbort(); + backstabAdapter(); } else { - transactionForwardComplete(); + transactionForwardComplete(); + endAdapting(); } - - /* - * ICAP is done, so we don't need this any more. - */ - delete icap; - - cbdataReferenceDone(icap); - - if (ctrl.fd >= 0) - comm_close(ctrl.fd); - else - delete this; } void @@ -3452,12 +3446,6 @@ FtpStateData::abortAdapting() { debug(11,5)("FtpStateData::abortAdapting() called\n"); - /* - * ICAP has given up, we're done with it too - */ - delete icap; - cbdataReferenceDone(icap); - if (entry->isEmpty()) { ErrorState *err; err = errorCon(ERR_ICAP_FAILURE, HTTP_INTERNAL_SERVER_ERROR, request); @@ -3466,10 +3454,30 @@ FtpStateData::abortAdapting() fwd->dontRetry(true); } + endAdapting(); +} + +// internal helper to terminate adotation when called by the adapter +void +FtpStateData::backstabAdapter() +{ + debug(11,5)("HttpStateData::backstabAdapter() called for %p\n", icap); + assert(icap); + icap->ownerAbort(); + endAdapting(); +} + +void +FtpStateData::endAdapting() +{ + delete icap; + icap = NULL; + if (ctrl.fd >= 0) comm_close(ctrl.fd); else delete this; } + #endif diff --git a/src/http.cc b/src/http.cc index c5546b6d15..591ebf3899 100644 --- a/src/http.cc +++ b/src/http.cc @@ -1,6 +1,6 @@ /* - * $Id: http.cc,v 1.508 2006/10/01 17:26:34 adrian Exp $ + * $Id: http.cc,v 1.509 2006/10/31 23:30:57 wessels Exp $ * * DEBUG: section 11 Hypertext Transfer Protocol (HTTP) * AUTHOR: Harvest Derived @@ -1984,8 +1984,11 @@ HttpStateData::icapAclCheckDone(ICAPServiceRep::Pointer service) { icapAccessCheckPending = false; - if (service == NULL) { - // handle case where no service is selected; + const bool startedIcap = startIcap(service); + + if (!startedIcap && (!service || service->bypass)) { + // handle ICAP start failure when no service was selected + // or where the selected service was optional entry->replaceHttpReply(reply); haveParsedReplyHeaders(); @@ -1997,11 +2000,8 @@ HttpStateData::icapAclCheckDone(ICAPServiceRep::Pointer service) return; } - if (doIcap(service) < 0) { - /* - * XXX Maybe instead of an error page we should - * handle the reply normally (without ICAP). - */ + if (!startedIcap) { + // handle start failure for an essential ICAP service ErrorState *err = errorCon(ERR_ICAP_FAILURE, HTTP_INTERNAL_SERVER_ERROR, orig_request); err->xerrno = errno; errorAppendEntry(entry, err); @@ -2023,15 +2023,15 @@ HttpStateData::icapSpaceAvailable() maybeReadData(); } -void +bool HttpStateData::takeAdaptedHeaders(HttpReply *rep) { debug(11,5)("HttpStateData::takeAdaptedHeaders() called\n"); if (!entry->isAccepting()) { debug(11,5)("\toops, entry is not Accepting!\n"); - icap->ownerAbort(); - return; + backstabAdapter(); + return false; } assert (rep); @@ -2043,9 +2043,10 @@ HttpStateData::takeAdaptedHeaders(HttpReply *rep) haveParsedReplyHeaders(); debug(11,5)("HttpStateData::takeAdaptedHeaders() finished\n"); + return true; } -void +bool HttpStateData::takeAdaptedBody(MemBuf *buf) { debug(11,5)("HttpStateData::takeAdaptedBody() called\n"); @@ -2054,50 +2055,38 @@ HttpStateData::takeAdaptedBody(MemBuf *buf) if (!entry->isAccepting()) { debug(11,5)("\toops, entry is not Accepting!\n"); - icap->ownerAbort(); - return; + backstabAdapter(); + return false; } entry->write(StoreIOBuffer(buf, currentOffset)); // write everything currentOffset += buf->contentSize(); buf->consume(buf->contentSize()); // consume everything written + return true; } +// called when ICAP adaptation is about to finish successfully, destroys icap +// must be called by the ICAP code void -HttpStateData::doneAdapting() +HttpStateData::finishAdapting() { - debug(11,5)("HttpStateData::doneAdapting() called\n"); + debug(11,5)("HttpStateData::finishAdapting() called by %p\n", icap); - if (!entry->isAccepting()) { + if (!entry->isAccepting()) { // XXX: do we need this check here? debug(11,5)("\toops, entry is not Accepting!\n"); - icap->ownerAbort(); + backstabAdapter(); } else { fwd->complete(); + endAdapting(); } - - /* - * ICAP is done, so we don't need this any more. - */ - delete icap; - - cbdataReferenceDone(icap); - - if (fd >= 0) - comm_close(fd); - else - httpStateFree(fd, this); } +// called when there was an ICAP error, destroys icap +// must be called by the ICAP code void HttpStateData::abortAdapting() { - debug(11,5)("HttpStateData::abortAdapting() called\n"); - - /* - * ICAP has given up, we're done with it too - */ - delete icap; - cbdataReferenceDone(icap); + debug(11,5)("HttpStateData::abortAdapting() called by %p\n", icap); if (entry->isEmpty()) { ErrorState *err; @@ -2108,11 +2097,32 @@ HttpStateData::abortAdapting() flags.do_next_read = 0; } - if (fd >= 0) { + endAdapting(); +} + +// internal helper to terminate adotation when called by the adapter +void +HttpStateData::backstabAdapter() +{ + debug(11,5)("HttpStateData::backstabAdapter() called for %p\n", icap); + assert(icap); + icap->ownerAbort(); + endAdapting(); +} + +// internal helper to delete icap and close the HTTP connection +void +HttpStateData::endAdapting() +{ + debug(11,5)("HttpStateData::endAdapting() called, deleting %p\n", icap); + + delete icap; + icap = NULL; + + if (fd >= 0) comm_close(fd); - } else { - httpStateFree(-1, this); // deletes this - } + else + httpStateFree(fd, this); // deletes us } #endif diff --git a/src/http.h b/src/http.h index cf1fd726b9..fcd3c09dc7 100644 --- a/src/http.h +++ b/src/http.h @@ -1,6 +1,6 @@ /* - * $Id: http.h,v 1.25 2006/09/19 07:56:57 adrian Exp $ + * $Id: http.h,v 1.26 2006/10/31 23:30:57 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -71,13 +71,13 @@ public: void readReply(size_t len, comm_err_t flag, int xerrno); void maybeReadData(); int cacheableReply(); -#if ICAP_CLIENT - void takeAdaptedHeaders(HttpReply *); - void takeAdaptedBody(MemBuf *); - void doneAdapting(); - void abortAdapting(); - void icapSpaceAvailable(); +#if ICAP_CLIENT + virtual bool takeAdaptedHeaders(HttpReply *); + virtual bool takeAdaptedBody(MemBuf *); + virtual void finishAdapting(); // deletes icap + virtual void abortAdapting(); // deletes icap + virtual void icapSpaceAvailable(); #endif peer *_peer; /* peer request made to */ @@ -134,6 +134,11 @@ private: http_state_flags flags); static bool decideIfWeDoRanges (HttpRequest * orig_request); +#if ICAP_CLIENT + void backstabAdapter(); + void endAdapting(); +#endif + private: CBDATA_CLASS2(HttpStateData); };