Reduce helper callback code duplication (#1490)

No Squid functionality changes are expected.

diff --git a/src/helper.cc b/src/helper.cc
index 1092a3731d089463cb26cc99559fb09250e1cbb7..b2c09297da98ad4f87398d6e874f9c290d8f081a 100644 (file)
--- a/src/helper.cc
+++ b/src/helper.cc
@@ -123,18 +123,14 @@ Helper::SessionBase::closeWritePipeSafely(const char * const id_name)
 }
 
 void
-Helper::SessionBase::dropQueued()
+Helper::SessionBase::dropQueued(Client &client)
 {
     while (!requests.empty()) {
         // XXX: re-schedule these on another helper?
         const auto r = requests.front();
         requests.pop_front();
-        void *cbdata;
-        if (cbdataReferenceValidDone(r->request.data, &cbdata)) {
-            r->reply.result = Helper::Unknown;
-            r->request.callback(cbdata, r->reply);
-        }
-
+        r->reply.result = Helper::Unknown;
+        client.callBack(*r);
         delete r;
     }
 }
@@ -170,9 +166,9 @@ Helper::Session::~Session()
 }
 
 void
-Helper::Session::dropQueued()
+Helper::Session::dropQueued(Client &client)
 {
-    SessionBase::dropQueued();
+    SessionBase::dropQueued(client);
     requestsIndex.clear();
 }
 
@@ -572,6 +568,18 @@ Helper::Client::submit(const char * const buf, HLPCB * const callback, void * co
     debugs(84, DBG_DATA, Raw("buf", buf, strlen(buf)));
 }
 
+void
+Helper::Client::callBack(Xaction &r)
+{
+    const auto callback = r.request.callback;
+    Assure(callback);
+
+    r.request.callback = nullptr;
+    void *cbdata = nullptr;
+    if (cbdataReferenceValidDone(r.request.data, &cbdata))
+        callback(cbdata, r.reply);
+}
+
 /// Submit request or callback the caller with a Helper::Error error.
 /// If the reservation is not set then reserves a new helper.
 void
@@ -663,7 +671,7 @@ statefulhelper::submit(const char *buf, HLPCB * callback, void *data, const Help
         if (!lastServer) {
             debugs(84, DBG_CRITICAL, "ERROR: Helper " << id_name << " reservation expired (" << reservation << ")");
             r->reply.result = Helper::TimedOut;
-            r->request.callback(r->request.data, r->reply);
+            callBack(*r);
             delete r;
             return;
         }
@@ -904,7 +912,7 @@ void
 Helper::Session::HelperServerClosed(Session * const srv)
 {
     srv->parent->sessionClosed(*srv);
-    srv->dropQueued();
+    srv->dropQueued(*srv->parent);
     delete srv;
 }
 
@@ -913,7 +921,7 @@ void
 helper_stateful_server::HelperServerClosed(helper_stateful_server *srv)
 {
     srv->parent->sessionClosed(*srv);
-    srv->dropQueued();
+    srv->dropQueued(*srv->parent);
     delete srv;
 }
 
@@ -962,11 +970,7 @@ helperReturnBuffer(Helper::Session * srv, const Helper::Client::Pointer &hlp, ch
                 debugs(84, DBG_IMPORTANT, "ERROR: helper: " << r->reply << ", attempt #" << (r->request.retries + 1) << " of 2");
                 retry = true;
             } else {
-                HLPCB *callback = r->request.callback;
-                r->request.callback = nullptr;
-                void *cbdata = nullptr;
-                if (cbdataReferenceValidDone(r->request.data, &cbdata))
-                    callback(cbdata, r->reply);
+                hlp->callBack(*r);
             }
         }
 
@@ -1197,7 +1201,7 @@ helperStatefulHandleRead(const Comm::ConnectionPointer &conn, char *, size_t len
         if (cbdataReferenceValid(r->request.data)) {
             r->reply.finalize();
             r->reply.reservationId = srv->reservationId;
-            r->request.callback(r->request.data, r->reply);
+            hlp->callBack(*r);
         } else {
             debugs(84, DBG_IMPORTANT, "StatefulHandleRead: no callback data registered");
             called = 0;
@@ -1483,7 +1487,7 @@ helperStatefulDispatch(helper_stateful_server * srv, Helper::Xaction * r)
         /* we don't care about releasing this helper. The request NEVER
          * gets to the helper. So we throw away the return code */
         r->reply.result = Helper::Unknown;
-        r->request.callback(r->request.data, r->reply);
+        hlp->callBack(*r);
         /* throw away the placeholder */
         delete r;
         /* and push the queue. Note that the callback may have submitted a new
@@ -1554,23 +1558,22 @@ Helper::Session::checkForTimedOutRequests(bool const retry)
         requestsIndex.erase(it);
         requests.pop_front();
         debugs(84, 2, "Request " << r->request.Id << " timed-out, remove it from queue");
-        void *cbdata;
         bool retried = false;
         if (retry && r->request.retries < MAX_RETRIES && cbdataReferenceValid(r->request.data)) {
             debugs(84, 2, "Retry request " << r->request.Id);
             ++r->request.retries;
             parent->submitRequest(r);
             retried = true;
-        } else if (cbdataReferenceValidDone(r->request.data, &cbdata)) {
+        } else if (cbdataReferenceValid(r->request.data)) {
             if (!parent->onTimedOutResponse.isEmpty()) {
                 if (r->reply.accumulate(parent->onTimedOutResponse.rawContent(), parent->onTimedOutResponse.length()))
                     r->reply.finalize();
                 else
                     r->reply.result = Helper::TimedOut;
-                r->request.callback(cbdata, r->reply);
+                parent->callBack(*r);
             } else {
                 r->reply.result = Helper::TimedOut;
-                r->request.callback(cbdata, r->reply);
+                parent->callBack(*r);
             }
         }
         --stats.pending;

diff --git a/src/helper.h b/src/helper.h
index fbf2572e2fc892c360ec635b5687135759f0c57d..8b6c2cd84093615923d0d6a00c26cdd43c0a9ff1 100644 (file)
--- a/src/helper.h
+++ b/src/helper.h
@@ -100,6 +100,9 @@ public:
     /// \param madeProgress whether the died helper(s) responded to any requests
     void handleFewerServers(bool madeProgress);
 
+    /// sends transaction response to the transaction initiator
+    void callBack(Xaction &);
+
     /// Starts required helper process(es).
     /// The caller is responsible for checking that new processes are needed.
     virtual void openSessions();
@@ -210,7 +213,7 @@ public:
     virtual bool reserved() = 0;
 
     /// dequeues and sends an Unknown answer to all queued requests
-    virtual void dropQueued();
+    virtual void dropQueued(Client &);
 
 public:
     /// Helper program identifier; does not change when contents do,
@@ -292,7 +295,7 @@ public:
 
     /* SessionBase API */
     bool reserved() override {return false;}
-    void dropQueued() override;
+    void dropQueued(Client &) override;
 
     /// Read timeout handler
     static void requestTimeout(const CommTimeoutCbParams &io);