Index: source/graphics/ObjectManager.cpp =================================================================== --- source/graphics/ObjectManager.cpp +++ source/graphics/ObjectManager.cpp @@ -51,7 +51,7 @@ { RegisterFileReloadFunc(ReloadChangedFileCB, this); - m_QualityHook = std::make_unique(g_ConfigDB.RegisterHookAndCall("max_actor_quality", [this]() { this->ActorQualityChanged(); })); + m_QualityHook = std::make_unique(g_ConfigDB.RegisterHookAndCall("max_actor_quality", [this]() { ActorQualityChanged(); })); if (!CXeromyces::AddValidator(g_VFS, "actor", "art/actors/actor.rng")) LOGERROR("CObjectManager: failed to load actor grammar file 'art/actors/actor.rng'"); @@ -61,8 +61,6 @@ { UnloadObjects(); - g_ConfigDB.UnregisterHook(std::move(m_QualityHook)); - UnregisterFileReloadFunc(ReloadChangedFileCB, this); } Index: source/gui/tests/test_GuiManager.h =================================================================== --- source/gui/tests/test_GuiManager.h +++ source/gui/tests/test_GuiManager.h @@ -28,7 +28,7 @@ class TestGuiManager : public CxxTest::TestSuite { - CConfigDB* configDB; + std::unique_ptr configDB; public: void setUp() @@ -37,7 +37,7 @@ TS_ASSERT_OK(g_VFS->Mount(L"", DataDir() / "mods" / "_test.gui" / "", VFS_MOUNT_MUST_EXIST)); TS_ASSERT_OK(g_VFS->Mount(L"cache", DataDir() / "_testcache" / "", 0, VFS_MAX_PRIORITY)); - configDB = new CConfigDB; + configDB = std::make_unique(); CXeromyces::Startup(); @@ -48,7 +48,7 @@ { delete g_GUI; CXeromyces::Terminate(); - delete configDB; + configDB.reset(); g_VFS.reset(); DeleteDirectory(DataDir()/"_testcache"); } @@ -189,7 +189,6 @@ ScriptInterface::FromJSVal(prq, js_hotkey_pressed_value, hotkey_pressed_value); TS_ASSERT_EQUALS(hotkey_pressed_value, false); - configDB->RemoveValue(CFG_SYSTEM, test_hotkey_name); UnloadHotkeys(); } }; Index: source/ps/ConfigDB.h =================================================================== --- source/ps/ConfigDB.h +++ source/ps/ConfigDB.h @@ -29,9 +29,9 @@ #include "lib/file/vfs/vfs_path.h" #include "ps/CStr.h" -#include "ps/Singleton.h" #include +#include #include /** @@ -59,12 +59,22 @@ // Actually defined below - requires access to CConfigDB. class CConfigDBHook; -#define g_ConfigDB CConfigDB::GetSingleton() +#define g_ConfigDB (*CConfigDB::Instance()) -class CConfigDB : public Singleton +class CConfigDB { friend CConfigDBHook; public: + CConfigDB(); + ~CConfigDB(); + CConfigDB(const CConfigDB&) = delete; + CConfigDB(CConfigDB&&) = delete; + + static void Initialise(); + static void Shutdown(); + static bool IsInitialised(); + static CConfigDB* Instance(); + /** * Attempt to retrieve the value of a config variable with the given name; * will search CFG_COMMAND first, and then all namespaces from the specified @@ -178,17 +188,21 @@ * Register a simple lambda that will be called anytime the value changes in any namespace * This is simple on purpose, the hook is responsible for checking if it should do something. * When RegisterHookAndCall is called, the hook is immediately triggered. + * NB: CConfigDBHook will auto-unregister the hook when destroyed, + * so you can use it to tie the lifetime of the hook to your object. + * The hook will be deleted alongside ConfigDB anyways. */ - CConfigDBHook RegisterHookAndCall(const CStr& name, std::function hook); + [[nodiscard]] CConfigDBHook RegisterHookAndCall(const CStr& name, std::function hook); void UnregisterHook(CConfigDBHook&& hook); void UnregisterHook(std::unique_ptr hook); private: - static std::map m_Map[]; - static std::multimap> m_Hooks; - static VfsPath m_ConfigFile[]; - static bool m_HasChanges[]; + std::array, CFG_LAST> m_Map; + std::multimap> m_Hooks; + shared_ptr m_HookValidityToken; + std::array m_ConfigFile; + std::array m_HasChanges; }; class CConfigDBHook @@ -196,13 +210,30 @@ friend class CConfigDB; public: CConfigDBHook() = delete; + CConfigDBHook(const CConfigDBHook&) = delete; // Point the moved-from hook to end, which is checked for in UnregisterHook, // to avoid a double-erase error. - CConfigDBHook(CConfigDBHook&& h) : m_ConfigDB(h.m_ConfigDB) { m_Ptr = std::move(h.m_Ptr); h.m_Ptr = m_ConfigDB.m_Hooks.end(); } - CConfigDBHook(const CConfigDBHook&) = delete; + CConfigDBHook(CConfigDBHook&& h) + : m_ConfigDB(h.m_ConfigDB), m_Valid(std::move(h.m_Valid)) + { + m_Ptr = std::move(h.m_Ptr); + h.m_Ptr = m_ConfigDB.m_Hooks.end(); + } + // Unregisters the hook if the reference is still valid. + ~CConfigDBHook() + { + if (m_Valid.expired()) + return; + m_ConfigDB.UnregisterHook(std::move(*this)); + } private: - CConfigDBHook(CConfigDB& cdb, std::multimap>::iterator p) : m_ConfigDB(cdb), m_Ptr(p) {}; + CConfigDBHook(CConfigDB& cdb, std::multimap>::iterator p) + : m_ConfigDB(cdb), m_Ptr(p), m_Valid(cdb.m_HookValidityToken) + {}; + // Hooks might be stored in objects that outlive the ConfigDB that created them. + // This makes sure we don't attempt to access deleted objects. + std::weak_ptr m_Valid; std::multimap>::iterator m_Ptr; CConfigDB& m_ConfigDB; }; Index: source/ps/ConfigDB.cpp =================================================================== --- source/ps/ConfigDB.cpp +++ source/ps/ConfigDB.cpp @@ -89,11 +89,6 @@ } // anonymous namespace typedef std::map TConfigMap; -TConfigMap CConfigDB::m_Map[CFG_LAST]; -VfsPath CConfigDB::m_ConfigFile[CFG_LAST]; -bool CConfigDB::m_HasChanges[CFG_LAST]; - -std::multimap> CConfigDB::m_Hooks; #define GETVAL(type)\ void CConfigDB::GetValue(EConfigNamespace ns, const CStr& name, type& value)\ @@ -126,6 +121,37 @@ GETVAL(std::string) #undef GETVAL +std::unique_ptr g_ConfigDBPtr; + +void CConfigDB::Initialise() +{ + g_ConfigDBPtr = std::make_unique(); +} + +void CConfigDB::Shutdown() +{ + g_ConfigDBPtr.reset(); +} + +bool CConfigDB::IsInitialised() +{ + return !!g_ConfigDBPtr; +} + +CConfigDB* CConfigDB::Instance() +{ + return g_ConfigDBPtr.get(); +} + +CConfigDB::CConfigDB() +{ + m_HookValidityToken = std::make_shared(true); +} + +CConfigDB::~CConfigDB() +{ +} + bool CConfigDB::HasChanges(EConfigNamespace ns) const { CHECK_NS(false); @@ -147,7 +173,7 @@ CHECK_NS(;); std::lock_guard s(g_ConfigDBMutex); - TConfigMap::iterator it = m_Map[CFG_COMMAND].find(name); + TConfigMap::const_iterator it = m_Map[CFG_COMMAND].find(name); if (it != m_Map[CFG_COMMAND].end()) { values = it->second; @@ -170,7 +196,7 @@ CHECK_NS(CFG_LAST); std::lock_guard s(g_ConfigDBMutex); - TConfigMap::iterator it = m_Map[CFG_COMMAND].find(name); + TConfigMap::const_iterator it = m_Map[CFG_COMMAND].find(name); if (it != m_Map[CFG_COMMAND].end()) return CFG_COMMAND; @@ -496,8 +522,12 @@ void CConfigDB::UnregisterHook(CConfigDBHook&& hook) { - if (hook.m_Ptr != m_Hooks.end()) + if (!hook.m_Valid.expired() && hook.m_Ptr != m_Hooks.end()) + { m_Hooks.erase(hook.m_Ptr); + hook.m_Ptr = m_Hooks.end(); + hook.m_Valid.reset(); + } } void CConfigDB::UnregisterHook(std::unique_ptr hook) Index: source/ps/GameSetup/Config.cpp =================================================================== --- source/ps/GameSetup/Config.cpp +++ source/ps/GameSetup/Config.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2020 Wildfire Games. +/* Copyright (C) 2021 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -126,7 +126,7 @@ { TIMER(L"CONFIG_Init"); - new CConfigDB; + CConfigDB::Initialise(); // Load the global, default config file g_ConfigDB.SetConfigFile(CFG_DEFAULT, L"config/default.cfg"); Index: source/ps/GameSetup/GameSetup.cpp =================================================================== --- source/ps/GameSetup/GameSetup.cpp +++ source/ps/GameSetup/GameSetup.cpp @@ -719,7 +719,7 @@ from_config: TIMER_BEGIN(L"shutdown ConfigDB"); - delete &g_ConfigDB; + CConfigDB::Shutdown(); TIMER_END(L"shutdown ConfigDB"); SAFE_DELETE(g_Console); Index: source/ps/Joystick.cpp =================================================================== --- source/ps/Joystick.cpp +++ source/ps/Joystick.cpp @@ -1,4 +1,4 @@ -/* Copyright (C) 2014 Wildfire Games. +/* Copyright (C) 2021 Wildfire Games. * This file is part of 0 A.D. * * 0 A.D. is free software: you can redistribute it and/or modify @@ -33,6 +33,8 @@ void CJoystick::Initialise() { bool joystickEnable = false; + if (!CConfigDB::IsInitialised()) + return; CFG_GET_VAL("joystick.enable", joystickEnable); if (!joystickEnable) return; Index: source/ps/tests/test_ConfigDB.h =================================================================== --- source/ps/tests/test_ConfigDB.h +++ source/ps/tests/test_ConfigDB.h @@ -24,7 +24,7 @@ class TestConfigDB : public CxxTest::TestSuite { - CConfigDB* configDB; + std::unique_ptr configDB; public: void setUp() @@ -32,15 +32,14 @@ g_VFS = CreateVfs(); TS_ASSERT_OK(g_VFS->Mount(L"config", DataDir() / "_testconfig" / "")); - configDB = new CConfigDB; + configDB = std::make_unique(); } void tearDown() { DeleteDirectory(DataDir()/"_testconfig"); g_VFS.reset(); - - delete configDB; + configDB.reset(); } void test_setting_int() Index: source/ps/tests/test_Hotkeys.h =================================================================== --- source/ps/tests/test_Hotkeys.h +++ source/ps/tests/test_Hotkeys.h @@ -31,7 +31,7 @@ class TestHotkey : public CxxTest::TestSuite { - CConfigDB* configDB; + std::unique_ptr configDB; // Stores whether one of these was sent in the last fakeInput call. bool hotkeyPress = false; bool hotkeyUp = false; @@ -64,14 +64,14 @@ TS_ASSERT_OK(g_VFS->Mount(L"config", DataDir() / "_testconfig" / "")); TS_ASSERT_OK(g_VFS->Mount(L"cache", DataDir() / "_testcache" / "")); - configDB = new CConfigDB; + configDB = std::make_unique(); g_scancodes = {}; } void tearDown() { - delete configDB; + configDB.reset(); g_VFS.reset(); DeleteDirectory(DataDir()/"_testcache"); DeleteDirectory(DataDir()/"_testconfig"); Index: source/renderer/RenderingOptions.h =================================================================== --- source/renderer/RenderingOptions.h +++ source/renderer/RenderingOptions.h @@ -27,6 +27,7 @@ #ifndef INCLUDED_RENDERINGOPTIONS #define INCLUDED_RENDERINGOPTIONS +class CConfigDB; class CStr8; class CRenderer; Index: source/renderer/RenderingOptions.cpp =================================================================== --- source/renderer/RenderingOptions.cpp +++ source/renderer/RenderingOptions.cpp @@ -185,9 +185,6 @@ void CRenderingOptions::ClearHooks() { - if (CConfigDB::IsInitialised()) - for (CConfigDBHook& hook : *m_ConfigHooks) - g_ConfigDB.UnregisterHook(std::move(hook)); m_ConfigHooks->clear(); }