From e2cc8c07bcfb7ef139107764a060ab68081709ff Mon Sep 17 00:00:00 2001
From: Amos Jeffries <squid3@treenet.co.nz>
Date: Sat, 11 Mar 2017 07:12:05 +1300
Subject: [PATCH] Bug 4680: Memory leak in v5 r15076

Convrts the Http::Message lock/unlock macros to inline functions so the compiler can
catch this type of regression in future Pointer updates
---
 src/FwdState.cc     |  5 +----
 src/http/Message.h  | 18 ++++++++++++++++--
 src/mime.cc         |  9 ++++-----
 src/neighbors.cc    |  1 -
 src/peer_digest.cc  |  4 +---
 src/store_digest.cc |  4 +---
 6 files changed, 23 insertions(+), 18 deletions(-)

diff --git a/src/FwdState.cc b/src/FwdState.cc
index fc6bb1e832..afe01a598d 100644
--- a/src/FwdState.cc
+++ b/src/FwdState.cc
@@ -343,7 +343,6 @@ FwdState::Start(const Comm::ConnectionPointer &clientConn, StoreEntry *entry, Ht
      * Might want to assert that request is NULL at this point
      */
     entry->mem_obj->request = request;
-    HTTPMSGLOCK(entry->mem_obj->request);
 #if URL_CHECKSUM_DEBUG
 
     entry->mem_obj->checkUrlChecksum();
@@ -426,10 +425,8 @@ FwdState::fail(ErrorState * errorState)
     delete err;
     err = errorState;
 
-    if (!errorState->request) {
+    if (!errorState->request)
         errorState->request = request;
-        HTTPMSGLOCK(errorState->request);
-    }
 
     if (err->type != ERR_ZERO_SIZE_OBJECT)
         return;
diff --git a/src/http/Message.h b/src/http/Message.h
index 9dcb79a520..246d5bbe8a 100644
--- a/src/http/Message.h
+++ b/src/http/Message.h
@@ -140,8 +140,22 @@ protected:
 
 } // namespace Http
 
-#define HTTPMSGUNLOCK(a) if (a) { if ((a)->unlock() == 0) delete (a); (a)=NULL; }
-#define HTTPMSGLOCK(a) (a)->lock()
+inline void
+HTTPMSGUNLOCK(Http::Message *a)
+{
+    if (a) {
+        if (a->unlock() == 0)
+            delete a;
+       a = nullptr;
+    }
+}
+
+inline void
+HTTPMSGLOCK(Http::Message *a)
+{
+    if (a)
+        a->lock();
+}
 
 #endif /* SQUID_HTTPMSG_H */
 
diff --git a/src/mime.cc b/src/mime.cc
index bf411de46f..cc0208f34f 100644
--- a/src/mime.cc
+++ b/src/mime.cc
@@ -402,15 +402,14 @@ MimeIcon::created(StoreEntry *newEntry)
     EBIT_SET(e->flags, ENTRY_SPECIAL);
     e->setPublicKey();
     e->buffer();
-    HttpRequest *r = HttpRequest::CreateFromUrl(url_);
 
-    if (NULL == r)
+    HttpRequestPointer r(HttpRequest::CreateFromUrl(url_));
+    if (!r)
         fatalf("mimeLoadIcon: cannot parse internal URL: %s", url_);
 
     e->mem_obj->request = r;
-    HTTPMSGLOCK(e->mem_obj->request);
 
-    HttpReply *reply = new HttpReply;
+    HttpReplyPointer reply(new HttpReply);
 
     if (status == Http::scNoContent)
         reply->setHeaders(status, NULL, NULL, 0, -1, -1);
@@ -419,7 +418,7 @@ MimeIcon::created(StoreEntry *newEntry)
     reply->cache_control = new HttpHdrCc();
     reply->cache_control->maxAge(86400);
     reply->header.putCc(reply->cache_control);
-    e->replaceHttpReply(reply);
+    e->replaceHttpReply(reply.getRaw());
 
     if (status == Http::scOkay) {
         /* read the file into the buffer and append it to store */
diff --git a/src/neighbors.cc b/src/neighbors.cc
index 905b12b3ea..7fd6ac0e0b 100644
--- a/src/neighbors.cc
+++ b/src/neighbors.cc
@@ -1367,7 +1367,6 @@ peerCountMcastPeersStart(void *data)
     psstate->ping.start = current_time;
     mem = fake->mem_obj;
     mem->request = psstate->request;
-    HTTPMSGLOCK(mem->request);
     mem->start_ping = current_time;
     mem->ping_reply_callback = peerCountHandleIcpReply;
     mem->ircb_data = psstate;
diff --git a/src/peer_digest.cc b/src/peer_digest.cc
index ca577516a2..3306a2dd91 100644
--- a/src/peer_digest.cc
+++ b/src/peer_digest.cc
@@ -530,10 +530,8 @@ peerDigestFetchReply(void *data, char *buf, ssize_t size)
             /* our old entry is fine */
             assert(fetch->old_entry);
 
-            if (!fetch->old_entry->mem_obj->request) {
+            if (!fetch->old_entry->mem_obj->request)
                 fetch->old_entry->mem_obj->request = fetch->entry->mem_obj->request;
-                HTTPMSGLOCK(fetch->old_entry->mem_obj->request);
-            }
 
             assert(fetch->old_entry->mem_obj->request);
 
diff --git a/src/store_digest.cc b/src/store_digest.cc
index e4366472d2..95a428cb05 100644
--- a/src/store_digest.cc
+++ b/src/store_digest.cc
@@ -421,9 +421,7 @@ storeDigestRewriteStart(void *datanotused)
     assert(e);
     sd_state.rewrite_lock = e;
     debugs(71, 3, "storeDigestRewrite: url: " << url << " key: " << e->getMD5Text());
-    HttpRequest *req = HttpRequest::CreateFromUrl(url);
-    e->mem_obj->request = req;
-    HTTPMSGLOCK(e->mem_obj->request);
+    e->mem_obj->request = HttpRequest::CreateFromUrl(url);
     /* wait for rebuild (if any) to finish */
 
     if (sd_state.rebuild_lock) {
-- 
2.39.5