Fixed the case where a service group disappears during a nb ACL check.

Replaced "done" member with an existing AsyncJob mustStop mechanism.

Removed extra async call as unneeded because ACL callbacks are already async.

=== modified file 'src/adaptation/AccessCheck.cc'
--- src/adaptation/AccessCheck.cc	2009-06-17 06:10:46 +0000
+++ src/adaptation/AccessCheck.cc	2009-06-27 21:08:56 +0000
@@ -39,8 +39,7 @@
     AsyncJob("AccessCheck"), filter(aFilter),
     callback(aCallback),
     callback_data(cbdataReference(aCallbackData)),
-    acl_checklist(NULL),
-    done(FALSE)
+    acl_checklist(NULL)
 {
 #if ICAP_CLIENT
     Adaptation::Icap::History::Pointer h = filter.request->icapHistory();
@@ -104,9 +103,9 @@
         candidates.shift(); // the rule apparently went away (reconfigure)
     }
 
-    // when there are no canidates, fake answer 1
     debugs(93, 4, HERE << "NO candidates left");
-    noteAnswer(1);
+    callBack(NULL);
+    Must(done());
 }
 
 void
@@ -122,56 +121,57 @@
     ac->noteAnswer(answer==ACCESS_ALLOWED);
 }
 
+/// process the results of the ACL check
 void
 Adaptation::AccessCheck::noteAnswer(int answer)
 {
-    debugs(93, 5, HERE << "AccessCheck::noteAnswer " << answer);
-    if (candidates.size())
-        debugs(93, 5, HERE << "was checking rule" << topCandidate());
+    Must(!candidates.empty()); // the candidate we were checking must be there
+    debugs(93,5, HERE << topCandidate() << " answer=" << answer);
 
-    if (!answer) {
-        candidates.shift(); // the rule did not match
-        checkCandidates();
-        return;
+    if (answer) { // the rule matched
+        ServiceGroupPointer g = topGroup();
+        if (g != NULL) { // the corresponding group found
+            callBack(g);
+            Must(done());
+            return;
+        }
     }
 
-    /*
-     * We use an async call here to break deep function call sequences
-     */
-    // XXX: use AsyncCall for callback and remove
-    CallJobHere(93, 5, this, Adaptation::AccessCheck::do_callback);
+    // no match or the group disappeared during reconfiguration
+    candidates.shift();
+    checkCandidates();
 }
 
+/// call back with a possibly nil group; the job ends here because all failures
+/// at this point are fatal to the access check process
 void
-Adaptation::AccessCheck::do_callback()
+Adaptation::AccessCheck::callBack(const ServiceGroupPointer &g)
 {
-    debugs(93, 3, HERE);
-
-    if (candidates.size())
-        debugs(93, 3, HERE << "was checking rule" << topCandidate());
+    debugs(93,3, HERE << g);
 
     void *validated_cbdata;
-    if (!cbdataReferenceValidDone(callback_data, &validated_cbdata)) {
-        debugs(93,3,HERE << "do_callback: callback_data became invalid, skipping");
-        return;
+    if (cbdataReferenceValidDone(callback_data, &validated_cbdata)) {
+        callback(g, validated_cbdata);
     }
+    mustStop("done"); // called back or will never be able to call back
+}
 
+Adaptation::ServiceGroupPointer
+Adaptation::AccessCheck::topGroup() const
+{
     ServiceGroupPointer g;
     if (candidates.size()) {
         if (AccessRule *r = FindRule(topCandidate())) {
             g = FindGroup(r->groupId);
-            debugs(93,3,HERE << "do_callback: " << r->id << " with " << g);
+            debugs(93,5, HERE << "top group for " << r->id << " is " << g);
         } else {
-            debugs(93,3,HERE << "do_callback: no rule" << topCandidate());
+            debugs(93,5, HERE << "no rule for " << topCandidate());
         }
-        // TODO: should we shift and checkCandidates if and only if (!g) ?!
-        candidates.shift(); // done with topCandidate()
     } else {
-        debugs(93,3,HERE << "do_callback: no candidate rules");
+        debugs(93,5, HERE << "no candidates"); // should not happen
     }
 
-    callback(g, validated_cbdata);
-    done = TRUE;
+    return g;
 }
 
 /** Returns true iff the rule's service group will be used after ACL matches.

=== modified file 'src/adaptation/AccessCheck.h'
--- src/adaptation/AccessCheck.h	2009-06-17 06:10:46 +0000
+++ src/adaptation/AccessCheck.h	2009-06-27 21:02:39 +0000
@@ -39,11 +39,11 @@
     typedef int Candidate;
     typedef Vector<Candidate> Candidates;
     Candidates candidates;
-    Candidate topCandidate() { return *candidates.begin(); }
+    Candidate topCandidate() const { return *candidates.begin(); }
+    ServiceGroupPointer topGroup() const; // may return nil
 
-    void do_callback();
+    void callBack(const ServiceGroupPointer &g);
     bool isCandidate(AccessRule &r);
-    bool done;
 
 public:
     void check();
@@ -51,8 +51,8 @@
     static void AccessCheckCallbackWrapper(int, void*);
     void noteAnswer(int answer);
 
-//AsyncJob virtual methods
-    virtual bool doneAll() const { return AsyncJob::doneAll() && done;}
+    // AsyncJob API
+    virtual bool doneAll() const { return false; } /// not done until mustStop
 
 private:
     CBDATA_CLASS2(AccessCheck);