From: wessels <> Date: Thu, 12 Jan 2006 05:40:39 +0000 (+0000) Subject: Management of {adapated,virgin}->data->header was becoming a problem. X-Git-Tag: SQUID_3_0_PRE4~368 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=d9eb908283ce682b1963942b0778127cb88d4285;p=thirdparty%2Fsquid.git Management of {adapated,virgin}->data->header was becoming a problem. One side might delete the header before the other side was done with it. This patch moves deletion of ->data->header to MsgPipeData class so that it goes away only when the MsgPipe goes away. --- diff --git a/src/ICAP/ICAPClientReqmodPrecache.cc b/src/ICAP/ICAPClientReqmodPrecache.cc index ae91578eca..fd81eef476 100644 --- a/src/ICAP/ICAPClientReqmodPrecache.cc +++ b/src/ICAP/ICAPClientReqmodPrecache.cc @@ -39,7 +39,7 @@ void ICAPClientReqmodPrecache::startReqMod(ClientHttpRequest *aHttp, HttpRequest virgin->source = this; virgin->data = new MsgPipeData; virgin->data->cause = NULL; - virgin->data->header = requestLink(request); + virgin->data->setHeader(request); virgin->data->body = new MemBuf; virgin->data->body->init(ICAP::MsgPipeBufSizeMin, ICAP::MsgPipeBufSizeMax); @@ -182,21 +182,10 @@ void ICAPClientReqmodPrecache::stop(Notify notify) void ICAPClientReqmodPrecache::freeVirgin() { // virgin->data->cause should be NULL; - - if (virgin->data->header) { - requestUnlink(dynamic_cast(virgin->data->header)); - virgin->data->header = NULL; - } - virgin = NULL; // refcounted } void ICAPClientReqmodPrecache::freeAdapted() { - if (adapted->data->header) { - requestUnlink(dynamic_cast(adapted->data->header)); - adapted->data->header = NULL; - } - adapted = NULL; // refcounted } diff --git a/src/ICAP/ICAPClientRespmodPrecache.cc b/src/ICAP/ICAPClientRespmodPrecache.cc index f3d8518339..c2e3b0ed7b 100644 --- a/src/ICAP/ICAPClientRespmodPrecache.cc +++ b/src/ICAP/ICAPClientRespmodPrecache.cc @@ -39,8 +39,8 @@ void ICAPClientRespmodPrecache::startRespMod(HttpStateData *anHttpState, HttpReq virgin = new MsgPipe("virgin"); // this is the place to create a refcount ptr virgin->source = this; virgin->data = new MsgPipeData; - virgin->data->cause = requestLink(request); - virgin->data->header = reply; + virgin->data->setCause(request); + virgin->data->setHeader(reply); virgin->data->body = new MemBuf; virgin->data->body->init(ICAP::MsgPipeBufSizeMin, ICAP::MsgPipeBufSizeMax); @@ -49,7 +49,7 @@ void ICAPClientRespmodPrecache::startRespMod(HttpStateData *anHttpState, HttpReq #if ICAP_ANCHOR_LOOPBACK adapted->data = new MsgPipeData; - adapted->data->cause = request; // should not hurt + adapted->data->setCause(request); // should not hurt #else ICAPInitXaction(service, virgin, adapted); @@ -88,7 +88,7 @@ void ICAPClientRespmodPrecache::doneSending() #if ICAP_ANCHOR_LOOPBACK /* simple assignments are not the right way to do this */ - adapted->data->header = virgin->data->header; + adapted->data->setHeader(virgin->data->header); adapted->data->body = virgin->data->body; noteSourceFinish(adapted); return; @@ -125,11 +125,7 @@ void ICAPClientRespmodPrecache::noteSinkAbort(MsgPipe *p) // ICAP client has received new HTTP headers (if any) at this point void ICAPClientRespmodPrecache::noteSourceStart(MsgPipe *p) { - debug(93,5)("ICAPClientRespmodPrecache::noteSourceStart() called\n"); - - /* - * May want to assert that adapted != NULL here - */ + debugs(93,5, HERE << "ICAPClientRespmodPrecache::noteSourceStart() called"); HttpReply *reply = dynamic_cast(adapted->data->header); /* @@ -223,26 +219,10 @@ void ICAPClientRespmodPrecache::stop(Notify notify) void ICAPClientRespmodPrecache::freeVirgin() { - requestUnlink(virgin->data->cause); - virgin->data->cause = NULL; - virgin->data->header = NULL; virgin = NULL; // refcounted } void ICAPClientRespmodPrecache::freeAdapted() { - /* - * Note on adapted->data->header. ICAPXaction-side created it - * but gave control of it to us. Normally we give it to - * HttpStateData::takeAdaptedHeader. If not, we have to - * make sure it gets deleted; - */ - - if (adapted->data->header != NULL) { - debug(93,3)("hey, adapted->data->header is still set!\n"); - delete adapted->data->header; - adapted->data->header = NULL; - } - adapted = NULL; // refcounted } diff --git a/src/ICAP/ICAPModXact.cc b/src/ICAP/ICAPModXact.cc index c1382a991c..d5f22a9241 100644 --- a/src/ICAP/ICAPModXact.cc +++ b/src/ICAP/ICAPModXact.cc @@ -500,15 +500,6 @@ void ICAPModXact::stopSending(bool nicely) state.sending = State::sendingDone; - /* - * adapted->data->header should be a link_count'ed HttpRequest - */ - - if (adapted->data->header) { - requestUnlink(dynamic_cast(adapted->data->header)); - adapted->data->header = NULL; - } - adapted = NULL; // refcounted } @@ -550,9 +541,9 @@ void ICAPModXact::maybeAllocateHttpMsg() return; if (gotEncapsulated("res-hdr")) { - adapted->data->header = new HttpReply; + adapted->data->setHeader(new HttpReply); } else if (gotEncapsulated("req-hdr")) { - adapted->data->header = requestLink(new HttpRequest); + adapted->data->setHeader(new HttpRequest); } else throw TexcHere("Neither res-hdr nor req-hdr in maybeAllocateHttpMsg()"); } diff --git a/src/ICAP/MsgPipeData.h b/src/ICAP/MsgPipeData.h index d4949d86cc..99d4fee826 100644 --- a/src/ICAP/MsgPipeData.h +++ b/src/ICAP/MsgPipeData.h @@ -1,6 +1,6 @@ /* - * $Id: MsgPipeData.h,v 1.3 2005/12/22 22:26:31 wessels Exp $ + * $Id: MsgPipeData.h,v 1.4 2006/01/11 22:40:39 wessels Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -35,6 +35,8 @@ #define SQUID_MSGPIPEDATA_H #include "HttpMsg.h" +#include "HttpRequest.h" +#include "HttpReply.h" #include "MemBuf.h" // MsgPipeData contains information about the HTTP message being sent @@ -52,8 +54,8 @@ public: ~MsgPipeData() { - assert(NULL == cause); - assert(NULL == header); + clearCause(); + clearHeader(); if (body) { body->clean(); @@ -61,6 +63,21 @@ public: } }; + void setCause(HttpRequest *r) + { + cause = requestLink(r); + }; + + void setHeader(HttpMsg *msg) + { + clearHeader(); + + if (HttpRequest *req = dynamic_cast(msg)) + header = requestLink(req); + else if (HttpReply *rep = dynamic_cast(msg)) + header = rep; + }; + public: typedef HttpMsg Header; typedef MemBuf Body; @@ -71,6 +88,23 @@ public: // HTTP request header for piped responses (the cause of the response) HttpRequest *cause; + +private: + + void clearCause() + { + requestUnlink(cause); + cause = NULL; + }; + + void clearHeader() + { + if (HttpRequest *req = dynamic_cast(header)) + requestUnlink(req); + + header = NULL; + }; + }; #endif /* SQUID_MSGPIPEDATA_H */