Fix race conditions in Config Layers
API has been made stricter, layers are now managed with shared pointers, so using them temporarily increased their reference counters. Additionally, any s_layers map has been guarded by a read/write lock, as concurrent write/reads to it were possible.
This commit is contained in:
parent
dea2b9c509
commit
cb4eecde52
|
@ -5,51 +5,78 @@
|
||||||
#include <algorithm>
|
#include <algorithm>
|
||||||
#include <list>
|
#include <list>
|
||||||
#include <map>
|
#include <map>
|
||||||
|
#if __APPLE__
|
||||||
|
#include <mutex>
|
||||||
|
#else
|
||||||
|
#include <shared_mutex>
|
||||||
|
#endif
|
||||||
|
|
||||||
#include "Common/Config/Config.h"
|
#include "Common/Config/Config.h"
|
||||||
|
|
||||||
namespace Config
|
namespace Config
|
||||||
{
|
{
|
||||||
|
using Layers = std::map<LayerType, std::shared_ptr<Layer>>;
|
||||||
|
|
||||||
static Layers s_layers;
|
static Layers s_layers;
|
||||||
static std::list<ConfigChangedCallback> s_callbacks;
|
static std::list<ConfigChangedCallback> s_callbacks;
|
||||||
static u32 s_callback_guards = 0;
|
static u32 s_callback_guards = 0;
|
||||||
|
|
||||||
Layers* GetLayers()
|
// Mac supports shared_mutex since 10.12 and we're targeting 10.10,
|
||||||
{
|
// so only use unique locks there...
|
||||||
return &s_layers;
|
#if __APPLE__
|
||||||
}
|
static std::mutex s_layers_rw_lock;
|
||||||
|
|
||||||
void AddLayer(std::unique_ptr<Layer> layer)
|
using ReadLock = std::unique_lock<std::mutex>;
|
||||||
|
using WriteLock = std::unique_lock<std::mutex>;
|
||||||
|
#else
|
||||||
|
static std::shared_mutex s_layers_rw_lock;
|
||||||
|
|
||||||
|
using ReadLock = std::shared_lock<std::shared_mutex>;
|
||||||
|
using WriteLock = std::unique_lock<std::shared_mutex>;
|
||||||
|
#endif
|
||||||
|
|
||||||
|
void AddLayerInternal(std::shared_ptr<Layer> layer)
|
||||||
{
|
{
|
||||||
s_layers[layer->GetLayer()] = std::move(layer);
|
{
|
||||||
|
WriteLock lock(s_layers_rw_lock);
|
||||||
|
|
||||||
|
const Config::LayerType layer_type = layer->GetLayer();
|
||||||
|
s_layers.insert_or_assign(layer_type, std::move(layer));
|
||||||
|
}
|
||||||
InvokeConfigChangedCallbacks();
|
InvokeConfigChangedCallbacks();
|
||||||
}
|
}
|
||||||
|
|
||||||
void AddLayer(std::unique_ptr<ConfigLayerLoader> loader)
|
void AddLayer(std::unique_ptr<ConfigLayerLoader> loader)
|
||||||
{
|
{
|
||||||
AddLayer(std::make_unique<Layer>(std::move(loader)));
|
AddLayerInternal(std::make_shared<Layer>(std::move(loader)));
|
||||||
}
|
}
|
||||||
|
|
||||||
Layer* GetLayer(LayerType layer)
|
std::shared_ptr<Layer> GetLayer(LayerType layer)
|
||||||
{
|
{
|
||||||
if (!LayerExists(layer))
|
ReadLock lock(s_layers_rw_lock);
|
||||||
return nullptr;
|
|
||||||
return s_layers[layer].get();
|
std::shared_ptr<Layer> result;
|
||||||
|
const auto it = s_layers.find(layer);
|
||||||
|
if (it != s_layers.end())
|
||||||
|
{
|
||||||
|
result = it->second;
|
||||||
|
}
|
||||||
|
return result;
|
||||||
}
|
}
|
||||||
|
|
||||||
void RemoveLayer(LayerType layer)
|
void RemoveLayer(LayerType layer)
|
||||||
{
|
{
|
||||||
s_layers.erase(layer);
|
{
|
||||||
|
WriteLock lock(s_layers_rw_lock);
|
||||||
|
|
||||||
|
s_layers.erase(layer);
|
||||||
|
}
|
||||||
InvokeConfigChangedCallbacks();
|
InvokeConfigChangedCallbacks();
|
||||||
}
|
}
|
||||||
bool LayerExists(LayerType layer)
|
|
||||||
{
|
|
||||||
return s_layers.find(layer) != s_layers.end();
|
|
||||||
}
|
|
||||||
|
|
||||||
void AddConfigChangedCallback(ConfigChangedCallback func)
|
void AddConfigChangedCallback(ConfigChangedCallback func)
|
||||||
{
|
{
|
||||||
s_callbacks.emplace_back(func);
|
s_callbacks.emplace_back(std::move(func));
|
||||||
}
|
}
|
||||||
|
|
||||||
void InvokeConfigChangedCallbacks()
|
void InvokeConfigChangedCallbacks()
|
||||||
|
@ -64,15 +91,23 @@ void InvokeConfigChangedCallbacks()
|
||||||
// Explicit load and save of layers
|
// Explicit load and save of layers
|
||||||
void Load()
|
void Load()
|
||||||
{
|
{
|
||||||
for (auto& layer : s_layers)
|
{
|
||||||
layer.second->Load();
|
ReadLock lock(s_layers_rw_lock);
|
||||||
|
|
||||||
|
for (auto& layer : s_layers)
|
||||||
|
layer.second->Load();
|
||||||
|
}
|
||||||
InvokeConfigChangedCallbacks();
|
InvokeConfigChangedCallbacks();
|
||||||
}
|
}
|
||||||
|
|
||||||
void Save()
|
void Save()
|
||||||
{
|
{
|
||||||
for (auto& layer : s_layers)
|
{
|
||||||
layer.second->Save();
|
ReadLock lock(s_layers_rw_lock);
|
||||||
|
|
||||||
|
for (auto& layer : s_layers)
|
||||||
|
layer.second->Save();
|
||||||
|
}
|
||||||
InvokeConfigChangedCallbacks();
|
InvokeConfigChangedCallbacks();
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -84,13 +119,17 @@ void Init()
|
||||||
|
|
||||||
void Shutdown()
|
void Shutdown()
|
||||||
{
|
{
|
||||||
|
WriteLock lock(s_layers_rw_lock);
|
||||||
|
|
||||||
s_layers.clear();
|
s_layers.clear();
|
||||||
s_callbacks.clear();
|
s_callbacks.clear();
|
||||||
}
|
}
|
||||||
|
|
||||||
void ClearCurrentRunLayer()
|
void ClearCurrentRunLayer()
|
||||||
{
|
{
|
||||||
s_layers[LayerType::CurrentRun] = std::make_unique<Layer>(LayerType::CurrentRun);
|
WriteLock lock(s_layers_rw_lock);
|
||||||
|
|
||||||
|
s_layers.insert_or_assign(LayerType::CurrentRun, std::make_shared<Layer>(LayerType::CurrentRun));
|
||||||
}
|
}
|
||||||
|
|
||||||
static const std::map<System, std::string> system_to_name = {
|
static const std::map<System, std::string> system_to_name = {
|
||||||
|
@ -129,13 +168,16 @@ const std::string& GetLayerName(LayerType layer)
|
||||||
|
|
||||||
LayerType GetActiveLayerForConfig(const ConfigLocation& config)
|
LayerType GetActiveLayerForConfig(const ConfigLocation& config)
|
||||||
{
|
{
|
||||||
|
ReadLock lock(s_layers_rw_lock);
|
||||||
|
|
||||||
for (auto layer : SEARCH_ORDER)
|
for (auto layer : SEARCH_ORDER)
|
||||||
{
|
{
|
||||||
if (!LayerExists(layer))
|
const auto it = s_layers.find(layer);
|
||||||
continue;
|
if (it != s_layers.end())
|
||||||
|
{
|
||||||
if (GetLayer(layer)->Exists(config))
|
if (it->second->Exists(config))
|
||||||
return layer;
|
return layer;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// If config is not present in any layer, base layer is considered active.
|
// If config is not present in any layer, base layer is considered active.
|
||||||
|
|
|
@ -16,16 +16,12 @@
|
||||||
|
|
||||||
namespace Config
|
namespace Config
|
||||||
{
|
{
|
||||||
using Layers = std::map<LayerType, std::unique_ptr<Layer>>;
|
|
||||||
using ConfigChangedCallback = std::function<void()>;
|
using ConfigChangedCallback = std::function<void()>;
|
||||||
|
|
||||||
// Layer management
|
// Layer management
|
||||||
Layers* GetLayers();
|
|
||||||
void AddLayer(std::unique_ptr<Layer> layer);
|
|
||||||
void AddLayer(std::unique_ptr<ConfigLayerLoader> loader);
|
void AddLayer(std::unique_ptr<ConfigLayerLoader> loader);
|
||||||
Layer* GetLayer(LayerType layer);
|
std::shared_ptr<Layer> GetLayer(LayerType layer);
|
||||||
void RemoveLayer(LayerType layer);
|
void RemoveLayer(LayerType layer);
|
||||||
bool LayerExists(LayerType layer);
|
|
||||||
|
|
||||||
void AddConfigChangedCallback(ConfigChangedCallback func);
|
void AddConfigChangedCallback(ConfigChangedCallback func);
|
||||||
void InvokeConfigChangedCallbacks();
|
void InvokeConfigChangedCallbacks();
|
||||||
|
|
Loading…
Reference in New Issue