From 9af1f3b732f9e0ef2192134a948ffc1275589575 Mon Sep 17 00:00:00 2001
From: Amos Jeffries <squid3@treenet.co.nz>
Date: Tue, 21 Jan 2014 20:29:15 -0700
Subject: [PATCH] Fix external_acl_type async loop failures

When external_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 helper 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 tracking down the issue and testing solutions.
---
 src/acl/Checklist.cc | 11 +++++++++--
 src/acl/Checklist.h  |  1 +
 2 files changed, 10 insertions(+), 2 deletions(-)

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();
 
-- 
2.47.2