Skip to content

Commit c111a0e

Browse files
madelsongithub-actions
authored and
github-actions
committed
Implement feedback from #75054
1 parent d42bd87 commit c111a0e

File tree

4 files changed

+13
-4
lines changed

4 files changed

+13
-4
lines changed

src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1092,6 +1092,7 @@ public DictionaryEntry Entry
10921092
object? value = null;
10931093
// 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).
10941094
// Lock order MUST match RuntimeResourceSet.GetObject to avoid deadlock.
1095+
Debug.Assert(!Monitor.IsEntered(_reader));
10951096
lock (_reader._resCache)
10961097
{
10971098
if (_reader._resCache.TryGetValue(key, out ResourceLocator locator))

src/libraries/System.Private.CoreLib/src/System/Resources/RuntimeResourceSet.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using System.Collections.Generic;
66
using System.Diagnostics;
77
using System.IO;
8+
using System.Threading;
89

910
namespace System.Resources
1011
#if RESOURCES_EXTENSIONS
@@ -278,6 +279,7 @@ private IDictionaryEnumerator GetEnumeratorHelper()
278279

279280
// Lock the cache first, then the reader (reader locks implicitly through its methods).
280281
// Lock order MUST match ResourceReader.ResourceEnumerator.Entry to avoid deadlock.
282+
Debug.Assert(!Monitor.IsEntered(reader));
281283
lock (cache)
282284
{
283285
// Find the offset within the data section

src/libraries/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -499,7 +499,6 @@ public static void EmbeddedResourcesAreUpToDate()
499499
}
500500
}
501501

502-
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsBinaryFormatterSupported))]
503502
/// <summary>
504503
/// This test has multiple threads simultaneously loop over the keys of a moderately-sized resx using
505504
/// <see cref="ResourceManager"/> and call <see cref="ResourceManager.GetString(string)"/> for each key.
@@ -513,6 +512,7 @@ public static void EmbeddedResourcesAreUpToDate()
513512
/// Whether to use <see cref="IDictionaryEnumerator.Entry"/> vs. <see cref="IDictionaryEnumerator.Key"/> when enumerating;
514513
/// these follow fairly different code paths.
515514
/// </param>]
515+
[ConditionalTheory(typeof(PlatformDetection), nameof(PlatformDetection.IsBinaryFormatterSupported))]
516516
[InlineData(false)]
517517
[InlineData(true)]
518518
public static void TestResourceManagerIsSafeForConcurrentAccessAndEnumeration(bool useEnumeratorEntry)

src/libraries/System.Resources.ResourceManager/tests/ResourceManagerTests.cs

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -243,9 +243,15 @@ public static void TestResourceManagerIsSafeForConcurrentAccessAndEnumeration(bo
243243

244244
const int Threads = 10;
245245
using Barrier barrier = new(Threads);
246-
Task task = Task.WhenAll(Enumerable.Range(0, Threads).Select(_ => Task.Run(WaitForBarrierThenEnumerateResources)));
247-
248-
Assert.True(task.Wait(TimeSpan.FromSeconds(10)));
246+
Task[] tasks = Enumerable.Range(0, Threads)
247+
.Select(_ => Task.Factory.StartNew(
248+
WaitForBarrierThenEnumerateResources,
249+
CancellationToken.None,
250+
TaskCreationOptions.LongRunning,
251+
TaskScheduler.Default))
252+
.ToArray();
253+
254+
Assert.True(Task.WaitAll(tasks, TimeSpan.FromSeconds(30)));
249255

250256
void WaitForBarrierThenEnumerateResources()
251257
{

0 commit comments

Comments
 (0)