--------------------- PatchSet 7548 Date: 2008/04/02 22:48:23 Author: chtsanti Branch: async-calls Tag: (none) Log: In the case the SSL and ICAP are enable the ClientHttpRequest is an AsyncJob object. In this case the httpRequestFree function, which exists in client_side.cc file and called in various places, can destroy an ClientHttpRequest object while we are in Call, causing bad crashes to squid. This patch replaces the httpRequestFree function the an ClientHttpRequest::httpRequestFree method which uses the safe for AsyncCalls AsyncJob::deleteThis() method, to delete ClientHttpRequest objects. Also uses cbdataLocks/Unlocks for the ClientHttpRequest objects anywhere it is required. Members: src/ESIInclude.cc:1.14.4.2->1.14.4.3 src/client_side.cc:1.139.4.11->1.139.4.12 src/client_side_request.cc:1.79.4.13->1.79.4.14 src/client_side_request.h:1.30.4.10->1.30.4.11 Index: squid3/src/ESIInclude.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ESIInclude.cc,v retrieving revision 1.14.4.2 retrieving revision 1.14.4.3 diff -u -r1.14.4.2 -r1.14.4.3 --- squid3/src/ESIInclude.cc 7 Feb 2008 17:43:23 -0000 1.14.4.2 +++ squid3/src/ESIInclude.cc 2 Apr 2008 22:48:23 -0000 1.14.4.3 @@ -1,6 +1,6 @@ /* - * $Id: ESIInclude.cc,v 1.14.4.2 2008/02/07 17:43:23 chtsanti Exp $ + * $Id: ESIInclude.cc,v 1.14.4.3 2008/04/02 22:48:23 chtsanti Exp $ * * DEBUG: section 86 ESI processing * AUTHOR: Robert Collins @@ -110,7 +110,8 @@ rep = NULL; esiStream->include->fail (esiStream); esiStream->finished = 1; - httpRequestFree (http); + http->httpRequestFree (); + cbdataInternalUnlock(http); return; } @@ -157,7 +158,8 @@ debugs(86, 5, "Finished reading upstream data in subrequest"); esiStream->include->subRequestDone (esiStream, true); esiStream->finished = 1; - httpRequestFree (http); + http->httpRequestFree (); + cbdataInternalUnlock(http); return; } @@ -181,14 +183,16 @@ debugs(86, 3, "ESI subrequest finished OK"); esiStream->include->subRequestDone (esiStream, true); esiStream->finished = 1; - httpRequestFree (http); + http->httpRequestFree (); + cbdataInternalUnlock(http); return; case STREAM_FAILED: debugs(86, 1, "ESI subrequest failed transfer"); esiStream->include->fail (esiStream); esiStream->finished = 1; - httpRequestFree (http); + http->httpRequestFree (); + cbdataInternalUnlock(http); return; case STREAM_NONE: { Index: squid3/src/client_side.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/client_side.cc,v retrieving revision 1.139.4.11 retrieving revision 1.139.4.12 diff -u -r1.139.4.11 -r1.139.4.12 --- squid3/src/client_side.cc 19 Mar 2008 22:01:10 -0000 1.139.4.11 +++ squid3/src/client_side.cc 2 Apr 2008 22:48:23 -0000 1.139.4.12 @@ -1,6 +1,6 @@ /* - * $Id: client_side.cc,v 1.139.4.11 2008/03/19 22:01:10 chtsanti Exp $ + * $Id: client_side.cc,v 1.139.4.12 2008/04/02 22:48:23 chtsanti Exp $ * * DEBUG: section 33 Client-side Routines * AUTHOR: Duane Wessels @@ -242,8 +242,9 @@ if (connRegistered_) deRegisterWithConn(); - - httpRequestFree(http); + + http->httpRequestFree(); + cbdataReferenceDone(http); /* clean up connection links to us */ assert(this != next.getRaw()); @@ -295,7 +296,7 @@ ClientSocketContext *newContext; assert(http != NULL); newContext = new ClientSocketContext; - newContext->http = http; + newContext->http = cbdataReference(http); return newContext; } @@ -548,13 +549,13 @@ clientStreamAbort((clientStreamNode *)client_stream.tail->data, this); } -void +/*void httpRequestFree(void *data) { ClientHttpRequest *http = (ClientHttpRequest *)data; assert(http != NULL); delete http; -} +}*/ bool ConnStateData::areAllContextsForThisConnection() const Index: squid3/src/client_side_request.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/client_side_request.cc,v retrieving revision 1.79.4.13 retrieving revision 1.79.4.14 diff -u -r1.79.4.13 -r1.79.4.14 --- squid3/src/client_side_request.cc 20 Mar 2008 21:22:46 -0000 1.79.4.13 +++ squid3/src/client_side_request.cc 2 Apr 2008 22:48:23 -0000 1.79.4.14 @@ -1,6 +1,6 @@ /* - * $Id: client_side_request.cc,v 1.79.4.13 2008/03/20 21:22:46 chtsanti Exp $ + * $Id: client_side_request.cc,v 1.79.4.14 2008/04/02 22:48:23 chtsanti Exp $ * * DEBUG: section 85 Client-side Request Routines * AUTHOR: Robert Collins (Originally Duane Wessels in client_side.c) @@ -294,6 +294,8 @@ http->req_sz = 0; tempBuffer.length = taillen; tempBuffer.data = tailbuf; + /*http should be unLocked/released by the caller module (ESI module for example)*/ + cbdataInternalLock(http); /* client stream setup */ clientStreamInit(&http->client_stream, clientGetMoreData, clientReplyDetach, clientReplyStatus, new clientReplyContext(http), streamcallback, Index: squid3/src/client_side_request.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/client_side_request.h,v retrieving revision 1.30.4.10 retrieving revision 1.30.4.11 diff -u -r1.30.4.10 -r1.30.4.11 --- squid3/src/client_side_request.h 20 Mar 2008 21:22:45 -0000 1.30.4.10 +++ squid3/src/client_side_request.h 2 Apr 2008 22:48:23 -0000 1.30.4.11 @@ -1,6 +1,6 @@ /* - * $Id: client_side_request.h,v 1.30.4.10 2008/03/20 21:22:45 chtsanti Exp $ + * $Id: client_side_request.h,v 1.30.4.11 2008/04/02 22:48:23 chtsanti Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -152,10 +152,13 @@ ClientRequestContext *calloutContext; void doCallouts(); -#if ICAP_CLIENT +#if ICAP_CLIENT || USE_SSL //AsyncJob virtual methods virtual bool doneAll() const { return ICAPInitiator::doneAll() && BodyConsumer::doneAll() && false;} + void httpRequestFree() { deleteThis("httpRequestFree"); } +#else + void httpRequestFree() { delete this; } #endif private: