Implement feedback from #75054

dotnet · Feb 10, 2023 · faba965 · faba965
1 parent 478b697
commit faba965
Show file tree

Hide file tree

Showing 4 changed files with 13 additions and 4 deletions.
diff --git a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
@@ -1108,6 +1108,7 @@ public DictionaryEntry Entry
                     object? value = null;
                     // Lock the cache first, then the reader (in this case, we don't actually need to lock the reader and cache at the same time).
                     // Lock order MUST match RuntimeResourceSet.GetObject to avoid deadlock.
+                    Debug.Assert(!Monitor.IsEntered(_reader));
                     lock (_reader._resCache)
                     {
                         if (_reader._resCache.TryGetValue(key, out ResourceLocator locator))

diff --git a/src/libraries/System.Private.CoreLib/src/System/Resources/RuntimeResourceSet.cs b/src/libraries/System.Private.CoreLib/src/System/Resources/RuntimeResourceSet.cs
@@ -5,6 +5,7 @@
 using System.Collections.Generic;
 using System.Diagnostics;
 using System.IO;
+using System.Threading;
 
 namespace System.Resources
 #if RESOURCES_EXTENSIONS
@@ -285,6 +286,7 @@ private IDictionaryEnumerator GetEnumeratorHelper()
 
             // Lock the cache first, then the reader (reader locks implicitly through its methods).
             // Lock order MUST match ResourceReader.ResourceEnumerator.Entry to avoid deadlock.
+            Debug.Assert(!Monitor.IsEntered(reader));
             lock (cache)
             {
                 // Find the offset within the data section

diff --git a/src/libraries/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs b/src/libraries/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs
@@ -501,7 +501,6 @@ public static void EmbeddedResourcesAreUpToDate()
             }
         }
 
-        [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsBinaryFormatterSupported))]
         /// <summary>
         /// This test has multiple threads simultaneously loop over the keys of a moderately-sized resx using
         /// <see cref="ResourceManager"/> and call <see cref="ResourceManager.GetString(string)"/> for each key.
@@ -515,6 +514,7 @@ public static void EmbeddedResourcesAreUpToDate()
         /// Whether to use <see cref="IDictionaryEnumerator.Entry"/> vs. <see cref="IDictionaryEnumerator.Key"/> when enumerating;
         /// these follow fairly different code paths.
         /// </param>]
+        [ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsBinaryFormatterSupported))]
         [InlineData(false)]
         [InlineData(true)]
         public static void TestResourceManagerIsSafeForConcurrentAccessAndEnumeration(bool useEnumeratorEntry)

diff --git a/src/libraries/System.Resources.ResourceManager/tests/ResourceManagerTests.cs b/src/libraries/System.Resources.ResourceManager/tests/ResourceManagerTests.cs
@@ -247,9 +247,15 @@ public static void TestResourceManagerIsSafeForConcurrentAccessAndEnumeration(bo
 
             const int Threads = 10;
             using Barrier barrier = new(Threads);
-            Task task = Task.WhenAll(Enumerable.Range(0, Threads).Select(_ => Task.Run(WaitForBarrierThenEnumerateResources)));
-
-            Assert.True(task.Wait(TimeSpan.FromSeconds(10)));
+            Task[] tasks = Enumerable.Range(0, Threads)
+                .Select(_ => Task.Factory.StartNew(
+                    WaitForBarrierThenEnumerateResources,
+                    CancellationToken.None,
+                    TaskCreationOptions.LongRunning,
+                    TaskScheduler.Default))
+                .ToArray();
+
+            Assert.True(Task.WaitAll(tasks, TimeSpan.FromSeconds(30)));
 
             void WaitForBarrierThenEnumerateResources()
             {