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

diff --git a/src/FwdState.cc b/src/FwdState.cc
index fc6bb1e8328528c0871a07b8a977cdc6659fa649..afe01a598ddb0329c78c96cdbc4e4ff017695b00 100644 (file)
--- 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 9dcb79a52064bc02c3468f7f798a92ac5dc427e0..246d5bbe8a803d57ef65fadd11fbe74df000cda1 100644 (file)
--- 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 bf411de46f84d746e6d5361879109fb8411f9e59..cc0208f34f9e58c39ba5de66ef94d6cc0dceaafe 100644 (file)
--- 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 905b12b3ea7864db9016eb8b589f53cd6b6f405c..7fd6ac0e0b3244caa0567bf839b7a34fe1e06034 100644 (file)
--- 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 ca577516a25c97fec7d844403f15772bcdcd18ec..3306a2dd917ef3e3d09e6b8e79dd910d6748b58c 100644 (file)
--- 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 e4366472d235c98e0ffa3c002bdd4016f0f16796..95a428cb05e04d6ddd6a0b7b791940b01f3b5d49 100644 (file)
--- 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) {