From: Amos Jeffries <squid3@treenet.co.nz>
Date: Fri, 17 Jan 2014 11:44:26 +0000 (-0800)
Subject: Fix external_acl_type async loop failures
X-Git-Tag: SQUID_3_5_0_1~408
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=1707b9ad8af903902da6295250f5d80bc22b6c14;p=thirdparty%2Fsquid.git

Fix external_acl_type async loop failures

When externa_acl_type uses %LOGIN and is required to trigger async
authentication lookups it returns and hits the async loop prevention
check when starting to trigger its own external helper lookup. This
results in a DUNNO output from the helper as final status instead of
the real helepr lookup result.

Avoid these by allowing async helpers to loop several times before
aborting the lookups.

Also, extend debug message to indicate loop count.

Thanks to Peter Benko for trackign down the issue and testing solutions.
---

diff --git a/src/acl/Checklist.cc b/src/acl/Checklist.cc
index 4a8e246aa2..39cc724dcc 100644
--- a/src/acl/Checklist.cc
+++ b/src/acl/Checklist.cc
@@ -64,6 +64,7 @@ ACLChecklist::preCheck(const char *what)
     // concurrent checks using the same Checklist are not supported
     assert(!occupied_);
     occupied_ = true;
+    asyncLoopDepth_ = 0;
 
     AclMatchedName = NULL;
     finished_ = false;
@@ -77,6 +78,7 @@ ACLChecklist::matchChild(const Acl::InnerNode *current, Acl::Nodes::const_iterat
     // Remember the current tree location to prevent "async loop" cases where
     // the same child node wants to go async more than once.
     matchLoc_ = Breadcrumb(current, pos);
+    asyncLoopDepth_ = 0;
 
     // if there are any breadcrumbs left, then follow them on the way down
     bool result = false;
@@ -116,11 +118,16 @@ ACLChecklist::goAsync(AsyncState *state)
 
     // TODO: add a once-in-a-while WARNING about async loops?
     if (matchLoc_ == asyncLoc_) {
-        debugs(28, 2, this << " a slow ACL resumes by going async again!");
-        return false;
+        debugs(28, 2, this << " a slow ACL resumes by going async again! (loop #" << asyncLoopDepth_ << ")");
+        // external_acl_type may cause async auth lookup plus its own async check
+        // which has the appearance of a loop. Allow some retries.
+        // TODO: make it configurable and check BH retry attempts vs this check?
+        if (asyncLoopDepth_ > 5)
+            return false;
     }
 
     asyncLoc_ = matchLoc_; // prevent async loops
+    ++asyncLoopDepth_;
 
     asyncStage_ = asyncStarting;
     changeState(state);
diff --git a/src/acl/Checklist.h b/src/acl/Checklist.h
index f26560d158..c63b1f301e 100644
--- a/src/acl/Checklist.h
+++ b/src/acl/Checklist.h
@@ -233,6 +233,7 @@ private: /* internal methods */
     AsyncState *state_;
     Breadcrumb matchLoc_; ///< location of the node running matches() now
     Breadcrumb asyncLoc_; ///< currentNode_ that called goAsync()
+    unsigned asyncLoopDepth_; ///< how many times the current async state has resumed
 
     bool callerGone();