Index: ps/trunk/source/graphics/ObjectManager.cpp =================================================================== --- ps/trunk/source/graphics/ObjectManager.cpp +++ ps/trunk/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: ps/trunk/source/gui/tests/test_GuiManager.h =================================================================== --- ps/trunk/source/gui/tests/test_GuiManager.h +++ ps/trunk/source/gui/tests/test_GuiManager.h @@ -26,9 +26,11 @@ #include "ps/Hotkey.h" #include "ps/XML/Xeromyces.h" +#include + class TestGuiManager : public CxxTest::TestSuite { - CConfigDB* configDB; + std::unique_ptr configDB; public: void setUp() @@ -37,7 +39,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 +50,7 @@ { delete g_GUI; CXeromyces::Terminate(); - delete configDB; + configDB.reset(); g_VFS.reset(); DeleteDirectory(DataDir()/"_testcache"); } @@ -189,7 +191,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: ps/trunk/source/ps/ConfigDB.h =================================================================== --- ps/trunk/source/ps/ConfigDB.h +++ ps/trunk/source/ps/ConfigDB.h @@ -29,9 +29,11 @@ #include "lib/file/vfs/vfs_path.h" #include "ps/CStr.h" -#include "ps/Singleton.h" +#include #include +#include +#include #include /** @@ -59,12 +61,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 +190,22 @@ * 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; + std::array m_ConfigFile; + std::array m_HasChanges; + + mutable std::recursive_mutex m_Mutex; }; class CConfigDBHook @@ -196,12 +213,23 @@ 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_Ptr = std::move(h.m_Ptr); + h.m_Ptr = m_ConfigDB.m_Hooks.end(); + } + // Unregisters the hook. Must be called before the original ConfigDB gets deleted. + ~CConfigDBHook() + { + 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) + {}; std::multimap>::iterator m_Ptr; CConfigDB& m_ConfigDB; Index: ps/trunk/source/ps/ConfigDB.cpp =================================================================== --- ps/trunk/source/ps/ConfigDB.cpp +++ ps/trunk/source/ps/ConfigDB.cpp @@ -37,8 +37,6 @@ std::for_each(hooks.lower_bound(name), hooks.upper_bound(name), [](const std::pair>& hook) { hook.second(); }); } -std::recursive_mutex g_ConfigDBMutex; - // These entries will not be printed to logfiles, so that logfiles can be shared without leaking personal or sensitive data const std::unordered_set g_UnloggedEntries = { "lobby.password", @@ -89,17 +87,12 @@ } // 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)\ {\ CHECK_NS(;);\ - std::lock_guard s(g_ConfigDBMutex);\ + std::lock_guard s(m_Mutex);\ TConfigMap::iterator it = m_Map[CFG_COMMAND].find(name);\ if (it != m_Map[CFG_COMMAND].end())\ {\ @@ -126,11 +119,41 @@ 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() +{ +} + +CConfigDB::~CConfigDB() +{ +} + bool CConfigDB::HasChanges(EConfigNamespace ns) const { CHECK_NS(false); - std::lock_guard s(g_ConfigDBMutex); + std::lock_guard s(m_Mutex); return m_HasChanges[ns]; } @@ -138,7 +161,7 @@ { CHECK_NS(;); - std::lock_guard s(g_ConfigDBMutex); + std::lock_guard s(m_Mutex); m_HasChanges[ns] = value; } @@ -146,8 +169,8 @@ { CHECK_NS(;); - std::lock_guard s(g_ConfigDBMutex); - TConfigMap::iterator it = m_Map[CFG_COMMAND].find(name); + std::lock_guard s(m_Mutex); + TConfigMap::const_iterator it = m_Map[CFG_COMMAND].find(name); if (it != m_Map[CFG_COMMAND].end()) { values = it->second; @@ -169,8 +192,8 @@ { CHECK_NS(CFG_LAST); - std::lock_guard s(g_ConfigDBMutex); - TConfigMap::iterator it = m_Map[CFG_COMMAND].find(name); + std::lock_guard s(m_Mutex); + TConfigMap::const_iterator it = m_Map[CFG_COMMAND].find(name); if (it != m_Map[CFG_COMMAND].end()) return CFG_COMMAND; @@ -186,7 +209,7 @@ std::map CConfigDB::GetValuesWithPrefix(EConfigNamespace ns, const CStr& prefix) const { - std::lock_guard s(g_ConfigDBMutex); + std::lock_guard s(m_Mutex); std::map ret; CHECK_NS(ret); @@ -209,7 +232,7 @@ { CHECK_NS(;); - std::lock_guard s(g_ConfigDBMutex); + std::lock_guard s(m_Mutex); TConfigMap::iterator it = m_Map[ns].find(name); if (it == m_Map[ns].end()) it = m_Map[ns].insert(m_Map[ns].begin(), make_pair(name, CConfigValueSet(1))); @@ -232,7 +255,7 @@ { CHECK_NS(;); - std::lock_guard s(g_ConfigDBMutex); + std::lock_guard s(m_Mutex); TConfigMap::iterator it = m_Map[ns].find(name); if (it == m_Map[ns].end()) it = m_Map[ns].insert(m_Map[ns].begin(), make_pair(name, CConfigValueSet(1))); @@ -244,7 +267,7 @@ { CHECK_NS(;); - std::lock_guard s(g_ConfigDBMutex); + std::lock_guard s(m_Mutex); TConfigMap::iterator it = m_Map[ns].find(name); if (it == m_Map[ns].end()) return; @@ -257,7 +280,7 @@ { CHECK_NS(;); - std::lock_guard s(g_ConfigDBMutex); + std::lock_guard s(m_Mutex); m_ConfigFile[ns] = path; } @@ -265,7 +288,7 @@ { CHECK_NS(false); - std::lock_guard s(g_ConfigDBMutex); + std::lock_guard s(m_Mutex); shared_ptr buffer; size_t buflen; @@ -428,7 +451,7 @@ { CHECK_NS(false); - std::lock_guard s(g_ConfigDBMutex); + std::lock_guard s(m_Mutex); return WriteFile(ns, m_ConfigFile[ns]); } @@ -436,7 +459,7 @@ { CHECK_NS(false); - std::lock_guard s(g_ConfigDBMutex); + std::lock_guard s(m_Mutex); shared_ptr buf; AllocateAligned(buf, 1*MiB, maxSectorSize); char* pos = (char*)buf.get(); @@ -468,7 +491,7 @@ { CHECK_NS(false); - std::lock_guard s(g_ConfigDBMutex); + std::lock_guard s(m_Mutex); return WriteValueToFile(ns, name, value, m_ConfigFile[ns]); } @@ -476,7 +499,7 @@ { CHECK_NS(false); - std::lock_guard s(g_ConfigDBMutex); + std::lock_guard s(m_Mutex); TConfigMap newMap; m_Map[ns].swap(newMap); @@ -491,13 +514,18 @@ CConfigDBHook CConfigDB::RegisterHookAndCall(const CStr& name, std::function hook) { hook(); + std::lock_guard s(m_Mutex); return CConfigDBHook(*this, m_Hooks.emplace(name, hook)); } void CConfigDB::UnregisterHook(CConfigDBHook&& hook) { if (hook.m_Ptr != m_Hooks.end()) + { + std::lock_guard s(m_Mutex); m_Hooks.erase(hook.m_Ptr); + hook.m_Ptr = m_Hooks.end(); + } } void CConfigDB::UnregisterHook(std::unique_ptr hook) Index: ps/trunk/source/ps/GameSetup/Config.cpp =================================================================== --- ps/trunk/source/ps/GameSetup/Config.cpp +++ ps/trunk/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: ps/trunk/source/ps/GameSetup/GameSetup.cpp =================================================================== --- ps/trunk/source/ps/GameSetup/GameSetup.cpp +++ ps/trunk/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: ps/trunk/source/ps/Joystick.cpp =================================================================== --- ps/trunk/source/ps/Joystick.cpp +++ ps/trunk/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: ps/trunk/source/ps/tests/test_ConfigDB.h =================================================================== --- ps/trunk/source/ps/tests/test_ConfigDB.h +++ ps/trunk/source/ps/tests/test_ConfigDB.h @@ -20,11 +20,13 @@ #include "lib/file/vfs/vfs.h" #include "ps/ConfigDB.h" +#include + extern PIVFS g_VFS; class TestConfigDB : public CxxTest::TestSuite { - CConfigDB* configDB; + std::unique_ptr configDB; public: void setUp() @@ -32,15 +34,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: ps/trunk/source/ps/tests/test_Hotkeys.h =================================================================== --- ps/trunk/source/ps/tests/test_Hotkeys.h +++ ps/trunk/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: ps/trunk/source/renderer/RenderingOptions.h =================================================================== --- ps/trunk/source/renderer/RenderingOptions.h +++ ps/trunk/source/renderer/RenderingOptions.h @@ -27,6 +27,7 @@ #ifndef INCLUDED_RENDERINGOPTIONS #define INCLUDED_RENDERINGOPTIONS +class CConfigDB; class CStr8; class CRenderer; Index: ps/trunk/source/renderer/RenderingOptions.cpp =================================================================== --- ps/trunk/source/renderer/RenderingOptions.cpp +++ ps/trunk/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(); }