FancyZones: make FancyZonesData thread-safe

src/modules/fancyzones/lib/FancyZones.cpp

@@ -430,9 +430,13 @@ void FancyZones::ToggleEditor() noexcept
         std::to_wstring(width) + L"_" +
         std::to_wstring(height);
 
-    const auto& deviceInfo = fancyZonesData.GetDeviceInfoMap().at(zoneWindow->UniqueId());
+    const auto deviceInfo = fancyZonesData.FindDeviceInfo(zoneWindow->UniqueId());
+    if(!deviceInfo.has_value())
+    {
+       return;
+    }
 
-    JSONHelpers::DeviceInfoJSON deviceInfoJson{ zoneWindow->UniqueId(), deviceInfo };
+    JSONHelpers::DeviceInfoJSON deviceInfoJson{ zoneWindow->UniqueId(), *deviceInfo };
     fancyZonesData.SerializeDeviceInfoToTmpFile(deviceInfoJson, ZoneWindowUtils::GetActiveZoneSetTmpPath());
 
     const std::wstring params =

src/modules/fancyzones/lib/JsonHelpers.cpp

@@ -135,13 +135,10 @@ namespace JSONHelpers
         jsonFilePath = result + L"\\" + std::wstring(FANCY_ZONES_DATA_FILE);
     }
 
-    const std::wstring& FancyZonesData::GetPersistFancyZonesJSONPath() const
-    {
-        return jsonFilePath;
-    }
-
     json::JsonObject FancyZonesData::GetPersistFancyZonesJSON()
     {
+        std::scoped_lock lock{ dataLock };
+
         std::wstring save_file_path = GetPersistFancyZonesJSONPath();
 
         auto result = json::from_file(save_file_path);
@@ -155,8 +152,23 @@ namespace JSONHelpers
         }
     }
 
