Merge pull request #11880 from iwubcode/race_condition_asset

VideoCommon: prevent potential data issue when reloading Asset data
dolphin-emu · Jun 3, 2023 · 3245786 · 3245786
2 parents d03e09c + 58d383b
commit 3245786
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 6 deletions.
diff --git a/Source/Core/VideoCommon/Assets/CustomAsset.h b/Source/Core/VideoCommon/Assets/CustomAsset.h
@@ -64,7 +64,11 @@ class CustomLoadableAsset : public CustomAsset
 public:
   using CustomAsset::CustomAsset;
 
-  const UnderlyingType* GetData() const
+  // Callees should understand that the type returned is
+  // a local copy and 'GetData()' needs to be called
+  // to ensure the latest copy is available if
+  // they want to handle reloads
+  [[nodiscard]] std::shared_ptr<UnderlyingType> GetData() const
   {
     std::lock_guard lk(m_lock);
     if (m_loaded)
@@ -75,7 +79,7 @@ class CustomLoadableAsset : public CustomAsset
 protected:
   bool m_loaded = false;
   mutable std::mutex m_lock;
-  UnderlyingType m_data;
+  std::shared_ptr<UnderlyingType> m_data;
 };
 
 }  // namespace VideoCommon
diff --git a/Source/Core/VideoCommon/Assets/TextureAsset.cpp b/Source/Core/VideoCommon/Assets/TextureAsset.cpp
@@ -10,20 +10,24 @@ namespace VideoCommon
 CustomAssetLibrary::LoadInfo RawTextureAsset::LoadImpl(const CustomAssetLibrary::AssetID& asset_id)
 {
   std::lock_guard lk(m_lock);
-  const auto loaded_info = m_owning_library->LoadTexture(asset_id, &m_data);
+  auto potential_data = std::make_shared<CustomTextureData>();
+  const auto loaded_info = m_owning_library->LoadTexture(asset_id, potential_data.get());
   if (loaded_info.m_bytes_loaded == 0)
     return {};
   m_loaded = true;
+  m_data = std::move(potential_data);
   return loaded_info;
 }
 
 CustomAssetLibrary::LoadInfo GameTextureAsset::LoadImpl(const CustomAssetLibrary::AssetID& asset_id)
 {
   std::lock_guard lk(m_lock);
-  const auto loaded_info = m_owning_library->LoadGameTexture(asset_id, &m_data);
+  auto potential_data = std::make_shared<CustomTextureData>();
+  const auto loaded_info = m_owning_library->LoadGameTexture(asset_id, potential_data.get());
   if (loaded_info.m_bytes_loaded == 0)
     return {};
   m_loaded = true;
+  m_data = std::move(potential_data);
   return loaded_info;
 }
 
@@ -39,7 +43,7 @@ bool GameTextureAsset::Validate(u32 native_width, u32 native_height) const
     return false;
   }
 
-  if (m_data.m_levels.empty())
+  if (m_data->m_levels.empty())
   {
     ERROR_LOG_FMT(VIDEO,
                   "Game texture can't be validated for asset '{}' because no data was available.",
@@ -49,7 +53,7 @@ bool GameTextureAsset::Validate(u32 native_width, u32 native_height) const
 
   // Verify that the aspect ratio of the texture hasn't changed, as this could have
   // side-effects.
-  const VideoCommon::CustomTextureData::Level& first_mip = m_data.m_levels[0];
+  const VideoCommon::CustomTextureData::Level& first_mip = m_data->m_levels[0];
   if (first_mip.width * native_height != first_mip.height * native_width)
   {
     ERROR_LOG_FMT(