VideoCommon: avoid race conditions with asset load/unload by moving the lock to the entire function, favor atomics for the memory/time getters

This commit is contained in:
iwubcode
2025-06-06 19:55:03 -05:00
parent b3f50c969e
commit 774a84a953
2 changed files with 8 additions and 15 deletions

View File

@ -13,6 +13,7 @@ CustomAsset::CustomAsset(std::shared_ptr<CustomAssetLibrary> library,
std::size_t CustomAsset::Load() std::size_t CustomAsset::Load()
{ {
std::lock_guard lk(m_info_lock);
// The load time needs to come from before the data is actually read. // The load time needs to come from before the data is actually read.
// Using a time point from after the read marks the asset as more up-to-date than it actually is, // Using a time point from after the read marks the asset as more up-to-date than it actually is,
// and has potential to race (and not be updated) if a change happens immediately after load. // and has potential to race (and not be updated) if a change happens immediately after load.
@ -21,7 +22,6 @@ std::size_t CustomAsset::Load()
const auto load_information = LoadImpl(m_asset_id); const auto load_information = LoadImpl(m_asset_id);
if (load_information.bytes_loaded > 0) if (load_information.bytes_loaded > 0)
{ {
std::lock_guard lk(m_info_lock);
m_bytes_loaded = load_information.bytes_loaded; m_bytes_loaded = load_information.bytes_loaded;
m_last_loaded_time = load_time; m_last_loaded_time = load_time;
return m_bytes_loaded; return m_bytes_loaded;
@ -31,19 +31,13 @@ std::size_t CustomAsset::Load()
std::size_t CustomAsset::Unload() std::size_t CustomAsset::Unload()
{ {
std::lock_guard lk(m_info_lock);
UnloadImpl(); UnloadImpl();
std::size_t bytes_loaded = 0; return m_bytes_loaded.exchange(0);
{
std::lock_guard lk(m_info_lock);
bytes_loaded = m_bytes_loaded;
m_bytes_loaded = 0;
}
return bytes_loaded;
} }
const CustomAsset::TimeType& CustomAsset::GetLastLoadedTime() const CustomAsset::TimeType CustomAsset::GetLastLoadedTime() const
{ {
std::lock_guard lk(m_info_lock);
return m_last_loaded_time; return m_last_loaded_time;
} }
@ -59,7 +53,6 @@ const CustomAssetLibrary::AssetID& CustomAsset::GetAssetId() const
std::size_t CustomAsset::GetByteSizeInMemory() const std::size_t CustomAsset::GetByteSizeInMemory() const
{ {
std::lock_guard lk(m_info_lock);
return m_bytes_loaded; return m_bytes_loaded;
} }

View File

@ -6,9 +6,9 @@
#include "Common/CommonTypes.h" #include "Common/CommonTypes.h"
#include "VideoCommon/Assets/CustomAssetLibrary.h" #include "VideoCommon/Assets/CustomAssetLibrary.h"
#include <atomic>
#include <memory> #include <memory>
#include <mutex> #include <mutex>
#include <optional>
namespace VideoCommon namespace VideoCommon
{ {
@ -36,7 +36,7 @@ public:
std::size_t Unload(); std::size_t Unload();
// Returns the time that the data was last loaded // Returns the time that the data was last loaded
const TimeType& GetLastLoadedTime() const; TimeType GetLastLoadedTime() const;
// Returns an id that uniquely identifies this asset // Returns an id that uniquely identifies this asset
const CustomAssetLibrary::AssetID& GetAssetId() const; const CustomAssetLibrary::AssetID& GetAssetId() const;
@ -60,8 +60,8 @@ private:
std::size_t m_handle; std::size_t m_handle;
mutable std::mutex m_info_lock; mutable std::mutex m_info_lock;
std::size_t m_bytes_loaded = 0; std::atomic<std::size_t> m_bytes_loaded = 0;
TimeType m_last_loaded_time = {}; std::atomic<TimeType> m_last_loaded_time = {};
}; };
// An abstract class that is expected to // An abstract class that is expected to