From: wessels <> Date: Fri, 9 Dec 2005 03:08:46 +0000 (+0000) Subject: Fixed a bug and some confusion with ACLChecklist::fastCheck() X-Git-Tag: SQUID_3_0_PRE4~463 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=108d65b2f8eddf61ed1c2663a5b90e03b142ee02;p=thirdparty%2Fsquid.git Fixed a bug and some confusion with ACLChecklist::fastCheck() - Added cbdataReferenceDone(accessList) call to fastCheck() in the "if finished()" block. - peerAllowedToUse() was setting accessList = NULL after calling fastCheck(). Since neither fastCheck() nor peerAllowedToUse() were calling cbdataReferenceDone(), the cbdata lock_count would never decrease and eventually hit an assertion. - In general, the fastCheck() callers do not need to call cbdataReferenceDone, nor set accessList to NULL. The cbdataReferenceDone() call should always happen in fastCheck(). If not, it will happen in the ACLCheckList destructor. - It might be even better if fastCheck() called cbdataReference() as well, to avoid this situation where locking happens in one place and unlocking in another. This is future work, however. --- diff --git a/src/ACLChecklist.cc b/src/ACLChecklist.cc index 339b58236f..c177f17445 100644 --- a/src/ACLChecklist.cc +++ b/src/ACLChecklist.cc @@ -1,5 +1,5 @@ /* - * $Id: ACLChecklist.cc,v 1.28 2005/10/23 11:55:31 hno Exp $ + * $Id: ACLChecklist.cc,v 1.29 2005/12/08 20:08:46 wessels Exp $ * * DEBUG: section 28 Access Control * AUTHOR: Duane Wessels @@ -409,6 +409,7 @@ ACLChecklist::fastCheck() if (finished()) { PROF_stop(aclCheckFast); + cbdataReferenceDone(accessList); return currentAnswer() == ACCESS_ALLOWED; } diff --git a/src/DelayId.cc b/src/DelayId.cc index d965d6ce4b..2ed9457abe 100644 --- a/src/DelayId.cc +++ b/src/DelayId.cc @@ -1,6 +1,6 @@ /* - * $Id: DelayId.cc,v 1.17 2005/09/09 17:31:33 wessels Exp $ + * $Id: DelayId.cc,v 1.18 2005/12/08 20:08:46 wessels Exp $ * * DEBUG: section 77 Delay Pools * AUTHOR: Robert Collins @@ -116,9 +116,10 @@ DelayId::DelayClient(ClientHttpRequest * http) ch.accessList = cbdataReference(DelayPools::delay_data[pool].access); + /* cbdataReferenceDone() happens in either fastCheck() or ~ACLCheckList */ + if (DelayPools::delay_data[pool].theComposite().getRaw() && ch.fastCheck()) { - ch.accessList = NULL; DelayId result (pool + 1); CompositePoolNode::CompositeSelectionDetails details; details.src_addr = ch.src_addr; @@ -127,8 +128,6 @@ DelayId::DelayClient(ClientHttpRequest * http) result.compositePosition(DelayPools::delay_data[pool].theComposite()->id(details)); return result; } - - ch.accessList = NULL; } diff --git a/src/cbdata.cc b/src/cbdata.cc index 4c7fc8023a..6a0ddabff0 100644 --- a/src/cbdata.cc +++ b/src/cbdata.cc @@ -1,6 +1,6 @@ /* - * $Id: cbdata.cc,v 1.65 2005/11/05 00:08:32 wessels Exp $ + * $Id: cbdata.cc,v 1.66 2005/12/08 20:08:46 wessels Exp $ * * DEBUG: section 45 Callback Data Registry * ORIGINAL AUTHOR: Duane Wessels @@ -364,7 +364,7 @@ cbdataInternalLock(const void *p) c->check(__LINE__); - assert(c->locks < 65535); + assert(c->locks < 655); c->locks++; } diff --git a/src/client_side.cc b/src/client_side.cc index 2599fb746d..40ca91652c 100644 --- a/src/client_side.cc +++ b/src/client_side.cc @@ -1,6 +1,6 @@ /* - * $Id: client_side.cc,v 1.705 2005/12/06 23:03:34 wessels Exp $ + * $Id: client_side.cc,v 1.706 2005/12/08 20:08:46 wessels Exp $ * * DEBUG: section 33 Client-side Routines * AUTHOR: Duane Wessels @@ -2869,12 +2869,11 @@ httpAccept(int sock, int newfd, ConnectionDetail *details, identChecklist.accessList = cbdataReference(Config.accessList.identLookup); + /* cbdataReferenceDone() happens in either fastCheck() or ~ACLCheckList */ + if (identChecklist.fastCheck()) identStart(&details->me, &details->peer, clientIdentDone, connState); - cbdataReferenceDone(identChecklist.accessList); - - identChecklist.accessList = NULL; #endif @@ -3052,11 +3051,11 @@ httpsAccept(int sock, int newfd, ConnectionDetail *details, identChecklist.accessList = cbdataReference(Config.accessList.identLookup); + /* cbdataReferenceDone() happens in either fastCheck() or ~ACLCheckList */ + if (identChecklist.fastCheck()) identStart(&details->me, &details->peer, clientIdentDone, connState); - identChecklist.accessList = NULL; - #endif commSetSelect(newfd, COMM_SELECT_READ, clientNegotiateSSL, connState, 0); diff --git a/src/forward.cc b/src/forward.cc index 4549eaf10d..28e22f2c7d 100644 --- a/src/forward.cc +++ b/src/forward.cc @@ -1,6 +1,6 @@ /* - * $Id: forward.cc,v 1.131 2005/12/06 23:03:34 wessels Exp $ + * $Id: forward.cc,v 1.132 2005/12/08 20:08:46 wessels Exp $ * * DEBUG: section 17 Request Forwarding * AUTHOR: Duane Wessels @@ -876,8 +876,8 @@ fwdStart(int fd, StoreEntry * e, HttpRequest * r) ch.my_port = r->my_port; ch.request = requestLink(r); ch.accessList = cbdataReference(Config.accessList.miss); + /* cbdataReferenceDone() happens in either fastCheck() or ~ACLCheckList */ answer = ch.fastCheck(); - ch.accessList = NULL; if (answer == 0) { err_type page_id; diff --git a/src/http.cc b/src/http.cc index 86173e440d..85ed132df1 100644 --- a/src/http.cc +++ b/src/http.cc @@ -1,6 +1,6 @@ /* - * $Id: http.cc,v 1.470 2005/12/06 23:03:34 wessels Exp $ + * $Id: http.cc,v 1.471 2005/12/08 20:08:46 wessels Exp $ * * DEBUG: section 11 Hypertext Transfer Protocol (HTTP) * AUTHOR: Harvest Derived @@ -1907,6 +1907,8 @@ httpSendRequestEntityDone(int fd, void *data) if (Config.accessList.brokenPosts) ch.accessList = cbdataReference(Config.accessList.brokenPosts); + /* cbdataReferenceDone() happens in either fastCheck() or ~ACLCheckList */ + if (!Config.accessList.brokenPosts) { debug(11, 5) ("httpSendRequestEntityDone: No brokenPosts list\n"); HttpStateData::SendComplete(fd, NULL, 0, COMM_OK, data); @@ -1917,8 +1919,6 @@ httpSendRequestEntityDone(int fd, void *data) debug(11, 2) ("httpSendRequestEntityDone: matched brokenPosts\n"); comm_old_write(fd, "\r\n", 2, HttpStateData::SendComplete, data, NULL); } - - ch.accessList = NULL; } static void diff --git a/src/icp_v2.cc b/src/icp_v2.cc index f6076224a2..29407dfe21 100644 --- a/src/icp_v2.cc +++ b/src/icp_v2.cc @@ -1,6 +1,6 @@ /* - * $Id: icp_v2.cc,v 1.87 2005/06/09 16:04:30 serassio Exp $ + * $Id: icp_v2.cc,v 1.88 2005/12/08 20:08:47 wessels Exp $ * * DEBUG: section 12 Internet Cache Protocol * AUTHOR: Duane Wessels @@ -399,8 +399,8 @@ icpAccessAllowed(struct sockaddr_in *from, HttpRequest * icp_request) checklist.my_addr = no_addr; checklist.request = requestLink(icp_request); checklist.accessList = cbdataReference(Config.accessList.icp); + /* cbdataReferenceDone() happens in either fastCheck() or ~ACLCheckList */ int result = checklist.fastCheck(); - checklist.accessList = NULL; return result; } diff --git a/src/neighbors.cc b/src/neighbors.cc index 1bc96f104f..c40bd44063 100644 --- a/src/neighbors.cc +++ b/src/neighbors.cc @@ -1,6 +1,6 @@ /* - * $Id: neighbors.cc,v 1.331 2005/06/09 16:04:30 serassio Exp $ + * $Id: neighbors.cc,v 1.332 2005/12/08 20:08:47 wessels Exp $ * * DEBUG: section 15 Neighbor Routines * AUTHOR: Harvest Derived @@ -187,6 +187,8 @@ peerAllowedToUse(const peer * p, HttpRequest * request) checklist.accessList = cbdataReference(p->access); + /* cbdataReferenceDone() happens in either fastCheck() or ~ACLCheckList */ + #if 0 && USE_IDENT /* * this is currently broken because 'request->user_ident' has been @@ -198,11 +200,7 @@ peerAllowedToUse(const peer * p, HttpRequest * request) #endif - int result = checklist.fastCheck(); - - checklist.accessList = NULL; - - return result; + return checklist.fastCheck(); } /* Return TRUE if it is okay to send an ICP request to this peer. */ diff --git a/src/snmp_core.cc b/src/snmp_core.cc index a5f6c33dec..c744ffd068 100644 --- a/src/snmp_core.cc +++ b/src/snmp_core.cc @@ -1,6 +1,6 @@ /* - * $Id: snmp_core.cc,v 1.70 2005/12/08 18:33:55 wessels Exp $ + * $Id: snmp_core.cc,v 1.71 2005/12/08 20:08:47 wessels Exp $ * * DEBUG: section 49 SNMP support * AUTHOR: Glenn Chisholm @@ -536,14 +536,14 @@ snmpDecodePacket(snmp_request_t * rq) PDU = snmp_pdu_create(0); rq->session.Version = SNMP_VERSION_1; Community = snmp_parse(&rq->session, PDU, buf, len); - ACLChecklist checklist; checklist.src_addr = rq->from.sin_addr; checklist.snmp_community = (char *) Community; if (Community) { + ACLChecklist checklist; checklist.accessList = cbdataReference(Config.accessList.snmp); + /* cbdataReferenceDone() happens in either fastCheck() or ~ACLCheckList */ allow = checklist.fastCheck(); - checklist.accessList = NULL; } if ((snmp_coexist_V2toV1(PDU)) && (Community) && (allow)) { diff --git a/src/tunnel.cc b/src/tunnel.cc index 3a1414f351..ad29cd09e0 100644 --- a/src/tunnel.cc +++ b/src/tunnel.cc @@ -1,6 +1,6 @@ /* - * $Id: tunnel.cc,v 1.155 2005/12/01 21:35:40 serassio Exp $ + * $Id: tunnel.cc,v 1.156 2005/12/08 20:08:47 wessels Exp $ * * DEBUG: section 26 Secure Sockets Layer Proxy * AUTHOR: Duane Wessels @@ -604,8 +604,8 @@ sslStart(ClientHttpRequest * http, size_t * size_ptr, int *status_ptr) ch.my_port = request->my_port; ch.request = requestLink(request); ch.accessList = cbdataReference(Config.accessList.miss); + /* cbdataReferenceDone() happens in either fastCheck() or ~ACLCheckList */ answer = ch.fastCheck(); - ch.accessList = NULL; if (answer == 0) { err = errorCon(ERR_FORWARDING_DENIED, HTTP_FORBIDDEN);