]> git.ipfire.org Git - thirdparty/squid.git/commitdiff
Remove unused/disabled/broken LEAK_CHECK_MODE code (#1130)
authorAlex Rousskov <rousskov@measurement-factory.com>
Wed, 31 Aug 2022 17:15:31 +0000 (17:15 +0000)
committerSquid Anubis <squid-anubis@squid-cache.org>
Wed, 31 Aug 2022 17:15:36 +0000 (17:15 +0000)
LEAK_CHECK_MODE had not worked since before 2006 commit afec404. The
disabled code does not compile since 2012 commit b65ce00 and would crash
current Squids during shutdown. We should not fix it because the
underlying "delete essentialService" idea is deeply flawed.

Writing correct code that uses essential modules/services that may
"suddenly" disappear or crash is nearly impossible. The trickle of
assertions due to missing Store::Root() is a case in point. These risks
and efforts are also unnecessary: We can and should build APIs that
provide essential services without disappearing or crashing. Keeping
heap-allocated service objects during shutdown helps with that.

Valgrind and other modern leak detection tools are capable of
distinguishing still-reachable at-exit memory from runtime memory leaks.
We do not need to delete all still-reachable at-exit objects to enable
that leak detection functionality.

The OS will reclaim allocated heap memory anyway.

N.B. Despite the legacy code implications, we should distinguish
low-level new/delete (a.k.a. fooInit() and fooFree()) memory management
code (which is an internal service matter that essential service users
should not be exposed to!) with configure-reconfigure-reconfigure-sync
events. There is value in notifying services about configuration
(changes) and various shutdown stages, of course. We already have the
RunnersRegistry API for handling such notifications.

Even without explicit "delete essentialService" calls, we may still have
problems during C++ post-exit() cleanup where destructors of various
globals are called "concurrently", with few order guarantees. We should
avoid non-heap-allocated globals with "complicated" destructors, but
their elimination is out of scope here.

21 files changed:
include/squid.h
src/Store.h
src/client_db.cc
src/client_db.h
src/event.cc
src/event.h
src/fqdncache.cc
src/fqdncache.h
src/icmp/net_db.cc
src/icmp/net_db.h
src/ipcache.cc
src/ipcache.h
src/main.cc
src/stat.cc
src/stat.h
src/store.cc
src/tests/stub_client_db.cc
src/tests/stub_event.cc
src/tests/stub_fqdncache.cc
src/tests/stub_ipcache.cc
src/tests/stub_libicmp.cc

index 6d9a66c05ddd61ff49f7fa8ec1370b88798c04df..74652aede982f5a7b31d7023e767be65e30f06d9 100644 (file)
 #define SQUID_UDP_SO_RCVBUF SQUID_DETECT_UDP_SO_RCVBUF
 #endif
 
-/*
- * Determine if this is a leak check build or standard
- */
-#if PURIFY || WITH_VALGRIND
-#define LEAK_CHECK_MODE 1
-#endif
-
 /* temp hack: needs to be pre-defined for now. */
 #define SQUID_MAXPATHLEN 256
 
index a8e686af0baceecdb85f273eba9f48f50e649cf6..62894acffac428a7cc47fb0536ce0091b7504f85 100644 (file)
@@ -427,9 +427,6 @@ void storeInit(void);
 /// \ingroup StoreAPI
 void storeConfigure(void);
 
-/// \ingroup StoreAPI
-void storeFreeMemory(void);
-
 /// \ingroup StoreAPI
 int expiresMoreThan(time_t, time_t);
 
index 994931605e2985e270991cc0a855072801626563..91bba5c5945b02cb071699017b50ff833e21672c 100644 (file)
@@ -346,14 +346,6 @@ ClientInfo::~ClientInfo()
     debugs(77, 9, "ClientInfo destructed, this=" << static_cast<void*>(this));
 }
 
