]> git.ipfire.org Git - thirdparty/squid.git/commit
Fix Controller.cc TheRoot assertion during shutdown (#1707)
authorAlex Rousskov <rousskov@measurement-factory.com>
Wed, 28 Feb 2024 19:27:15 +0000 (19:27 +0000)
committerSquid Anubis <squid-anubis@squid-cache.org>
Wed, 28 Feb 2024 19:27:20 +0000 (19:27 +0000)
commit1f50e07b88dc02c1413a87f10128d29c12937e02
tree26f561bd6572fe3c69098acc0393b610beaec286
parentc813943dc78aba2a707a0b49020b06ccbd43cee3
Fix Controller.cc TheRoot assertion during shutdown (#1707)

    FATAL: assertion failed: Controller.cc:933: "TheRoot"

Store is an essential service, used by a lot of Squid code. As was
already established in 2022 commit 23b7963, this service should be
available during shutdown. That commit correctly removed explicit Store
service termination, but missed the fact that the reference-counting
TheRoot pointer (that provides access to the Store Controller singleton)
gets _automatically_ destroyed during C++ cleanup. This change removes
TheRoot reference counting, making that Controller singleton immortal.

Squid asserted when exiting with active entries in the shared memory
cache because TheRoot destruction leads to Controller destruction,
Controller destructor cleans up cache index, and that cleanup code may
result in calls to Store::Root() that dereferences destroyed TheRoot.

These assertions were seen when Squid was shutdown cleanly (e.g., using
SIGTERM) and when a kid process was exiting due to a fatal() error.

Making Store::Controller singleton immortal means that the class
destructor is never called. Fortunately, the destructor did nothing
particularly useful; Store flushing is performed by Controller::sync()
which is explicitly called during early stages of clean shutdown. The
now-unused destructor code was removed: Implementing this destructor in
a meaningful way (while avoiding accessing a being-destructed global
Store!) requires heroic efforts (which would be wasted since the
destructor is never actually called).

Also made Store Controller singleton available on the first use,
complying with Store's "essential, widely used service" status and
removing the need for an explicit Store::Init(void) call.

Also removed unit tests that required support for dynamic replacement of
Store Controller singleton. The minuscule value of the removed tests did
not justify the costs of supporting a replaceable Store Controller.

Also removed Store::FreeMemory(). Its implementation contradicted Store
status as an essential service. The function was only used by unit
tests, and its removal addresses the corresponding commit 23b7963 TODO.
src/main.cc
src/store/Controller.cc
src/store/Controller.h
src/tests/stub_libstore.cc
src/tests/testPackableStream.cc
src/tests/testRock.cc
src/tests/testStore.cc
src/tests/testStore.h
src/tests/testStoreController.cc
src/tests/testStoreHashIndex.cc
src/tests/testUfs.cc