From 51debaeb47de93edec1ba10161df74a2f7f49209 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Tue, 27 Sep 2022 18:57:42 +0200 Subject: [PATCH] Revert "Android: Don't hold gameFileCache lock during updateAdditionalMetadata" This reverts commit fb265b610de08df5509e56beeb3dbff68f7d4396. The optimization in that commit is safe when the executor thread is writing and the GUI thread is reading, but I had failed to take into account that it's unsafe when the GUI thread is writing and the executor thread is reading. (The native UpdateAdditionalMetadata function loops through m_cached_files, which is unsafe if another thread is adding elements to m_cached_files simultaneously.) Losing out on this optimization isn't too bad, because 719930bb390ed0020b99a7942289d83218e99d69 makes it very unlikely that both threads will want the lock at the same time. --- .../services/GameFileCacheManager.java | 29 +++++++++---------- Source/CMakeLists.txt | 2 -- Source/Core/UICommon/GameFileCache.cpp | 3 +- Source/VSProps/Base.Dolphin.props | 2 -- 4 files changed, 15 insertions(+), 21 deletions(-) diff --git a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/services/GameFileCacheManager.java b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/services/GameFileCacheManager.java index b359915901..4e883413b4 100644 --- a/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/services/GameFileCacheManager.java +++ b/Source/Android/app/src/main/java/org/dolphinemu/dolphinemu/services/GameFileCacheManager.java @@ -227,25 +227,24 @@ public final class GameFileCacheManager { String[] gamePaths = GameFileCache.getAllGamePaths(); - boolean changed; synchronized (sGameFileCache) { - changed = sGameFileCache.update(gamePaths); - } - if (changed) - { - updateGameFileArray(); - } + boolean changed = sGameFileCache.update(gamePaths); + if (changed) + { + updateGameFileArray(); + } - boolean additionalMetadataChanged = sGameFileCache.updateAdditionalMetadata(); - if (additionalMetadataChanged) - { - updateGameFileArray(); - } + boolean additionalMetadataChanged = sGameFileCache.updateAdditionalMetadata(); + if (additionalMetadataChanged) + { + updateGameFileArray(); + } - if (changed || additionalMetadataChanged) - { - sGameFileCache.save(); + if (changed || additionalMetadataChanged) + { + sGameFileCache.save(); + } } } diff --git a/Source/CMakeLists.txt b/Source/CMakeLists.txt index 7e5b548864..4a4cd61b51 100644 --- a/Source/CMakeLists.txt +++ b/Source/CMakeLists.txt @@ -13,8 +13,6 @@ if(CMAKE_SYSTEM_NAME MATCHES "Windows") add_definitions(-D_CRT_SECURE_NO_DEPRECATE) add_definitions(-D_CRT_NONSTDC_NO_WARNINGS) add_definitions(-D_SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING) - # The replacement for the old atomic shared_ptr functions was added in C++20, so we can't use it yet - add_definitions(-D_SILENCE_CXX20_OLD_SHARED_PTR_ATOMIC_SUPPORT_DEPRECATION_WARNING) endif() if (NOT MSVC) diff --git a/Source/Core/UICommon/GameFileCache.cpp b/Source/Core/UICommon/GameFileCache.cpp index f9b26d344d..59c4c9a292 100644 --- a/Source/Core/UICommon/GameFileCache.cpp +++ b/Source/Core/UICommon/GameFileCache.cpp @@ -4,7 +4,6 @@ #include "UICommon/GameFileCache.h" #include -#include #include #include #include @@ -204,7 +203,7 @@ bool GameFileCache::UpdateAdditionalMetadata(std::shared_ptr* game_fil if (custom_cover_changed) copy->CustomCoverCommit(); - std::atomic_store(game_file, std::move(copy)); + *game_file = std::move(copy); return true; } diff --git a/Source/VSProps/Base.Dolphin.props b/Source/VSProps/Base.Dolphin.props index 43af6653a1..aadb04d5dd 100644 --- a/Source/VSProps/Base.Dolphin.props +++ b/Source/VSProps/Base.Dolphin.props @@ -29,8 +29,6 @@ _WINSOCK_DEPRECATED_NO_WARNINGS;%(PreprocessorDefinitions) _SILENCE_CXX17_CODECVT_HEADER_DEPRECATION_WARNING;%(PreprocessorDefinitions) - - _SILENCE_CXX20_OLD_SHARED_PTR_ATOMIC_SUPPORT_DEPRECATION_WARNING;%(PreprocessorDefinitions) _ARCH_64=1;_M_X86=1;_M_X86_64=1;%(PreprocessorDefinitions)