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 9d4ccfb201c5d20cbcd4de29cc8c96550ad4ea3d..4a8e246aa25010b0f523d834fdac0c05c90acf81 100644 (file)
--- 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 c94e4eb826b7a56f2773c2245e70a44f4860a790..f26560d158879a023e0ca96cc217e335cdd9ad1e 100644 (file)
--- 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_;