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 4a8e246aa25010b0f523d834fdac0c05c90acf81..39cc724dccb4dcc2406f6c6ed731da96cb0175bf 100644 (file)
--- 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 f26560d158879a023e0ca96cc217e335cdd9ad1e..c63b1f301e7b6d583d6a61e60d309f6c0f9beacc 100644 (file)
--- 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();