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.
DiskIOModule::SetupAllModules();
/* we may want the parsing process to set this up in the future */
- Store::Init();
Acl::Init();
Auth::Init(); /* required for config parsing. NOP if !USE_AUTH */
Ip::ProbeTransport(); // determine IPv4 or IPv6 capabilities before parsing.
assert(!store_table);
}
+/// this destructor is never called because Controller singleton is immortal
Store::Controller::~Controller()
{
- delete sharedMemStore;
- delete transients;
- delete disks;
-
- if (store_table) {
- hashFreeItems(store_table, destroyStoreEntry);
- hashFreeMemory(store_table);
- store_table = nullptr;
- }
+ // assert at runtime because we cannot `= delete` an overridden destructor
+ assert(!"Controller is never destroyed");
}
void
assert(!transients || e.hasTransients());
}
-namespace Store {
-static RefCount<Controller> TheRoot;
-}
-
Store::Controller&
Store::Root()
{
- assert(TheRoot);
- return *TheRoot;
-}
-
-void
-Store::Init(Controller *root)
-{
- TheRoot = root ? root : new Controller;
-}
-
-void
-Store::FreeMemory()
-{
- TheRoot = nullptr;
+ static const auto root = new Controller();
+ return *root;
}
{
public:
Controller();
- ~Controller() override;
/* Storage API */
void create() override;
static int store_dirs_rebuilding;
private:
+ ~Controller() override;
+
bool memoryCacheHasSpaceFor(const int pagesRequired) const;
void referenceBusy(StoreEntry &e);
/// safely access controller singleton
extern Controller &Root();
-/// initialize the storage module; a custom root is used by unit tests only
-extern void Init(Controller *root = nullptr);
-
-/// undo Init()
-extern void FreeMemory();
-
} // namespace Store
#endif /* SQUID_SRC_STORE_CONTROLLER_H */
namespace Store
{
Controller::Controller() {STUB_NOP}
-Controller::~Controller() {STUB_NOP}
+Controller::~Controller() STUB
void Controller::create() STUB
void Controller::init() STUB
uint64_t Controller::maxSize() const STUB_RETVAL(0)
StoreSearch *Controller::search() STUB_RETVAL(nullptr)
bool Controller::SmpAware() STUB_RETVAL(false)
int Controller::store_dirs_rebuilding = 0;
-Controller nil;
-Controller &Root() STUB_RETVAL(Store::nil)
-void Init(Controller *) STUB
-void FreeMemory() STUB
+Controller &Root() STUB_RETREF(Controller)
}
#include "store/Disk.h"
void
TestPackableStream::testGetStream()
{
- /* Setup a store root so we can create a StoreEntry */
- Store::Init();
-
CapturingStoreEntry * anEntry = new CapturingStoreEntry();
{
anEntry->lock("test");
CPPUNIT_ASSERT_EQUAL(String("12345677.7 some text !."), anEntry->_appended_text);
}
delete anEntry; // does the unlock()
- Store::FreeMemory();
}
// This test uses main() from ./testStore.cc.
if (0 > system ("rm -rf " TESTDIR))
throw std::runtime_error("Failed to clean test work directory");
- Store::Init();
-
store = new Rock::SwapDir();
addSwapDir(store);
{
CPPUNIT_NS::TestFixture::tearDown();
- Store::FreeMemory();
-
store = nullptr;
free_cachedir(&Config.cacheSwap);
CPPUNIT_TEST_SUITE_REGISTRATION( TestStore );
-int
-StoreControllerStub::callback()
-{
- return 1;
-}
-
-StoreEntry*
-StoreControllerStub::get(const cache_key*)
-{
- return nullptr;
-}
-
-void
-StoreControllerStub::get(String, void (*)(StoreEntry*, void*), void*)
-{}
-
-void
-StoreControllerStub::init()
-{}
-
-uint64_t
-StoreControllerStub::maxSize() const
-{
- return 3;
-}
-
-uint64_t
-StoreControllerStub::minSize() const
-{
- return 1;
-}
-
-uint64_t
-StoreControllerStub::currentSize() const
-{
- return 2;
-}
-
-uint64_t
-StoreControllerStub::currentCount() const
-{
- return 2;
-}
-
-int64_t
-StoreControllerStub::maxObjectSize() const
-{
- return 1;
-}
-
-void
-StoreControllerStub::getStats(StoreInfoStats &) const
-{
-}
-
-void
-StoreControllerStub::stat(StoreEntry &) const
-{
- const_cast<StoreControllerStub *>(this)->statsCalled = true;
-}
-
-StoreSearch *
-StoreControllerStub::search()
-{
- return nullptr;
-}
-
-void
-TestStore::testSetRoot()
-{
- Store::Controller *aStore(new StoreControllerStub);
- Store::Init(aStore);
-
- CPPUNIT_ASSERT_EQUAL(&Store::Root(), aStore);
- Store::FreeMemory();
-}
-
-void
-TestStore::testUnsetRoot()
-{
- Store::Controller *aStore(new StoreControllerStub);
- Store::Controller *aStore2(new StoreControllerStub);
- Store::Init(aStore);
- Store::FreeMemory();
- Store::Init(aStore2);
- CPPUNIT_ASSERT_EQUAL(&Store::Root(),aStore2);
- Store::FreeMemory();
-}
-
-void
-TestStore::testStats()
-{
- StoreControllerStub *aStore(new StoreControllerStub);
- Store::Init(aStore);
- CPPUNIT_ASSERT_EQUAL(false, aStore->statsCalled);
- StoreEntry entry;
- Store::Stats(&entry);
- CPPUNIT_ASSERT_EQUAL(true, aStore->statsCalled);
- Store::FreeMemory();
-}
-
-void
-TestStore::testMaxSize()
-{
- Store::Controller *aStore(new StoreControllerStub);
- Store::Init(aStore);
- CPPUNIT_ASSERT_EQUAL(static_cast<uint64_t>(3), aStore->maxSize());
- Store::FreeMemory();
-}
-
namespace Store {
/// check rawType that may be ignored
#ifndef SQUID_SRC_TESTS_TESTSTORE_H
#define SQUID_SRC_TESTS_TESTSTORE_H
-#include "Store.h"
-#include "store/Controlled.h"
-
#include "compat/cppunit.h"
/*
class TestStore: public CPPUNIT_NS::TestFixture
{
CPPUNIT_TEST_SUITE( TestStore );
- CPPUNIT_TEST( testSetRoot );
- CPPUNIT_TEST( testUnsetRoot );
- CPPUNIT_TEST( testStats );
- CPPUNIT_TEST( testMaxSize );
CPPUNIT_TEST( testSwapMetaTypeClassification );
CPPUNIT_TEST_SUITE_END();
public:
protected:
- void testSetRoot();
- void testUnsetRoot();
- void testStats();
- void testMaxSize();
void testSwapMetaTypeClassification();
};
-/// allows testing of methods without having all the other components live
-class StoreControllerStub : public Store::Controller
-{
-
-public:
- StoreControllerStub() : statsCalled (false) {}
-
- bool statsCalled;
-
- int callback() override;
-
- virtual StoreEntry* get(const cache_key*);
-
- virtual void get(String, void (*)(StoreEntry*, void*), void*);
-
- void init() override;
-
- void maintain() override {};
-
- uint64_t maxSize() const override;
-
- uint64_t minSize() const override;
-
- uint64_t currentSize() const override;
-
- uint64_t currentCount() const override;
-
- int64_t maxObjectSize() const override;
-
- void getStats(StoreInfoStats &) const override;
-
- void stat(StoreEntry &) const override; /* output stats to the provided store entry */
-
- virtual void reference(StoreEntry &) {} /* Reference this object */
-
- virtual bool dereference(StoreEntry &) { return true; }
-
- virtual StoreSearch *search();
-};
-
-typedef RefCount<StoreControllerStub> StoreControllerStubPointer;
-
#endif /* SQUID_SRC_TESTS_TESTSTORE_H */
void
TestStoreController::testStats()
{
- Store::Init();
StoreEntry *logEntry = new StoreEntry;
logEntry->createMemObject("dummy_storeId", nullptr, HttpRequestMethod());
logEntry->store_status = STORE_PENDING;
free_cachedir(&Config.cacheSwap);
CPPUNIT_ASSERT_EQUAL(true, aStore->statsCalled);
CPPUNIT_ASSERT_EQUAL(true, aStore2->statsCalled);
- Store::FreeMemory();
}
static void
StoreEntry *logEntry = new StoreEntry;
logEntry->createMemObject("dummy_storeId", nullptr, HttpRequestMethod());
logEntry->store_status = STORE_PENDING;
- Store::Init();
TestSwapDirPointer aStore (new TestSwapDir);
TestSwapDirPointer aStore2 (new TestSwapDir);
addSwapDir(aStore);
addSwapDir(aStore2);
CPPUNIT_ASSERT_EQUAL(static_cast<uint64_t>(6), Store::Root().maxSize());
free_cachedir(&Config.cacheSwap);
- Store::FreeMemory();
}
static StoreEntry *
TestStoreController::testSearch()
{
commonInit();
- Store::Init();
TestSwapDirPointer aStore (new TestSwapDir);
TestSwapDirPointer aStore2 (new TestSwapDir);
addSwapDir(aStore);
CPPUNIT_ASSERT_EQUAL(true, search->isDone());
CPPUNIT_ASSERT_EQUAL(static_cast<StoreEntry *>(nullptr), search->currentItem());
//CPPUNIT_ASSERT_EQUAL(false, search->next());
-
- Store::FreeMemory();
}
// This test uses main() from ./testStore.cc.
StoreEntry *logEntry = new StoreEntry;
logEntry->createMemObject("dummy_storeId", nullptr, HttpRequestMethod());
logEntry->store_status = STORE_PENDING;
- Store::Init();
TestSwapDirPointer aStore (new TestSwapDir);
TestSwapDirPointer aStore2 (new TestSwapDir);
addSwapDir(aStore);
free_cachedir(&Config.cacheSwap);
CPPUNIT_ASSERT_EQUAL(true, aStore->statsCalled);
CPPUNIT_ASSERT_EQUAL(true, aStore2->statsCalled);
- Store::FreeMemory();
}
void
StoreEntry *logEntry = new StoreEntry;
logEntry->createMemObject("dummy_storeId", nullptr, HttpRequestMethod());
logEntry->store_status = STORE_PENDING;
- Store::Init();
TestSwapDirPointer aStore (new TestSwapDir);
TestSwapDirPointer aStore2 (new TestSwapDir);
addSwapDir(aStore);
addSwapDir(aStore2);
CPPUNIT_ASSERT_EQUAL(static_cast<uint64_t>(6), Store::Root().maxSize());
free_cachedir(&Config.cacheSwap);
- Store::FreeMemory();
}
static StoreEntry *
TestStoreHashIndex::testSearch()
{
commonInit();
- Store::Init();
TestSwapDirPointer aStore (new TestSwapDir);
TestSwapDirPointer aStore2 (new TestSwapDir);
addSwapDir(aStore);
CPPUNIT_ASSERT_EQUAL(true, search->isDone());
CPPUNIT_ASSERT_EQUAL(static_cast<StoreEntry *>(nullptr), search->currentItem());
//CPPUNIT_ASSERT_EQUAL(false, search->next());
-
- Store::FreeMemory();
}
// This test uses main() from ./testStore.cc.
if (0 > system ("rm -rf " TESTDIR))
throw std::runtime_error("Failed to clean test work directory");
- Store::Init();
-
MySwapDirPointer aStore (new Fs::Ufs::UFSSwapDir("ufs", "Blocking"));
aStore->IO = new Fs::Ufs::UFSStrategy(DiskIOModule::Find("Blocking")->createStrategy());
CPPUNIT_ASSERT_EQUAL(true, search->isDone());
CPPUNIT_ASSERT_EQUAL(static_cast<StoreEntry *>(nullptr), search->currentItem());
- Store::FreeMemory();
-
free_cachedir(&Config.cacheSwap);
// TODO: here we should test a dirty rebuild
if (0 > system ("rm -rf " TESTDIR))
throw std::runtime_error("Failed to clean test work directory");
- // This assertion may fail if previous test cases fail.
- // Apparently, CPPUNIT_ASSERT* failure may prevent destructors of local
- // objects such as "StorePointer aRoot" from being called.
- CPPUNIT_ASSERT(!store_table); // or StoreHashIndex ctor will abort below
-
- Store::Init();
MySwapDirPointer aStore (new Fs::Ufs::UFSSwapDir("ufs", "Blocking"));
addSwapDir(aStore);
commonInit();
safe_free(config_line);
CPPUNIT_ASSERT(aStore->IO->io != nullptr);
- Store::FreeMemory();
free_cachedir(&Config.cacheSwap);
safe_free(Config.replPolicy->type);
delete Config.replPolicy;
projects / thirdparty / squid.git / commitdiff
