]> git.ipfire.org Git - thirdparty/squid.git/commit
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)
commit3a851845a5d5c7bc5046fd237848d1e6c5e73c13
tree5f3bd71567dcc1cf7f2b6ba23520fb0ee0f0dc5f
parent6c9a5e8df38c6bd90845743a9e145b639d44b9b2
Remove unused/disabled/broken LEAK_CHECK_MODE code (#1130)

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