From: Alex Rousskov <rousskov@measurement-factory.com>
Date: Sat, 8 Jun 2013 23:21:23 +0000 (-0600)
Subject: Fix detection of concurrent ACLChecklist checks, avoiding !accessList asserts.
X-Git-Tag: SQUID_3_4_0_1~60
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7c469a686d688fceacc4eee8a2417b4c5e77e793;p=thirdparty%2Fsquid.git

Fix detection of concurrent ACLChecklist checks, avoiding !accessList asserts.

Concurrent checks are not supported, but it is possible for the same
ACLChecklist to be used for a sequence of checks, alternating fastCheck(void)
and fastCheck(list) calls. We needed a different/dedicated mechanism to detect
check concurrency (added ACLChecklist::occupied_), and we needed to preserve
(and then restore) pre-set accessList during fastCheck(list) checks.
---

diff --git a/src/acl/Checklist.cc b/src/acl/Checklist.cc
index 9d4ccfb201..4a8e246aa2 100644
--- a/src/acl/Checklist.cc
+++ b/src/acl/Checklist.cc
@@ -60,6 +60,11 @@ void
 ACLChecklist::preCheck(const char *what)
 {
     debugs(28, 3, HERE << this << " checking " << what);
+
+    // concurrent checks using the same Checklist are not supported
+    assert(!occupied_);
+    occupied_ = true;
+
     AclMatchedName = NULL;
     finished_ = false;
 }
@@ -148,6 +153,9 @@ ACLChecklist::checkCallback(allow_t answer)
     if (cbdataReferenceValidDone(callback_data, &cbdata_))
         callback_(answer, cbdata_);
 
+    // not really meaningful just before delete, but here for completeness sake
+    occupied_ = false;
+
     delete this;
 }
 
@@ -156,6 +164,7 @@ ACLChecklist::ACLChecklist() :
         callback (NULL),
         callback_data (NULL),
         asyncCaller_(false),
+        occupied_(false),
         finished_(false),
         allow_(ACCESS_DENIED),
         asyncStage_(asyncNone),
@@ -287,10 +296,10 @@ ACLChecklist::fastCheck(const Acl::Tree * list)
     preCheck("fast ACLs");
     asyncCaller_ = false;
 
-    // This call is not compatible with a pre-set accessList because we cannot
-    // tell whether this Checklist is used by some other concurent call, which
-    // is not supported.
-    assert(!accessList);
+    // Concurrent checks are not supported, but sequential checks are, and they
+    // may use a mixture of fastCheck(void) and fastCheck(list) calls.
+    const Acl::Tree * const savedList = accessList;
+
     accessList = cbdataReference(list);
 
     // assume DENY/ALLOW on mis/matches due to action-free accessList
@@ -301,6 +310,8 @@ ACLChecklist::fastCheck(const Acl::Tree * list)
         markFinished(ACCESS_DENIED, "ACLs failed to match");
 
     cbdataReferenceDone(accessList);
+    accessList = savedList;
+    occupied_ = false;
     PROF_stop(aclCheckFast);
     return currentAnswer();
 }
@@ -324,6 +335,7 @@ ACLChecklist::fastCheck()
         // if finished (on a match or in exceptional cases), stop
         if (finished()) {
             cbdataReferenceDone(acl);
+            occupied_ = false;
             PROF_stop(aclCheckFast);
             return currentAnswer();
         }
@@ -334,6 +346,7 @@ ACLChecklist::fastCheck()
     // There were no rules to match or no rules matched
     calcImplicitAnswer();
     cbdataReferenceDone(acl);
+    occupied_ = false;
     PROF_stop(aclCheckFast);
 
     return currentAnswer();
diff --git a/src/acl/Checklist.h b/src/acl/Checklist.h
index c94e4eb826..f26560d158 100644
--- a/src/acl/Checklist.h
+++ b/src/acl/Checklist.h
@@ -224,6 +224,7 @@ private: /* internal methods */
     void calcImplicitAnswer();
 
     bool asyncCaller_; ///< whether the caller supports async/slow ACLs
+    bool occupied_; ///< whether a check (fast or non-blocking) is in progress
     bool finished_;
     allow_t allow_;