Improve game cache locking

The mutex in the game cache wasn't very effective, instead move the
mutex out to the only place that concurrently writes to the cache.

This does mean that there isn't protection against calling
GameInterface::LoadPlugins() concurrently from multiple threads, but
no effort has been made to make libloot's public API thread-safe anyway.
This commit is contained in:
Oliver Hamlet
2025-03-02 22:40:47 +00:00
parent 3365a880e0
commit ccd4e41a99
3 changed files with 3 additions and 64 deletions
+3
View File
@@ -263,6 +263,7 @@ void Game::LoadPlugins(const std::vector<std::filesystem::path>& pluginPaths,
logger->trace("Starting plugin loading.");
}
std::mutex mutex;
std::for_each(
std::execution::par_unseq,
pluginPaths.begin(),
@@ -272,6 +273,8 @@ void Game::LoadPlugins(const std::vector<std::filesystem::path>& pluginPaths,
const auto resolvedPluginPath =
ResolvePluginPath(GetType(), DataPath(), pluginPath);
std::lock_guard<std::mutex> lock(mutex);
cache_.AddPlugin(
Plugin(GetType(), cache_, resolvedPluginPath, loadHeadersOnly));
} catch (const std::exception& e) {
-53
View File
@@ -24,53 +24,10 @@
#include "api/game/game_cache.h"
#include <thread>
#include "api/helpers/text.h"
using std::lock_guard;
using std::mutex;
namespace loot {
GameCache::GameCache(const GameCache& cache) {
lock_guard<mutex> lock(cache.mutex_);
plugins_ = cache.plugins_;
archivePaths_ = cache.archivePaths_;
}
GameCache::GameCache(GameCache&& cache) {
lock_guard<mutex> lock(cache.mutex_);
plugins_ = std::move(cache.plugins_);
archivePaths_ = std::move(cache.archivePaths_);
}
GameCache& GameCache::operator=(const GameCache& cache) {
if (&cache != this) {
std::scoped_lock lock(mutex_, cache.mutex_);
plugins_ = cache.plugins_;
archivePaths_ = cache.archivePaths_;
}
return *this;
}
GameCache& GameCache::operator=(GameCache&& cache) {
if (&cache != this) {
std::scoped_lock lock(mutex_, cache.mutex_);
plugins_ = std::move(cache.plugins_);
archivePaths_ = std::move(cache.archivePaths_);
}
return *this;
}
std::vector<const Plugin*> GameCache::GetPlugins() const {
lock_guard<mutex> lock(mutex_);
std::vector<const Plugin*> output(plugins_.size());
std::transform(
begin(plugins_), end(plugins_), begin(output), [](const auto& pair) {
@@ -80,8 +37,6 @@ std::vector<const Plugin*> GameCache::GetPlugins() const {
}
const Plugin* GameCache::GetPlugin(const std::string& pluginName) const {
lock_guard<mutex> lock(mutex_);
const auto it = plugins_.find(NormalizeFilename(pluginName));
if (it != end(plugins_))
return it->second.get();
@@ -90,8 +45,6 @@ const Plugin* GameCache::GetPlugin(const std::string& pluginName) const {
}
void GameCache::AddPlugin(Plugin&& plugin) {
lock_guard<mutex> lock(mutex_);
auto normalizedName = NormalizeFilename(plugin.GetName());
auto pluginPointer = std::make_shared<Plugin>(std::move(plugin));
@@ -104,20 +57,14 @@ void GameCache::AddPlugin(Plugin&& plugin) {
}
std::set<std::filesystem::path> GameCache::GetArchivePaths() const {
lock_guard<mutex> lock(mutex_);
return archivePaths_;
}
void GameCache::CacheArchivePaths(std::set<std::filesystem::path>&& paths) {
lock_guard<mutex> lock(mutex_);
archivePaths_ = std::move(paths);
}
void GameCache::ClearCachedPlugins() {
lock_guard<mutex> guard(mutex_);
plugins_.clear();
}
}
-11
View File
@@ -25,7 +25,6 @@
#ifndef LOOT_API_GAME_GAME_CACHE
#define LOOT_API_GAME_GAME_CACHE
#include <mutex>
#include <string>
#include <unordered_map>
@@ -34,14 +33,6 @@
namespace loot {
class GameCache {
public:
GameCache() = default;
GameCache(const GameCache& cache);
GameCache(GameCache&& cache);
~GameCache() = default;
GameCache& operator=(const GameCache& cache);
GameCache& operator=(GameCache&& cache);
std::vector<const Plugin*> GetPlugins() const;
const Plugin* GetPlugin(const std::string& pluginName) const;
void AddPlugin(Plugin&& plugin);
@@ -54,8 +45,6 @@ public:
private:
std::unordered_map<std::string, std::shared_ptr<const Plugin>> plugins_;
std::set<std::filesystem::path> archivePaths_;
mutable std::mutex mutex_;
};
}