Cleanup: remove use of MEM_DLINK_NODE for custom link-list

... implementation and replaces it all with a std::queue.

Also, de-duplicates the *Dequeue() functions by merging them into helper
class as a single nextRequest() getter method.

diff --git a/src/helper.cc b/src/helper.cc
index 590cb02843a6cc6e73ac05f96f286843c3ff50a2..cab6fa6a803a01d1b363748759ede3c9f73bbe75 100644 (file)
--- a/src/helper.cc
+++ b/src/helper.cc
@@ -54,8 +54,6 @@ static IOCB helperStatefulHandleRead;
 static void helperServerFree(helper_server *srv);
 static void helperStatefulServerFree(helper_stateful_server *srv);
 static void Enqueue(helper * hlp, Helper::Request *);
-static Helper::Request *Dequeue(helper * hlp);
-static Helper::Request *StatefulDequeue(statefulhelper * hlp);
 static helper_server *GetFirstAvailable(helper * hlp);
 static helper_stateful_server *StatefulGetFirstAvailable(statefulhelper * hlp);
 static void helperDispatch(helper_server * srv, Helper::Request * r);
@@ -667,7 +665,8 @@ helper::~helper()
 {
     /* note, don't free id_name, it probably points to static memory */
 
-    if (queue.head)
+    // TODO: if the queue is not empty it will leak Helper::Request's
+    if (!queue.empty())
         debugs(84, DBG_CRITICAL, "WARNING: freeing " << id_name << " helper with " << stats.queue_size << " requests queued");
 }
 
@@ -1102,8 +1101,7 @@ helperStatefulHandleRead(const Comm::ConnectionPointer &conn, char *, size_t len
 static void
 Enqueue(helper * hlp, Helper::Request * r)
 {
-    dlink_node *link = (dlink_node *)memAllocate(MEM_DLINK_NODE);
-    dlinkAddTail(r, link, &hlp->queue);
+    hlp->queue.push(r);
     ++ hlp->stats.queue_size;
 
     /* do this first so idle=N has a chance to grow the child pool before it hits critical. */
@@ -1132,8 +1130,7 @@ Enqueue(helper * hlp, Helper::Request * r)
 static void
 StatefulEnqueue(statefulhelper * hlp, Helper::Request * r)
 {
-    dlink_node *link = (dlink_node *)memAllocate(MEM_DLINK_NODE);
-    dlinkAddTail(r, link, &hlp->queue);
+    hlp->queue.push(r);
     ++ hlp->stats.queue_size;
 
     /* do this first so idle=N has a chance to grow the child pool before it hits critical. */
@@ -1159,35 +1156,15 @@ StatefulEnqueue(statefulhelper * hlp, Helper::Request * r)
     debugs(84, DBG_CRITICAL, "WARNING: Consider increasing the number of " << hlp->id_name << " processes in your config file.");
 }
 
-static Helper::Request *
-Dequeue(helper * hlp)
+Helper::Request *
+helper::nextRequest()
 {
-    dlink_node *link;
-    Helper::Request *r = NULL;
-
-    if ((link = hlp->queue.head)) {
-        r = (Helper::Request *)link->data;
-        dlinkDelete(link, &hlp->queue);
-        memFree(link, MEM_DLINK_NODE);
-        -- hlp->stats.queue_size;
-    }
-
-    return r;
-}
-
-static Helper::Request *
-StatefulDequeue(statefulhelper * hlp)
-{
-    dlink_node *link;
-    Helper::Request *r = NULL;
-
-    if ((link = hlp->queue.head)) {
-        r = (Helper::Request *)link->data;
-        dlinkDelete(link, &hlp->queue);
-        memFree(link, MEM_DLINK_NODE);
-        -- hlp->stats.queue_size;
-    }
+    if (queue.empty())
+        return nullptr;
 
+    auto *r = queue.front();
+    queue.pop();
+    --stats.queue_size;
     return r;
 }
 
@@ -1394,7 +1371,7 @@ helperKickQueue(helper * hlp)
     Helper::Request *r;
     helper_server *srv;
 
-    while ((srv = GetFirstAvailable(hlp)) && (r = Dequeue(hlp)))
+    while ((srv = GetFirstAvailable(hlp)) && (r = hlp->nextRequest()))
         helperDispatch(srv, r);
 }
 
@@ -1404,7 +1381,7 @@ helperStatefulKickQueue(statefulhelper * hlp)
     Helper::Request *r;
     helper_stateful_server *srv;
 
-    while ((srv = StatefulGetFirstAvailable(hlp)) && (r = StatefulDequeue(hlp)))
+    while ((srv = StatefulGetFirstAvailable(hlp)) && (r = hlp->nextRequest()))
         helperStatefulDispatch(srv, r);
 }
 

diff --git a/src/helper.h b/src/helper.h
index f531bca70bb0690fddca68042210b5829e9ab3a4..63eea4f590b4aec052c42d6ffa018edfc5c303e5 100644 (file)
--- a/src/helper.h
+++ b/src/helper.h
@@ -23,6 +23,7 @@
 
 #include <list>
 #include <map>
+#include <queue>
 
 class Packable;
 class wordlist;
@@ -62,9 +63,12 @@ public:
     }
     ~helper();
 
-    ///< whether at least one more request can be successfully submitted
+    /// whether at least one more request can be successfully submitted
     bool queueFull() const;
 
+    /// \returns next request in the queue, or nil.
+    Helper::Request *nextRequest();
+
     ///< If not full, submit request. Otherwise, either kill Squid or return false.
     bool trySubmit(const char *buf, HLPCB * callback, void *data);
 
@@ -78,7 +82,7 @@ public:
 public:
     wordlist *cmdline;
     dlink_list servers;
-    dlink_list queue;
+    std::queue<Helper::Request *> queue;
     const char *id_name;
     Helper::ChildConfig childs;    ///< Configuration settings for number running.
     int ipc_type;

diff --git a/src/mem/forward.h b/src/mem/forward.h
index 9e6a47f9b6ee6f0c098e0463f62fcba9598cd36c..baeab27b60cf28fb839c4cfec3a75a42e4298e84 100644 (file)
--- a/src/mem/forward.h
+++ b/src/mem/forward.h
@@ -47,7 +47,6 @@ typedef enum {
     MEM_ACL_DENY_INFO_LIST,
     MEM_ACL_NAME_LIST,
     MEM_LINK_LIST,
-    MEM_DLINK_NODE,
     MEM_DREAD_CTRL,
     MEM_DWRITE_Q,
     MEM_MD5_DIGEST,

diff --git a/src/mem/old_api.cc b/src/mem/old_api.cc
index 58bb27b4bac394bcc13da0b569b3f263686fefd3..85daace68da9169ad33814fc6047d96878bd1970 100644 (file)
--- a/src/mem/old_api.cc
+++ b/src/mem/old_api.cc
@@ -446,7 +446,6 @@ Mem::Init(void)
                 sizeof(AclDenyInfoList), 0);
     memDataInit(MEM_ACL_NAME_LIST, "acl_name_list", sizeof(AclNameList), 0);
     memDataInit(MEM_LINK_LIST, "link_list", sizeof(link_list), 10);
-    memDataInit(MEM_DLINK_NODE, "dlink_node", sizeof(dlink_node), 10);
     memDataInit(MEM_DREAD_CTRL, "dread_ctrl", sizeof(dread_ctrl), 0);
     memDataInit(MEM_DWRITE_Q, "dwrite_q", sizeof(dwrite_q), 0);
     memDataInit(MEM_NETDBENTRY, "netdbEntry", sizeof(netdbEntry), 0);