From: Eduard Bagdasaryan <eduard.bagdasaryan@measurement-factory.com>
Date: Wed, 15 May 2024 22:30:07 +0000 (+0000)
Subject: Ensure ACLChecklist::markFinished() is called (#1809)
X-Git-Tag: SQUID_7_0_1~122
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=96ab90cc6dcf08229694f85dca916d5f7fbfb09b;p=thirdparty%2Fsquid.git

Ensure ACLChecklist::markFinished() is called (#1809)

For example, ACLChecklist::resumeNonBlockingCheck() missed a
markFinished() call when prepNonBlocking() returned false after a
reconfiguration. Missing markFinished() calls result in the last
evaluated ACL name being unset in nonBlockingCheck() callbacks.
---

diff --git a/src/acl/Checklist.cc b/src/acl/Checklist.cc
index c04014500c..4dc5ad7e39 100644
--- a/src/acl/Checklist.cc
+++ b/src/acl/Checklist.cc
@@ -23,7 +23,7 @@ ACLChecklist::prepNonBlocking()
     assert(accessList);
 
     if (callerGone()) {
-        checkCallback(ACCESS_DUNNO); // the answer does not really matter
+        checkCallback("caller is gone"); // the answer does not really matter
         return false;
     }
 
@@ -34,8 +34,7 @@ ACLChecklist::prepNonBlocking()
 
     if (!cbdataReferenceValid(accessList)) {
         cbdataReferenceDone(accessList);
-        debugs(28, 4, "ACLChecklist::check: " << this << " accessList is invalid");
-        checkCallback(ACCESS_DUNNO);
+        checkCallback("accessList is invalid");
         return false;
     }
 
@@ -51,7 +50,7 @@ ACLChecklist::completeNonBlocking()
         calcImplicitAnswer();
 
     cbdataReferenceDone(accessList);
-    checkCallback(currentAnswer());
+    checkCallback(nullptr);
 }
 
 void
@@ -155,17 +154,20 @@ ACLChecklist::goAsync(AsyncStarter starter, const Acl::Node &acl)
 // ACLFilledChecklist overwrites this to unclock something before we
 // "delete this"
 void
-ACLChecklist::checkCallback(const Acl::Answer &answer)
+ACLChecklist::checkCallback(const char * const abortReason)
 {
+    if (abortReason)
+        markFinished(ACCESS_DUNNO, abortReason);
+    Assure(finished());
+
     ACLCB *callback_;
     void *cbdata_;
-    debugs(28, 3, "ACLChecklist::checkCallback: " << this << " answer=" << answer);
 
     callback_ = callback;
     callback = nullptr;
 
     if (cbdataReferenceValidDone(callback_data, &cbdata_))
-        callback_(answer, cbdata_);
+        callback_(currentAnswer(), cbdata_);
 
     // not really meaningful just before delete, but here for completeness sake
     occupied_ = false;
@@ -213,7 +215,7 @@ ACLChecklist::nonBlockingCheck(ACLCB * callback_, void *callback_data_)
      * We cannot select a sensible default for all callers here. */
     if (accessList == nullptr) {
         debugs(28, DBG_CRITICAL, "SECURITY ERROR: ACL " << this << " checked with nothing to match against!!");
-        checkCallback(ACCESS_DUNNO);
+        checkCallback("nonBlockingCheck() without accessList");
         return;
     }
 
diff --git a/src/acl/Checklist.h b/src/acl/Checklist.h
index 3f7fcd96bf..1089991694 100644
--- a/src/acl/Checklist.h
+++ b/src/acl/Checklist.h
@@ -159,7 +159,8 @@ public:
 
 private:
     /// Calls non-blocking check callback with the answer and destroys self.
-    void checkCallback(const Acl::Answer &answer);
+    /// If abortReason is provided, sets the final answer to ACCESS_DUNNO.
+    void checkCallback(const char *abortReason);
 
     void matchAndFinish();