+    std::optional<DeviceInfoData> FancyZonesData::FindDeviceInfo(const std::wstring& zoneWindowId) const
+    {
+        std::scoped_lock lock{ dataLock };
+        auto it = deviceInfoMap.find(zoneWindowId);
+        return it != end(deviceInfoMap) ? std::optional{ it->second } : std::nullopt;
+    }
+
+    std::optional<CustomZoneSetData> FancyZonesData::FindCustomZoneSet(const std::wstring& guuid) const
+    {
+        std::scoped_lock lock{ dataLock };
+        auto it = customZoneSetsMap.find(guuid);
+        return it != end(customZoneSetsMap) ? std::optional{ it->second } : std::nullopt;
+    }
+
     void FancyZonesData::AddDevice(const std::wstring& deviceId)
     {
+        std::scoped_lock lock{ dataLock };
         if (!deviceInfoMap.contains(deviceId))
         {
             // Creates default entry in map when ZoneWindow is created
@@ -168,6 +180,7 @@ namespace JSONHelpers
 
     void FancyZonesData::CloneDeviceInfo(const std::wstring& source, const std::wstring& destination)
     {
+        std::scoped_lock lock{ dataLock };
         // Clone information from source device if destination device is uninitialized (Blank).
         auto& destInfo = deviceInfoMap[destination];
         if (destInfo.activeZoneSet.type == ZoneSetLayoutType::Blank)
@@ -178,6 +191,7 @@ namespace JSONHelpers
 
     int FancyZonesData::GetAppLastZoneIndex(HWND window, const std::wstring_view& deviceId, const std::wstring_view& zoneSetId) const
     {
+        std::scoped_lock lock{ dataLock };
         auto processPath = get_process_path(window);
         if (!processPath.empty())
         {
@@ -197,6 +211,7 @@ namespace JSONHelpers
 
     bool FancyZonesData::RemoveAppLastZone(HWND window, const std::wstring_view& deviceId, const std::wstring_view& zoneSetId)
     {
+        std::scoped_lock lock{ dataLock };
         auto processPath = get_process_path(window);
         if (!processPath.empty())
         {
@@ -218,6 +233,7 @@ namespace JSONHelpers
 
     bool FancyZonesData::SetAppLastZone(HWND window, const std::wstring& deviceId, const std::wstring& zoneSetId, int zoneIndex)
     {
+        std::scoped_lock lock{ dataLock };
         auto processPath = get_process_path(window);
         if (processPath.empty())
         {
@@ -231,6 +247,7 @@ namespace JSONHelpers
 
     void FancyZonesData::SetActiveZoneSet(const std::wstring& deviceId, const ZoneSetData& data)
     {
+        std::scoped_lock lock{ dataLock };
         auto it = deviceInfoMap.find(deviceId);
         if (it != deviceInfoMap.end())
         {
@@ -240,12 +257,14 @@ namespace JSONHelpers
 
     void FancyZonesData::SerializeDeviceInfoToTmpFile(const DeviceInfoJSON& deviceInfo, std::wstring_view tmpFilePath) const
     {
+        std::scoped_lock lock{ dataLock };
         json::JsonObject deviceInfoJson = DeviceInfoJSON::ToJson(deviceInfo);
         json::to_file(tmpFilePath, deviceInfoJson);
     }
 
     void FancyZonesData::ParseDeviceInfoFromTmpFile(std::wstring_view tmpFilePath)
     {
+        std::scoped_lock lock{ dataLock };
         if (std::filesystem::exists(tmpFilePath))
         {
             if (auto zoneSetJson = json::from_file(tmpFilePath); zoneSetJson.has_value())
@@ -266,6 +285,7 @@ namespace JSONHelpers
 
     bool FancyZonesData::ParseCustomZoneSetFromTmpFile(std::wstring_view tmpFilePath)
     {
+        std::scoped_lock lock{ dataLock };
         bool res = true;
         if (std::filesystem::exists(tmpFilePath))
         {
@@ -291,6 +311,7 @@ namespace JSONHelpers
 
     bool FancyZonesData::ParseDeletedCustomZoneSetsFromTmpFile(std::wstring_view tmpFilePath)
     {
+        std::scoped_lock lock{ dataLock };
         bool res = true;
         if (std::filesystem::exists(tmpFilePath))
         {
@@ -317,6 +338,7 @@ namespace JSONHelpers
 
     bool FancyZonesData::ParseAppZoneHistory(const json::JsonObject& fancyZonesDataJSON)
     {
+        std::scoped_lock lock{ dataLock };
         try
         {
             auto appLastZones = fancyZonesDataJSON.GetNamedArray(L"app-zone-history");
@@ -344,6 +366,7 @@ namespace JSONHelpers
 
     json::JsonArray FancyZonesData::SerializeAppZoneHistory() const
     {
+        std::scoped_lock lock{ dataLock };
         json::JsonArray appHistoryArray;
 
         for (const auto& [appPath, appZoneHistoryData] : appZoneHistoryMap)
@@ -356,6 +379,7 @@ namespace JSONHelpers
 
     bool FancyZonesData::ParseDeviceInfos(const json::JsonObject& fancyZonesDataJSON)
     {
+        std::scoped_lock lock{ dataLock };
         try
         {
             auto devices = fancyZonesDataJSON.GetNamedArray(L"devices");
@@ -382,6 +406,7 @@ namespace JSONHelpers
 
     json::JsonArray FancyZonesData::SerializeDeviceInfos() const
     {
+        std::scoped_lock lock{ dataLock };
         json::JsonArray DeviceInfosJSON{};
 
         for (const auto& [deviceID, deviceData] : deviceInfoMap)
@@ -396,6 +421,7 @@ namespace JSONHelpers
 
     bool FancyZonesData::ParseCustomZoneSets(const json::JsonObject& fancyZonesDataJSON)
     {
+        std::scoped_lock lock{ dataLock };
         try
         {
             auto customZoneSets = fancyZonesDataJSON.GetNamedArray(L"custom-zone-sets");
@@ -418,6 +444,7 @@ namespace JSONHelpers
 
     json::JsonArray FancyZonesData::SerializeCustomZoneSets() const
     {
+        std::scoped_lock lock{ dataLock };
         json::JsonArray customZoneSetsJSON{};
 
         for (const auto& [zoneSetId, zoneSetData] : customZoneSetsMap)
@@ -430,6 +457,7 @@ namespace JSONHelpers
 
     void FancyZonesData::CustomZoneSetsToJsonFile(std::wstring_view filePath) const
     {
+        std::scoped_lock lock{ dataLock };
         const auto& customZoneSetsJson = SerializeCustomZoneSets();
         json::JsonObject root{};
         root.SetNamedValue(L"custom-zone-sets", customZoneSetsJson);
@@ -438,6 +466,7 @@ namespace JSONHelpers
 
     void FancyZonesData::LoadFancyZonesData()
     {
+        std::scoped_lock lock{ dataLock };
         std::wstring jsonFilePath = GetPersistFancyZonesJSONPath();
 
         if (!std::filesystem::exists(jsonFilePath))
@@ -461,6 +490,7 @@ namespace JSONHelpers
 
     void FancyZonesData::SaveFancyZonesData() const
     {
+        std::scoped_lock lock{ dataLock };
         json::JsonObject root{};
 
         root.SetNamedValue(L"app-zone-history", SerializeAppZoneHistory());
@@ -474,6 +504,7 @@ namespace JSONHelpers
     {
         std::wregex ex(L"^[0-9]{3,4}_[0-9]{3,4}$");
 
+        std::scoped_lock lock{ dataLock };
         wchar_t key[256];
         StringCchPrintf(key, ARRAYSIZE(key), L"%s", RegistryHelpers::REG_SETTINGS);
         HKEY hkey;
@@ -521,6 +552,7 @@ namespace JSONHelpers
 
     void FancyZonesData::MigrateDeviceInfoFromRegistry(const std::wstring& deviceId)
     {
+        std::scoped_lock lock{ dataLock };
         wchar_t key[256];
         StringCchPrintf(key, ARRAYSIZE(key), L"%s\\%s", RegistryHelpers::REG_SETTINGS, deviceId.c_str());
 
@@ -546,6 +578,7 @@ namespace JSONHelpers
 
     void FancyZonesData::MigrateCustomZoneSetsFromRegistry()
     {
+        std::scoped_lock lock{ dataLock };
         wchar_t key[256];
         StringCchPrintf(key, ARRAYSIZE(key), L"%s\\%s", RegistryHelpers::REG_SETTINGS, L"Layouts");
         HKEY hkey;

src/modules/fancyzones/lib/JsonHelpers.h

@@ -2,6 +2,7 @@
 
 #include <common/settings_helpers.h>
 #include <common/json.h>
+#include <mutex>
 
 #include <string>
 #include <strsafe.h>
@@ -159,34 +160,58 @@ namespace JSONHelpers
 
     class FancyZonesData
     {
+        mutable std::recursive_mutex dataLock;
+
     public:
         FancyZonesData();
 
-        const std::wstring& GetPersistFancyZonesJSONPath() const;
+        inline const std::wstring& GetPersistFancyZonesJSONPath() const
+        {
+            return jsonFilePath;
+        }
         json::JsonObject GetPersistFancyZonesJSON();
 
+        std::optional<DeviceInfoData> FindDeviceInfo(const std::wstring& zoneWindowId) const;
+
+        std::optional<CustomZoneSetData> FindCustomZoneSet(const std::wstring& guuid) const;
+
+        inline const std::wstring GetActiveDeviceId() const
+        {
+            return activeDeviceId;
+        }
+
+#if defined(UNIT_TESTS)
         inline const std::unordered_map<std::wstring, DeviceInfoData>& GetDeviceInfoMap() const
         {
             return deviceInfoMap;
         }
 
         inline const std::unordered_map<std::wstring, CustomZoneSetData>& GetCustomZoneSetsMap() const
         {
+
             return customZoneSetsMap;
         }
 
         inline const std::unordered_map<std::wstring, AppZoneHistoryData>& GetAppZoneHistoryMap() const
         {
+
             return appZoneHistoryMap;
         }
 
-        inline const std::wstring GetActiveDeviceId() const
+
+        inline void clear_data()
         {
-            return activeDeviceId;
+            appliedZoneSetsMap.clear();
+            appZoneHistoryMap.clear();
+            deviceInfoMap.clear();
+            customZoneSetsMap.clear();
+            activeDeviceId.clear();
         }
+#endif
 
-        void SetActiveDeviceId(const std::wstring& deviceId)
+        inline void SetActiveDeviceId(const std::wstring& deviceId)
         {
+            std::scoped_lock lock{ dataLock };
             activeDeviceId = deviceId;
         }
 

src/modules/fancyzones/lib/ZoneSet.cpp

@@ -497,13 +497,15 @@ bool ZoneSet::CalculateCustomLayout(Rect workArea, int spacing) noexcept
     if (SUCCEEDED_LOG(StringFromCLSID(m_config.Id, &guuidStr)))
     {
         const std::wstring guuid = guuidStr.get();
-        const auto& customZoneSets = JSONHelpers::FancyZonesDataInstance().GetCustomZoneSetsMap();
-        if (!customZoneSets.contains(guuid))
+
+        const auto zoneSetSearchResult = JSONHelpers::FancyZonesDataInstance().FindCustomZoneSet(guuid);
+
+        if(!zoneSetSearchResult.has_value())
         {
             return false;
         }
 
-        const auto& zoneSet = customZoneSets.at(guuid);
+        const auto& zoneSet = *zoneSetSearchResult;
         if (zoneSet.type == JSONHelpers::CustomLayoutType::Canvas && std::holds_alternative<JSONHelpers::CanvasLayoutInfo>(zoneSet.info))
         {
             const auto& zoneSetInfo = std::get<JSONHelpers::CanvasLayoutInfo>(zoneSet.info);

src/modules/fancyzones/lib/ZoneWindow.cpp

@@ -559,15 +559,21 @@ void ZoneWindow::InitializeZoneSets(MONITORINFO const& mi) noexcept
 void ZoneWindow::CalculateZoneSet() noexcept
 {
     const auto& fancyZonesData = JSONHelpers::FancyZonesDataInstance();
-    const auto& deviceInfoMap = fancyZonesData.GetDeviceInfoMap();
+    const auto deviceInfoData = fancyZonesData.FindDeviceInfo(m_uniqueId);
     const auto& activeDeviceId = fancyZonesData.GetActiveDeviceId();
-    const auto& activeZoneSet = deviceInfoMap.at(m_uniqueId).activeZoneSet;
 
     if (!activeDeviceId.empty() && activeDeviceId != m_uniqueId)
     {
         return;
     }
 
+    if (!deviceInfoData.has_value())
+    {
+        return;
+    }
+
+    const auto& activeZoneSet = deviceInfoData->activeZoneSet;
+
     if (activeZoneSet.uuid.empty() || activeZoneSet.type == JSONHelpers::ZoneSetLayoutType::Blank)
     {
         return;
@@ -585,9 +591,9 @@ void ZoneWindow::CalculateZoneSet() noexcept
         monitorInfo.cbSize = sizeof(monitorInfo);
         if (GetMonitorInfoW(m_monitor, &monitorInfo))
         {
-            bool showSpacing = deviceInfoMap.at(m_uniqueId).showSpacing;
-            int spacing = showSpacing ? deviceInfoMap.at(m_uniqueId).spacing : 0;
-            int zoneCount = deviceInfoMap.at(m_uniqueId).zoneCount;
+            bool showSpacing = deviceInfoData->showSpacing;
+            int spacing = showSpacing ? deviceInfoData->spacing : 0;
+            int zoneCount = deviceInfoData->zoneCount;
             zoneSet->CalculateZones(monitorInfo, zoneCount, spacing);
             UpdateActiveZoneSet(zoneSet.get());
         }

src/modules/fancyzones/tests/UnitTests/FancyZones.Spec.cpp

@@ -290,7 +290,7 @@ namespace FancyZonesUnitTests
             m_fzCallback = fancyZones.as<IFancyZonesCallback>();
             Assert::IsTrue(m_fzCallback != nullptr);
 
-            m_fancyZonesData = JSONHelpers::FancyZonesData();
+            m_fancyZonesData.clear_data();
         }
 
         TEST_METHOD_CLEANUP(Cleanup)

src/modules/fancyzones/tests/UnitTests/JsonHelpers.Tests.cpp

@@ -725,7 +725,7 @@ namespace FancyZonesUnitTests
         TEST_METHOD_INITIALIZE(Init)
         {
             m_hInst = (HINSTANCE)GetModuleHandleW(nullptr);
-            m_fzData = FancyZonesData();
+            m_fzData.clear_data();
         }
 
     public:

src/modules/fancyzones/tests/UnitTests/UnitTests.vcxproj

@@ -60,7 +60,7 @@
       <Optimization>Disabled</Optimization>
       <SDLCheck>true</SDLCheck>
       <AdditionalIncludeDirectories>..\..\..\..\common\Telemetry;..\..\..\..\;..\..\..\..\..\deps\cpprestsdk\include;..\..\;$(VCInstallDir)UnitTest\include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
-      <PreprocessorDefinitions>_DEBUG;%(PreprocessorDefinitions)</PreprocessorDefinitions>
+      <PreprocessorDefinitions>UNIT_TESTS;_DEBUG;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <UseFullPaths>true</UseFullPaths>
       <PrecompiledHeaderFile>pch.h</PrecompiledHeaderFile>
       <LanguageStandard>stdcpplatest</LanguageStandard>
@@ -81,7 +81,7 @@
       <IntrinsicFunctions>true</IntrinsicFunctions>
       <SDLCheck>true</SDLCheck>
       <AdditionalIncludeDirectories>..\..\..\..\common\Telemetry;..\..\..\..\;..\..\..\..\..\deps\cpprestsdk\include;..\..\;$(VCInstallDir)UnitTest\include;%(AdditionalIncludeDirectories)</AdditionalIncludeDirectories>
-      <PreprocessorDefinitions>NDEBUG;%(PreprocessorDefinitions)</PreprocessorDefinitions>
+      <PreprocessorDefinitions>UNIT_TESTS;NDEBUG;%(PreprocessorDefinitions)</PreprocessorDefinitions>
       <UseFullPaths>true</UseFullPaths>
       <PrecompiledHeaderFile>pch.h</PrecompiledHeaderFile>
       <LanguageStandard>stdcpplatest</LanguageStandard>

src/modules/fancyzones/tests/UnitTests/ZoneWindow.Spec.cpp

@@ -89,7 +89,7 @@ namespace FancyZonesUnitTests
             Assert::IsFalse(std::filesystem::exists(ZoneWindowUtils::GetAppliedZoneSetTmpPath()));
             Assert::IsFalse(std::filesystem::exists(ZoneWindowUtils::GetCustomZoneSetsTmpPath()));
 
-            m_fancyZonesData = JSONHelpers::FancyZonesData();
+            m_fancyZonesData.clear_data();
         }
 
         TEST_METHOD_CLEANUP(Cleanup)