-void
-clientdbFreeMemory(void)
-{
-    hashFreeItems(client_table, clientdbFreeItem);
-    hashFreeMemory(client_table);
-    client_table = nullptr;
-}
-
 static void
 clientdbScheduledGC(void *)
 {
index 2fe603998c82cfdff87bad1e3ae2df0a597d3861..66ff77d76ce2dcfb0af698316597a3325eb9f6ca 100644 (file)
@@ -30,7 +30,6 @@ class ClientInfo;
 void clientdbUpdate(const Ip::Address &, const LogTags &, AnyP::ProtocolType, size_t);
 int clientdbCutoffDenied(const Ip::Address &);
 void clientdbDump(StoreEntry *);
-void clientdbFreeMemory(void);
 int clientdbEstablished(const Ip::Address &, int);
 
 #if USE_DELAY_POOLS
index cefdb224018c1f96b875d32850193a79b106ec36..00c68d995d187b342b4371c8413931b68407a647 100644 (file)
@@ -141,12 +141,6 @@ eventDump(StoreEntry * sentry)
     EventScheduler::GetInstance()->dump(sentry);
 }
 
-void
-eventFreeMemory(void)
-{
-    EventScheduler::GetInstance()->clean();
-}
-
 int
 eventFind(EVH * func, void *arg)
 {
index 2e81d0095a51f876bf85adabdb894e63b85b381b..31fc9ab7bea25ada6af16791f6d27878a22b1b69 100644 (file)
@@ -21,7 +21,6 @@ void eventAdd(const char *name, EVH * func, void *arg, double when, int, bool cb
 void eventAddIsh(const char *name, EVH * func, void *arg, double delta_ish, int);
 void eventDelete(EVH * func, void *arg);
 void eventInit(void);
-void eventFreeMemory(void);
 int eventFind(EVH *, void *);
 
 class ev_entry
index a08a6d76fb62c2b418338285ec6445dcc71db50c..5cc354f77fdd4f17cd8c328cc6858a13bddf81c9 100644 (file)
@@ -127,7 +127,6 @@ static fqdncache_entry *fqdncache_get(const char *);
 static int fqdncacheExpiredEntry(const fqdncache_entry *);
 static void fqdncacheLockEntry(fqdncache_entry * f);
 static void fqdncacheUnlockEntry(fqdncache_entry * f);
-static FREE fqdncacheFreeEntry;
 static void fqdncacheAddEntry(fqdncache_entry * f);
 
 /// \ingroup FQDNCacheInternal
@@ -601,14 +600,6 @@ fqdncacheUnlockEntry(fqdncache_entry * f)
         fqdncacheRelease(f);
 }
 
-/// \ingroup FQDNCacheInternal
-static void
-fqdncacheFreeEntry(void *data)
-{
-    fqdncache_entry *f = (fqdncache_entry *)data;
-    delete f;
-}
-
 fqdncache_entry::~fqdncache_entry()
 {
     for (int k = 0; k < (int)name_count; ++k)
@@ -618,15 +609,6 @@ fqdncache_entry::~fqdncache_entry()
     xfree(error_message);
 }
 
-/// \ingroup FQDNCacheAPI
-void
-fqdncacheFreeMemory(void)
-{
-    hashFreeItems(fqdn_table, fqdncacheFreeEntry);
-    hashFreeMemory(fqdn_table);
-    fqdn_table = nullptr;
-}
-
 /**
  \ingroup FQDNCacheAPI
  *
index b5df1133b666bf95788fa9ae130c92342ccb2bab..3b803b56ea74890ef1cd504a6935f8ac864cec85 100644 (file)
@@ -27,7 +27,6 @@ typedef void FQDNH(const char *, const Dns::LookupDetails &details, void *);
 
 void fqdncache_init(void);
 void fqdnStats(StoreEntry *);
-void fqdncacheFreeMemory(void);
 void fqdncache_restart(void);
 void fqdncache_purgelru(void *);
 void fqdncacheAddEntryFromHosts(char *addr, SBufList &hostnames);
index d7ed9ee906abc501b54a65b76c8874f0451b0fc4..cda165551e84695b45c8b6c057a9746c28e91efc 100644 (file)
@@ -106,8 +106,6 @@ static net_db_peer *netdbPeerByName(const netdbEntry * n, const char *);
 static net_db_peer *netdbPeerAdd(netdbEntry * n, CachePeer * e);
 static const char *netdbPeerName(const char *name);
 static IPH netdbSendPing;
-static FREE netdbFreeNameEntry;
-static FREE netdbFreeNetdbEntry;
 static STCB netdbExchangeHandleReply;
 
 /* We have to keep a local list of CachePeer names.  The Peers structure
@@ -666,21 +664,6 @@ netdbPeerName(const char *name)
     return wordlistAdd(&peer_names, name);
 }
 
-static void
-netdbFreeNetdbEntry(void *data)
-{
-    netdbEntry *n = (netdbEntry *)data;
-    safe_free(n->peers);
-    delete n;
-}
-
-static void
-netdbFreeNameEntry(void *data)
-{
-    net_db_name *x = (net_db_name *)data;
-    delete x;
-}
-
 static void
 netdbExchangeHandleReply(void *data, StoreIOBuffer receivedData)
 {
@@ -953,21 +936,6 @@ netdbHandlePingReply(const Ip::Address &from, int hops, int rtt)
 #endif
 }
 
-void
-netdbFreeMemory(void)
-{
-#if USE_ICMP
-    hashFreeItems(addr_table, netdbFreeNetdbEntry);
-    hashFreeMemory(addr_table);
-    addr_table = nullptr;
-    hashFreeItems(host_table, netdbFreeNameEntry);
-    hashFreeMemory(host_table);
-    host_table = nullptr;
-    wordlistDestroy(&peer_names);
-    peer_names = nullptr;
-#endif
-}
-
 void
 netdbDump(StoreEntry * sentry)
 {
index 902dc7b97e16d645c0568a8578cefc65bc233835..68a1b5af54faf935db59284d23cc5df78c5d172d 100644 (file)
@@ -71,7 +71,6 @@ void netdbHandlePingReply(const Ip::Address &from, int hops, int rtt);
 void netdbPingSite(const char *hostname);
 void netdbDump(StoreEntry *);
 
-void netdbFreeMemory(void);
 int netdbHostHops(const char *host);
 int netdbHostRtt(const char *host);
 void netdbUpdatePeer(const AnyP::Uri &, CachePeer *, int rtt, int hops);
index f03e579a42f3bdebf91efacbcddc0a077f724dbf..e9b9811c707ea31f983e79ae9abfd4769debd784 100644 (file)
@@ -1082,15 +1082,6 @@ ipcache_entry::~ipcache_entry()
     xfree(hash.key);
 }
 
-/// \ingroup IPCacheAPI
-void
-ipcacheFreeMemory(void)
-{
-    hashFreeItems(ip_table, ipcacheFreeEntry);
-    hashFreeMemory(ip_table);
-    ip_table = nullptr;
-}
-
 /**
  \ingroup IPCacheAPI
  *
index 7b18e8fdcbffc4ffdbd26089ccdfb1b8ca7db67a..fe73c58ef00d61edccd20c83b59de841df35bad6 100644 (file)
@@ -226,7 +226,6 @@ void ipcacheInvalidateNegative(const char *);
 void ipcache_init(void);
 void ipcacheMarkBadAddr(const char *name, const Ip::Address &);
 void ipcacheMarkGoodAddr(const char *name, const Ip::Address &);
-void ipcacheFreeMemory(void);
 void ipcache_restart(void);
 int ipcacheAddEntryFromHosts(const char *name, const char *ipaddr);
 
index 7258363044b4583b2f347cab6deb8d008cc8c638..64184e6547d3a0df37fbd83488d5d329bf76c44b 100644 (file)
@@ -2114,21 +2114,6 @@ SquidShutdown()
     Store::Root().sync();       /* Flush log close */
     StoreFileSystem::FreeAllFs();
     DiskIOModule::FreeAllModules();
-#if LEAK_CHECK_MODE && 0 /* doesn't work at the moment */
-
-    configFreeMemory();
-    storeFreeMemory();
-    /*stmemFreeMemory(); */
-    netdbFreeMemory();
-    ipcacheFreeMemory();
-    fqdncacheFreeMemory();
-    asnFreeMemory();
-    clientdbFreeMemory();
-    statFreeMemory();
-    eventFreeMemory();
-    mimeFreeMemory();
-    errorClean();
-#endif
     Store::FreeMemory();
 
     fdDumpOpen();
index c17a50133fcfae159242a26e9346f403c3128b56..e0b532253a5871be6b995d4d4d8cdd78ec442634 100644 (file)
@@ -1569,17 +1569,6 @@ DumpCountersStats(Mgr::CountersActionData& stats, StoreEntry* sentry)
                       stats.hitValidationFailures);
 }
 
-void
-statFreeMemory(void)
-{
-    // TODO: replace with delete[]
-    for (int i = 0; i < N_COUNT_HIST; ++i)
-        CountHist[i] = StatCounters();
-
-    for (int i = 0; i < N_COUNT_HOUR_HIST; ++i)
-        CountHourHist[i] = StatCounters();
-}
-
 static void
 statPeerSelect(StoreEntry * sentry)
 {
index 71c1fd4083acc5c8cb7cd87693e7470d1a08025a..7397efd9835b628d71e5796fa54e40cb3dc6cee1 100644 (file)
@@ -12,7 +12,6 @@
 #define SQUID_STAT_H_
 
 void statInit(void);
-void statFreeMemory(void);
 double median_svc_get(int, int);
 void pconnHistCount(int, int);
 int stat5minClientRequests(void);
index ab732e7b7d9036ebff6c119b56e816c83909f04a..debb92655a69717b6efd7e5e936a336b1ad6f130 100644 (file)
@@ -1307,16 +1307,6 @@ StoreEntry::negativeCache()
     }
 }
 
-void
-storeFreeMemory(void)
-{
-    Store::FreeMemory();
-#if USE_CACHE_DIGESTS
-    delete store_digest;
-#endif
-    store_digest = nullptr;
-}
-
 int
 expiresMoreThan(time_t expires, time_t when)
 {
index a67df93c3b1a3cfa87d543d762581a51350389f4..ca6ddfd1c60a5bd83152effd75b3f92d81ce67d4 100644 (file)
@@ -15,7 +15,6 @@
 void clientdbUpdate(const Ip::Address &, const LogTags &, AnyP::ProtocolType, size_t) STUB
 int clientdbCutoffDenied(const Ip::Address &) STUB_RETVAL(-1)
 void clientdbDump(StoreEntry *) STUB
-void clientdbFreeMemory(void) STUB
 int clientdbEstablished(const Ip::Address &, int) STUB_RETVAL(-1)
 #if USE_DELAY_POOLS
 void clientdbSetWriteLimiter(ClientInfo *, const int,const double,const double) STUB
index 8946da350fac25cec1868febb718fc1c70035d77..7e5d5449daf6a6589de8aec4d8dda22453dd7ed8 100644 (file)
@@ -16,7 +16,6 @@ void eventAdd(const char *, EVH *, void *, double, int, bool) STUB_NOP
 void eventAddIsh(const char *, EVH *, void *, double, int) STUB
 void eventDelete(EVH *, void *) STUB
 void eventInit(void) STUB
-void eventFreeMemory(void) STUB
 int eventFind(EVH *, void *) STUB_RETVAL(-1)
 
 // ev_entry::ev_entry(char const * name, EVH * func, void *arg, double when, int weight, bool cbdata) STUB
index 3bc612fbeb90c2f0178ecc9e7707fa1ffbc93d91..8a1a12a779fd640dba063e9604c18f580bbd4bcb 100644 (file)
@@ -16,7 +16,6 @@ bool Dns::ResolveClientAddressesAsap = false;
 
 void fqdncache_init(void) STUB
 void fqdnStats(StoreEntry *) STUB
-void fqdncacheFreeMemory(void) STUB
 void fqdncache_restart(void) STUB
 void fqdncache_purgelru(void *) STUB
 void fqdncacheAddEntryFromHosts(char *, SBufList &) STUB
index 81a8e09295c6a8ced36f3ad8e8324633e76bb307..b694f035959e3015f582d56789d2d9a3a6d0d0b8 100644 (file)
@@ -20,7 +20,6 @@ void ipcacheInvalidateNegative(const char *) STUB
 void ipcache_init(void) STUB
 void ipcacheMarkBadAddr(const char *, const Ip::Address &) STUB
 void ipcacheMarkGoodAddr(const char *, const Ip::Address &) STUB
-void ipcacheFreeMemory(void) STUB
 void ipcache_restart(void) STUB
 int ipcacheAddEntryFromHosts(const char *, const char *) STUB_RETVAL(-1)
 
index 658fe8af6fe2817f47305bd42762d30da0815c2e..fc4c06a54149fa31eceb5ced105460aca8be7f93 100644 (file)
@@ -27,7 +27,6 @@ void netdbInit(void) STUB
 void netdbHandlePingReply(const Ip::Address &, int, int) STUB
 void netdbPingSite(const char *) STUB
 void netdbDump(StoreEntry *) STUB
-void netdbFreeMemory(void) STUB
 int netdbHostHops(const char *) STUB_RETVAL(-1)
 int netdbHostRtt(const char *) STUB_RETVAL(-1)
 void netdbUpdatePeer(const AnyP::Uri &, CachePeer *, int, int) STUB