From 075c9aec36bb66f2d790fd47388488f522198269 Mon Sep 17 00:00:00 2001
From: Michael Adelson <mike.adelson314@gmail.com>
Date: Sat, 3 Sep 2022 07:43:41 -0400
Subject: [PATCH 1/6] Fix thread-safety issues with enumerating
 ResourceManager.

Concurrently enumerating a ResourceManager while also calling GetString()
and similar methods was prone to both transient errors and deadlock.

The transient errors were caused by RuntimeResourceSet calling internal
methods on ResourceReader that did not properly lock. Now, all exposed
methods on the Reader are thread-safe.

The deadlock was called by inconsistent lock ordering between
ResourceReader.ResourceEnumerator and RuntimeResourceSet which both lock
on the RuntimeResourceSet's cache and on the ResourceReader itself. Now,
the enumerator does not need to take both locks at the same time.

Fix #74052
Fix #74868
---
 .../System/Resources/ResourceReader.Core.cs   |   4 +
 .../src/System/Resources/ResourceReader.cs    | 154 +++++----
 .../System/Resources/RuntimeResourceSet.cs    |   9 +-
 .../tests/ResourceManagerTests.cs             |  43 +++
 .../tests/Resources/AToZResx.Designer.cs      | 297 ++++++++++++++++++
 .../tests/Resources/AToZResx.resx             | 198 ++++++++++++
 ...tem.Resources.ResourceManager.Tests.csproj |  35 +--
 7 files changed, 652 insertions(+), 88 deletions(-)
 create mode 100644 src/libraries/System.Resources.ResourceManager/tests/Resources/AToZResx.Designer.cs
 create mode 100644 src/libraries/System.Resources.ResourceManager/tests/Resources/AToZResx.resx

diff --git a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.Core.cs b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.Core.cs
index 2153d23ee97c6..f5a4b1a440720 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.Core.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.Core.cs
@@ -51,6 +51,8 @@ internal ResourceReader(Stream stream, Dictionary<string, ResourceLocator> resCa
             "the user to only get one error.")]
         private object DeserializeObject(int typeIndex)
         {
+            Debug.Assert(Monitor.IsEntered(this));
+
             if (!AllowCustomResourceTypes)
             {
                 throw new NotSupportedException(SR.ResourceManager_ReflectionNotAllowed);
@@ -89,6 +91,8 @@ private object DeserializeObject(int typeIndex)
             "Custom readers as well as custom objects on the resources file are not observable by the trimmer and so required assemblies, types and members may be removed.")]
         private bool InitializeBinaryFormatter()
         {
+            Debug.Assert(Monitor.IsEntered(this));
+
             // If BinaryFormatter support is disabled for the app, the linker will replace this entire
             // method body with "return false;", skipping all reflection code below.
 
diff --git a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
index 16e2b161fc0db..36846b6a27897 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
@@ -9,6 +9,7 @@
 using System.IO;
 using System.Runtime.CompilerServices;
 using System.Text;
+using System.Threading;
 
 namespace System.Resources
 #if RESOURCES_EXTENSIONS
@@ -58,10 +59,11 @@ public sealed partial class
         // it make sense to use anything less than one page?
         private const int DefaultFileStreamBufferSize = 4096;
 
-        private BinaryReader _store;    // backing store we're reading from.
-        // Used by RuntimeResourceSet and this class's enumerator.  Maps
-        // resource name to a value, a ResourceLocator, or a
-        // LooselyLinkedManifestResource.
+        // Backing store we're reading from. Usages outside of constructor
+        // initialization must be protected by lock (this).
+        private BinaryReader _store;
+        // Used by RuntimeResourceSet and this class's enumerator.
+        // Accesses must be protected by lock(_resCache). 
         internal Dictionary<string, ResourceLocator>? _resCache;
         private long _nameSectionOffset;  // Offset to name section of file.
         private long _dataSectionOffset;  // Offset to Data section of file.
@@ -86,7 +88,6 @@ public sealed partial class
         // Version number of .resources file, for compatibility
         private int _version;
 
-
         public
 #if RESOURCES_EXTENSIONS
         DeserializingResourceReader(string fileName)
@@ -163,7 +164,7 @@ private unsafe void Dispose(bool disposing)
             }
         }
 
-        internal static unsafe int ReadUnalignedI4(int* p)
+        private static unsafe int ReadUnalignedI4(int* p)
         {
             return BinaryPrimitives.ReadInt32LittleEndian(new ReadOnlySpan<byte>(p, sizeof(int)));
         }
@@ -228,6 +229,7 @@ public IDictionaryEnumerator GetEnumerator()
             return new ResourceEnumerator(this);
         }
 
+        // Called from RuntimeResourceSet
         internal ResourceEnumerator GetEnumeratorInternal()
         {
             return new ResourceEnumerator(this);
@@ -237,6 +239,7 @@ internal ResourceEnumerator GetEnumeratorInternal()
         // To read the data, seek to _dataSectionOffset + dataPos, then
         // read the resource type & data.
         // This does a binary search through the names.
+        // Called from RuntimeResourceSet
         internal int FindPosForResource(string name)
         {
             Debug.Assert(_store != null, "ResourceReader is closed!");
@@ -321,6 +324,8 @@ internal int FindPosForResource(string name)
         private unsafe bool CompareStringEqualsName(string name)
         {
             Debug.Assert(_store != null, "ResourceReader is closed!");
+            Debug.Assert(Monitor.IsEntered(this));
+
             int byteLen = _store.Read7BitEncodedInt();
             if (byteLen < 0)
             {
@@ -453,68 +458,74 @@ private unsafe string AllocateStringForNameIndex(int index, out int dataOffset)
         }
 
         // This takes a virtual offset into the data section and reads a String
-        // from that location.
-        // Anyone who calls LoadObject should make sure they take a lock so
-        // no one can cause us to do a seek in here.
+        // from that location. Called from RuntimeResourceSet
         internal string? LoadString(int pos)
         {
             Debug.Assert(_store != null, "ResourceReader is closed!");
-            _store.BaseStream.Seek(_dataSectionOffset + pos, SeekOrigin.Begin);
-            string? s = null;
-            int typeIndex = _store.Read7BitEncodedInt();
-            if (_version == 1)
-            {
-                if (typeIndex == -1)
-                    return null;
-                if (FindType(typeIndex) != typeof(string))
-                    throw new InvalidOperationException(SR.Format(SR.InvalidOperation_ResourceNotString_Type, FindType(typeIndex).FullName));
-                s = _store.ReadString();
-            }
-            else
+
+            lock (this)
             {
-                ResourceTypeCode typeCode = (ResourceTypeCode)typeIndex;
-                if (typeCode != ResourceTypeCode.String && typeCode != ResourceTypeCode.Null)
+                _store.BaseStream.Seek(_dataSectionOffset + pos, SeekOrigin.Begin);
+                string? s = null;
+                int typeIndex = _store.Read7BitEncodedInt();
+                if (_version == 1)
                 {
-                    string? typeString;
-                    if (typeCode < ResourceTypeCode.StartOfUserTypes)
-                        typeString = typeCode.ToString();
-                    else
-                        typeString = FindType(typeCode - ResourceTypeCode.StartOfUserTypes).FullName;
-                    throw new InvalidOperationException(SR.Format(SR.InvalidOperation_ResourceNotString_Type, typeString));
-                }
-                if (typeCode == ResourceTypeCode.String) // ignore Null
+                    if (typeIndex == -1)
+                        return null;
+                    if (FindType(typeIndex) != typeof(string))
+                        throw new InvalidOperationException(SR.Format(SR.InvalidOperation_ResourceNotString_Type, FindType(typeIndex).FullName));
                     s = _store.ReadString();
+                }
+                else
+                {
+                    ResourceTypeCode typeCode = (ResourceTypeCode)typeIndex;
+                    if (typeCode != ResourceTypeCode.String && typeCode != ResourceTypeCode.Null)
+                    {
+                        string? typeString;
+                        if (typeCode < ResourceTypeCode.StartOfUserTypes)
+                            typeString = typeCode.ToString();
+                        else
+                            typeString = FindType(typeCode - ResourceTypeCode.StartOfUserTypes).FullName;
+                        throw new InvalidOperationException(SR.Format(SR.InvalidOperation_ResourceNotString_Type, typeString));
+                    }
+                    if (typeCode == ResourceTypeCode.String) // ignore Null
+                        s = _store.ReadString();
+                }
+                return s;
             }
-            return s;
         }
 
         // Called from RuntimeResourceSet
         internal object? LoadObject(int pos)
         {
-            if (_version == 1)
-                return LoadObjectV1(pos);
-            return LoadObjectV2(pos, out _);
+            lock (this)
+            {
+                return _version == 1 ? LoadObjectV1(pos) : LoadObjectV2(pos, out _);
+            }
         }
 
+        // Called from RuntimeResourceSet
         internal object? LoadObject(int pos, out ResourceTypeCode typeCode)
         {
-            if (_version == 1)
+            lock (this)
             {
-                object? o = LoadObjectV1(pos);
-                typeCode = (o is string) ? ResourceTypeCode.String : ResourceTypeCode.StartOfUserTypes;
-                return o;
+                if (_version == 1)
+                {
+                    object? o = LoadObjectV1(pos);
+                    typeCode = (o is string) ? ResourceTypeCode.String : ResourceTypeCode.StartOfUserTypes;
+                    return o;
+                }
+                return LoadObjectV2(pos, out typeCode);
             }
-            return LoadObjectV2(pos, out typeCode);
         }
 
         // This takes a virtual offset into the data section and reads an Object
         // from that location.
-        // Anyone who calls LoadObject should make sure they take a lock so
-        // no one can cause us to do a seek in here.
-        internal object? LoadObjectV1(int pos)
+        private object? LoadObjectV1(int pos)
         {
             Debug.Assert(_store != null, "ResourceReader is closed!");
             Debug.Assert(_version == 1, ".resources file was not a V1 .resources file!");
+            Debug.Assert(Monitor.IsEntered(this));
 
             try
             {
@@ -534,6 +545,8 @@ private unsafe string AllocateStringForNameIndex(int index, out int dataOffset)
 
         private object? _LoadObjectV1(int pos)
         {
+            Debug.Assert(Monitor.IsEntered(this));
+
             _store.BaseStream.Seek(_dataSectionOffset + pos, SeekOrigin.Begin);
             int typeIndex = _store.Read7BitEncodedInt();
             if (typeIndex == -1)
@@ -586,10 +599,11 @@ private unsafe string AllocateStringForNameIndex(int index, out int dataOffset)
             }
         }
 
-        internal object? LoadObjectV2(int pos, out ResourceTypeCode typeCode)
+        private object? LoadObjectV2(int pos, out ResourceTypeCode typeCode)
         {
             Debug.Assert(_store != null, "ResourceReader is closed!");
             Debug.Assert(_version >= 2, ".resources file was not a V2 (or higher) .resources file!");
+            Debug.Assert(Monitor.IsEntered(this));
 
             try
             {
@@ -609,6 +623,8 @@ private unsafe string AllocateStringForNameIndex(int index, out int dataOffset)
 
         private object? _LoadObjectV2(int pos, out ResourceTypeCode typeCode)
         {
+            Debug.Assert(Monitor.IsEntered(this));
+
             _store.BaseStream.Seek(_dataSectionOffset + pos, SeekOrigin.Begin);
             typeCode = (ResourceTypeCode)_store.Read7BitEncodedInt();
 
@@ -745,6 +761,7 @@ private unsafe string AllocateStringForNameIndex(int index, out int dataOffset)
         [MemberNotNull(nameof(_typeNamePositions))]
         private void ReadResources()
         {
+            Debug.Assert(!Monitor.IsEntered(this)); // only called during init
             Debug.Assert(_store != null, "ResourceReader is closed!");
 
             try
@@ -767,6 +784,8 @@ private void ReadResources()
         [MemberNotNull(nameof(_typeNamePositions))]
         private void _ReadResources()
         {
+            Debug.Assert(!Monitor.IsEntered(this)); // only called during init
+
             // Read ResourceManager header
             // Check for magic number
             int magicNum = _store.ReadInt32();
@@ -935,6 +954,8 @@ private void _ReadResources()
             "the user to only get one error.")]
         private Type FindType(int typeIndex)
         {
+            Debug.Assert(Monitor.IsEntered(this));
+
             if (!AllowCustomResourceTypes)
             {
                 throw new NotSupportedException(SR.ResourceManager_ReflectionNotAllowed);
@@ -952,6 +973,8 @@ private Type FindType(int typeIndex)
             "Custom readers as well as custom objects on the resources file are not observable by the trimmer and so required assemblies, types and members may be removed.")]
         private Type UseReflectionToGetType(int typeIndex)
         {
+            Debug.Assert(Monitor.IsEntered(this));
+
             long oldPos = _store.BaseStream.Position;
             try
             {
@@ -984,6 +1007,8 @@ private Type UseReflectionToGetType(int typeIndex)
         private string TypeNameFromTypeCode(ResourceTypeCode typeCode)
         {
             Debug.Assert(typeCode >= 0, "can't be negative");
+            Debug.Assert(Monitor.IsEntered(this));
+
             if (typeCode < ResourceTypeCode.StartOfUserTypes)
             {
                 Debug.Assert(!string.Equals(typeCode.ToString(), "LastPrimitive"), "Change ResourceTypeCode metadata order so LastPrimitive isn't what Enum.ToString prefers.");
@@ -1061,31 +1086,30 @@ public DictionaryEntry Entry
                     if (!_currentIsValid) throw new InvalidOperationException(SR.InvalidOperation_EnumNotStarted);
                     if (_reader._resCache == null) throw new InvalidOperationException(SR.ResourceReaderIsClosed);
 
-                    string key;
+                    string key = _reader.AllocateStringForNameIndex(_currentName, out _dataPosition); // AllocateStringForNameIndex could lock on _reader
+
                     object? value = null;
-                    lock (_reader)
-                    { // locks should be taken in the same order as in RuntimeResourceSet.GetObject to avoid deadlock
-                        lock (_reader._resCache)
+                    // 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.
+                    lock (_reader._resCache)
+                    {
+                        if (_reader._resCache.TryGetValue(key, out ResourceLocator locator))
                         {
-                            key = _reader.AllocateStringForNameIndex(_currentName, out _dataPosition); // AllocateStringForNameIndex could lock on _reader
-                            if (_reader._resCache.TryGetValue(key, out ResourceLocator locator))
-                            {
-                                value = locator.Value;
-                            }
-                            if (value == null)
-                            {
-                                if (_dataPosition == -1)
-                                    value = _reader.GetValueForNameIndex(_currentName);
-                                else
-                                    value = _reader.LoadObject(_dataPosition);
-                                // If enumeration and subsequent lookups happen very
-                                // frequently in the same process, add a ResourceLocator
-                                // to _resCache here.  But WinForms enumerates and
-                                // just about everyone else does lookups.  So caching
-                                // here may bloat working set.
-                            }
+                            value = locator.Value;
                         }
                     }
+                    if (value is null)
+                    {
+                        if (_dataPosition == -1)
+                            value = _reader.GetValueForNameIndex(_currentName);
+                        else
+                            value = _reader.LoadObject(_dataPosition);
+                        // If enumeration and subsequent lookups happen very
+                        // frequently in the same process, add a ResourceLocator
+                        // to _resCache here (we'll also need to extend the lock block!).
+                        // But WinForms enumerates and just about everyone else does lookups.
+                        // So caching here may bloat working set.
+                    }
                     return new DictionaryEntry(key, value);
                 }
             }
diff --git a/src/libraries/System.Private.CoreLib/src/System/Resources/RuntimeResourceSet.cs b/src/libraries/System.Private.CoreLib/src/System/Resources/RuntimeResourceSet.cs
index 90c55d9f30406..82b579f1801f6 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Resources/RuntimeResourceSet.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Resources/RuntimeResourceSet.cs
@@ -276,6 +276,8 @@ private IDictionaryEnumerator GetEnumeratorHelper()
             object? value;
             ResourceLocator resEntry;
 
+            // Lock the cache first, then the reader (reader locks implicitly through its methods).
+            // Lock order MUST match ResourceReader.ResourceEnumerator.Entry to avoid deadlock.
             lock (cache)
             {
                 // Find the offset within the data section
@@ -288,7 +290,7 @@ private IDictionaryEnumerator GetEnumeratorHelper()
 
                     // When data type cannot be cached
                     dataPos = resEntry.DataPosition;
-                    return isString ? reader.LoadString(dataPos) : reader.LoadObject(dataPos, out _);
+                    return isString ? reader.LoadString(dataPos) : reader.LoadObject(dataPos);
                 }
 
                 dataPos = reader.FindPosForResource(key);
@@ -346,14 +348,11 @@ private IDictionaryEnumerator GetEnumeratorHelper()
             return value;
         }
 
-        private static object? ReadValue (ResourceReader reader, int dataPos, bool isString, out ResourceLocator locator)
+        private static object? ReadValue(ResourceReader reader, int dataPos, bool isString, out ResourceLocator locator)
         {
             object? value;
             ResourceTypeCode typeCode;
 
-            // Normally calling LoadString or LoadObject requires
-            // taking a lock.  Note that in this case, we took a
-            // lock before calling this method.
             if (isString)
             {
                 value = reader.LoadString(dataPos);
diff --git a/src/libraries/System.Resources.ResourceManager/tests/ResourceManagerTests.cs b/src/libraries/System.Resources.ResourceManager/tests/ResourceManagerTests.cs
index a7ab855896769..c2965a8e86b28 100644
--- a/src/libraries/System.Resources.ResourceManager/tests/ResourceManagerTests.cs
+++ b/src/libraries/System.Resources.ResourceManager/tests/ResourceManagerTests.cs
@@ -12,6 +12,9 @@
 using System.Diagnostics;
 using Microsoft.DotNet.RemoteExecutor;
 using Xunit;
+using System.Threading;
+using System.Threading.Tasks;
+using System.Collections;
 
 [assembly:NeutralResourcesLanguage("en")]
 
@@ -218,6 +221,46 @@ public static void IgnoreCase(string key, string expectedValue)
             Assert.Equal(expectedValue, manager.GetString(key.ToLower(), culture));
         }
 
+        /// <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.
+        /// This has historically been prone to thread-safety bugs because of the shared cache state and internal
+        /// method calls between RuntimeResourceSet and <see cref="ResourceReader"/>.
+        ///
+        /// Running with <paramref name="useEnumeratorEntry"/> TRUE replicates https://github.com/dotnet/runtime/issues/74868,
+        /// while running with FALSE replicates the error from https://github.com/dotnet/runtime/issues/74052.
+        /// </summary>
+        /// <param name="useEnumeratorEntry">
+        /// Whether to use <see cref="IDictionaryEnumerator.Entry"/> vs. <see cref="IDictionaryEnumerator.Key"/> when enumerating;
+        /// these follow fairly different code paths.
+        /// </param>
+        [Theory]
+        [InlineData(false)]
+        [InlineData(true)]
+        public static void TestResourceManagerIsSafeForConcurrentAccessAndEnumeration(bool useEnumeratorEntry)
+        {
+            ResourceManager manager = new("System.Resources.Tests.Resources.AToZResx", typeof(ResourceManagerTests).GetTypeInfo().Assembly);
+
+            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)));
+
+            void WaitForBarrierThenEnumerateResources()
+            {
+                barrier.SignalAndWait();
+
+                ResourceSet set = manager.GetResourceSet(CultureInfo.InvariantCulture, createIfNotExists: true, tryParents: true);
+                IDictionaryEnumerator enumerator = set.GetEnumerator();
+                while (enumerator.MoveNext())
+                {
+                    object key = useEnumeratorEntry ? enumerator.Entry.Key : enumerator.Key;
+                    manager.GetObject((string)key);
+                    Thread.Sleep(1);
+                }
+            }
+        }
 
         public static IEnumerable<object[]> EnglishNonStringResourceData()
         {
diff --git a/src/libraries/System.Resources.ResourceManager/tests/Resources/AToZResx.Designer.cs b/src/libraries/System.Resources.ResourceManager/tests/Resources/AToZResx.Designer.cs
new file mode 100644
index 0000000000000..047939cdc644a
--- /dev/null
+++ b/src/libraries/System.Resources.ResourceManager/tests/Resources/AToZResx.Designer.cs
@@ -0,0 +1,297 @@
+//------------------------------------------------------------------------------
+// <auto-generated>
+//     This code was generated by a tool.
+//     Runtime Version:4.0.30319.42000
+//
+//     Changes to this file may cause incorrect behavior and will be lost if
+//     the code is regenerated.
+// </auto-generated>
+//------------------------------------------------------------------------------
+
+namespace System.Resources.Tests.Resources {
+    using System;
+    
+    
+    /// <summary>
+    ///   A strongly-typed resource class, for looking up localized strings, etc.
+    /// </summary>
+    // This class was auto-generated by the StronglyTypedResourceBuilder
+    // class via a tool like ResGen or Visual Studio.
+    // To add or remove a member, edit your .ResX file then rerun ResGen
+    // with the /str option, or rebuild your VS project.
+    [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "17.0.0.0")]
+    [global::System.Diagnostics.DebuggerNonUserCodeAttribute()]
+    [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()]
+    internal class AToZResx {
+        
+        private static global::System.Resources.ResourceManager resourceMan;
+        
+        private static global::System.Globalization.CultureInfo resourceCulture;
+        
+        [global::System.Diagnostics.CodeAnalysis.SuppressMessageAttribute("Microsoft.Performance", "CA1811:AvoidUncalledPrivateCode")]
+        internal AToZResx() {
+        }
+        
+        /// <summary>
+        ///   Returns the cached ResourceManager instance used by this class.
+        /// </summary>
+        [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)]
+        internal static global::System.Resources.ResourceManager ResourceManager {
+            get {
+                if (object.ReferenceEquals(resourceMan, null)) {
+                    global::System.Resources.ResourceManager temp = new global::System.Resources.ResourceManager("System.Resources.Tests.Resources.AToZResx", typeof(AToZResx).Assembly);
+                    resourceMan = temp;
+                }
+                return resourceMan;
+            }
+        }
+        
+        /// <summary>
+        ///   Overrides the current thread's CurrentUICulture property for all
+        ///   resource lookups using this strongly typed resource class.
+        /// </summary>
+        [global::System.ComponentModel.EditorBrowsableAttribute(global::System.ComponentModel.EditorBrowsableState.Advanced)]
+        internal static global::System.Globalization.CultureInfo Culture {
+            get {
+                return resourceCulture;
+            }
+            set {
+                resourceCulture = value;
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to a.
+        /// </summary>
+        internal static string A {
+            get {
+                return ResourceManager.GetString("A", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to b.
+        /// </summary>
+        internal static string B {
+            get {
+                return ResourceManager.GetString("B", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to c.
+        /// </summary>
+        internal static string C {
+            get {
+                return ResourceManager.GetString("C", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to d.
+        /// </summary>
+        internal static string D {
+            get {
+                return ResourceManager.GetString("D", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to e.
+        /// </summary>
+        internal static string E {
+            get {
+                return ResourceManager.GetString("E", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to f.
+        /// </summary>
+        internal static string F {
+            get {
+                return ResourceManager.GetString("F", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to g.
+        /// </summary>
+        internal static string G {
+            get {
+                return ResourceManager.GetString("G", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to h.
+        /// </summary>
+        internal static string H {
+            get {
+                return ResourceManager.GetString("H", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to i.
+        /// </summary>
+        internal static string I {
+            get {
+                return ResourceManager.GetString("I", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to j.
+        /// </summary>
+        internal static string J {
+            get {
+                return ResourceManager.GetString("J", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to k.
+        /// </summary>
+        internal static string K {
+            get {
+                return ResourceManager.GetString("K", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to l.
+        /// </summary>
+        internal static string L {
+            get {
+                return ResourceManager.GetString("L", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to m.
+        /// </summary>
+        internal static string M {
+            get {
+                return ResourceManager.GetString("M", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to n.
+        /// </summary>
+        internal static string N {
+            get {
+                return ResourceManager.GetString("N", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to o.
+        /// </summary>
+        internal static string O {
+            get {
+                return ResourceManager.GetString("O", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to p.
+        /// </summary>
+        internal static string P {
+            get {
+                return ResourceManager.GetString("P", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to q.
+        /// </summary>
+        internal static string Q {
+            get {
+                return ResourceManager.GetString("Q", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to r.
+        /// </summary>
+        internal static string R {
+            get {
+                return ResourceManager.GetString("R", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to s.
+        /// </summary>
+        internal static string S {
+            get {
+                return ResourceManager.GetString("S", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to t.
+        /// </summary>
+        internal static string T {
+            get {
+                return ResourceManager.GetString("T", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to u.
+        /// </summary>
+        internal static string U {
+            get {
+                return ResourceManager.GetString("U", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to v.
+        /// </summary>
+        internal static string V {
+            get {
+                return ResourceManager.GetString("V", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to w.
+        /// </summary>
+        internal static string W {
+            get {
+                return ResourceManager.GetString("W", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to x.
+        /// </summary>
+        internal static string X {
+            get {
+                return ResourceManager.GetString("X", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to y.
+        /// </summary>
+        internal static string Y {
+            get {
+                return ResourceManager.GetString("Y", resourceCulture);
+            }
+        }
+        
+        /// <summary>
+        ///   Looks up a localized string similar to z.
+        /// </summary>
+        internal static string Z {
+            get {
+                return ResourceManager.GetString("Z", resourceCulture);
+            }
+        }
+    }
+}
diff --git a/src/libraries/System.Resources.ResourceManager/tests/Resources/AToZResx.resx b/src/libraries/System.Resources.ResourceManager/tests/Resources/AToZResx.resx
new file mode 100644
index 0000000000000..e1c5ddaba7ed8
--- /dev/null
+++ b/src/libraries/System.Resources.ResourceManager/tests/Resources/AToZResx.resx
@@ -0,0 +1,198 @@
+<?xml version="1.0" encoding="utf-8"?>
+<root>
+  <!-- 
+    Microsoft ResX Schema 
+    
+    Version 2.0
+    
+    The primary goals of this format is to allow a simple XML format 
+    that is mostly human readable. The generation and parsing of the 
+    various data types are done through the TypeConverter classes 
+    associated with the data types.
+    
+    Example:
+    
+    ... ado.net/XML headers & schema ...
+    <resheader name="resmimetype">text/microsoft-resx</resheader>
+    <resheader name="version">2.0</resheader>
+    <resheader name="reader">System.Resources.ResXResourceReader, System.Windows.Forms, ...</resheader>
+    <resheader name="writer">System.Resources.ResXResourceWriter, System.Windows.Forms, ...</resheader>
+    <data name="Name1"><value>this is my long string</value><comment>this is a comment</comment></data>
+    <data name="Color1" type="System.Drawing.Color, System.Drawing">Blue</data>
+    <data name="Bitmap1" mimetype="application/x-microsoft.net.object.binary.base64">
+        <value>[base64 mime encoded serialized .NET Framework object]</value>
+    </data>
+    <data name="Icon1" type="System.Drawing.Icon, System.Drawing" mimetype="application/x-microsoft.net.object.bytearray.base64">
+        <value>[base64 mime encoded string representing a byte array form of the .NET Framework object]</value>
+        <comment>This is a comment</comment>
+    </data>
+                
+    There are any number of "resheader" rows that contain simple 
+    name/value pairs.
+    
+    Each data row contains a name, and value. The row also contains a 
+    type or mimetype. Type corresponds to a .NET class that support 
+    text/value conversion through the TypeConverter architecture. 
+    Classes that don't support this are serialized and stored with the 
+    mimetype set.
+    
+    The mimetype is used for serialized objects, and tells the 
+    ResXResourceReader how to depersist the object. This is currently not 
+    extensible. For a given mimetype the value must be set accordingly:
+    
+    Note - application/x-microsoft.net.object.binary.base64 is the format 
+    that the ResXResourceWriter will generate, however the reader can 
+    read any of the formats listed below.
+    
+    mimetype: application/x-microsoft.net.object.binary.base64
+    value   : The object must be serialized with 
+            : System.Runtime.Serialization.Formatters.Binary.BinaryFormatter
+            : and then encoded with base64 encoding.
+    
+    mimetype: application/x-microsoft.net.object.soap.base64
+    value   : The object must be serialized with 
+            : System.Runtime.Serialization.Formatters.Soap.SoapFormatter
+            : and then encoded with base64 encoding.
+
+    mimetype: application/x-microsoft.net.object.bytearray.base64
+    value   : The object must be serialized into a byte array 
+            : using a System.ComponentModel.TypeConverter
+            : and then encoded with base64 encoding.
+    -->
+  <xsd:schema id="root" xmlns="" xmlns:xsd="http://www.w3.org/2001/XMLSchema" xmlns:msdata="urn:schemas-microsoft-com:xml-msdata">
+    <xsd:import namespace="http://www.w3.org/XML/1998/namespace" />
+    <xsd:element name="root" msdata:IsDataSet="true">
+      <xsd:complexType>
+        <xsd:choice maxOccurs="unbounded">
+          <xsd:element name="metadata">
+            <xsd:complexType>
+              <xsd:sequence>
+                <xsd:element name="value" type="xsd:string" minOccurs="0" />
+              </xsd:sequence>
+              <xsd:attribute name="name" use="required" type="xsd:string" />
+              <xsd:attribute name="type" type="xsd:string" />
+              <xsd:attribute name="mimetype" type="xsd:string" />
+              <xsd:attribute ref="xml:space" />
+            </xsd:complexType>
+          </xsd:element>
+          <xsd:element name="assembly">
+            <xsd:complexType>
+              <xsd:attribute name="alias" type="xsd:string" />
+              <xsd:attribute name="name" type="xsd:string" />
+            </xsd:complexType>
+          </xsd:element>
+          <xsd:element name="data">
+            <xsd:complexType>
+              <xsd:sequence>
+                <xsd:element name="value" type="xsd:string" minOccurs="0" msdata:Ordinal="1" />
+                <xsd:element name="comment" type="xsd:string" minOccurs="0" msdata:Ordinal="2" />
+              </xsd:sequence>
+              <xsd:attribute name="name" type="xsd:string" use="required" msdata:Ordinal="1" />
+              <xsd:attribute name="type" type="xsd:string" msdata:Ordinal="3" />
+              <xsd:attribute name="mimetype" type="xsd:string" msdata:Ordinal="4" />
+              <xsd:attribute ref="xml:space" />
+            </xsd:complexType>
+          </xsd:element>
+          <xsd:element name="resheader">
+            <xsd:complexType>
+              <xsd:sequence>
+                <xsd:element name="value" type="xsd:string" minOccurs="0" msdata:Ordinal="1" />
+              </xsd:sequence>
+              <xsd:attribute name="name" type="xsd:string" use="required" />
+            </xsd:complexType>
+          </xsd:element>
+        </xsd:choice>
+      </xsd:complexType>
+    </xsd:element>
+  </xsd:schema>
+  <resheader name="resmimetype">
+    <value>text/microsoft-resx</value>
+  </resheader>
+  <resheader name="version">
+    <value>2.0</value>
+  </resheader>
+  <resheader name="reader">
+    <value>System.Resources.ResXResourceReader, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
+  </resheader>
+  <resheader name="writer">
+    <value>System.Resources.ResXResourceWriter, System.Windows.Forms, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b77a5c561934e089</value>
+  </resheader>
+  <data name="A" xml:space="preserve">
+    <value>a</value>
+  </data>
+  <data name="B" xml:space="preserve">
+    <value>b</value>
+  </data>
+  <data name="C" xml:space="preserve">
+    <value>c</value>
+  </data>
+  <data name="D" xml:space="preserve">
+    <value>d</value>
+  </data>
+  <data name="E" xml:space="preserve">
+    <value>e</value>
+  </data>
+  <data name="F" xml:space="preserve">
+    <value>f</value>
+  </data>
+  <data name="G" xml:space="preserve">
+    <value>g</value>
+  </data>
+  <data name="H" xml:space="preserve">
+    <value>h</value>
+  </data>
+  <data name="I" xml:space="preserve">
+    <value>i</value>
+  </data>
+  <data name="J" xml:space="preserve">
+    <value>j</value>
+  </data>
+  <data name="K" xml:space="preserve">
+    <value>k</value>
+  </data>
+  <data name="L" xml:space="preserve">
+    <value>l</value>
+  </data>
+  <data name="M" xml:space="preserve">
+    <value>m</value>
+  </data>
+  <data name="N" xml:space="preserve">
+    <value>n</value>
+  </data>
+  <data name="O" xml:space="preserve">
+    <value>o</value>
+  </data>
+  <data name="P" xml:space="preserve">
+    <value>p</value>
+  </data>
+  <data name="Q" xml:space="preserve">
+    <value>q</value>
+  </data>
+  <data name="R" xml:space="preserve">
+    <value>r</value>
+  </data>
+  <data name="S" xml:space="preserve">
+    <value>s</value>
+  </data>
+  <data name="T" xml:space="preserve">
+    <value>t</value>
+  </data>
+  <data name="U" xml:space="preserve">
+    <value>u</value>
+  </data>
+  <data name="V" xml:space="preserve">
+    <value>v</value>
+  </data>
+  <data name="W" xml:space="preserve">
+    <value>w</value>
+  </data>
+  <data name="X" xml:space="preserve">
+    <value>x</value>
+  </data>
+  <data name="Y" xml:space="preserve">
+    <value>y</value>
+  </data>
+  <data name="Z" xml:space="preserve">
+    <value>z</value>
+  </data>
+</root>
\ No newline at end of file
diff --git a/src/libraries/System.Resources.ResourceManager/tests/System.Resources.ResourceManager.Tests.csproj b/src/libraries/System.Resources.ResourceManager/tests/System.Resources.ResourceManager.Tests.csproj
index 66cb937a3d1e2..f5ee7f0c22b59 100644
--- a/src/libraries/System.Resources.ResourceManager/tests/System.Resources.ResourceManager.Tests.csproj
+++ b/src/libraries/System.Resources.ResourceManager/tests/System.Resources.ResourceManager.Tests.csproj
@@ -8,6 +8,11 @@
   </PropertyGroup>
   <ItemGroup>
     <Compile Include="MissingManifestResourceExceptionTests.cs" />
+    <Compile Include="Resources\AToZResx.Designer.cs">
+      <DesignTime>True</DesignTime>
+      <AutoGen>True</AutoGen>
+      <DependentUpon>AToZResx.resx</DependentUpon>
+    </Compile>
     <Compile Include="TrimCompatibilityTests.cs" />
     <Compile Include="NeutralResourcesLanguageAttributeTests.cs" />
     <Compile Include="ResourceManagerTests.cs" />
@@ -19,10 +24,13 @@
     <Compile Include="SatelliteContractVersionAttributeTests.cs" />
     <Compile Include="MissingSatelliteAssemblyException.cs" />
     <Compile Include="ResourceSetTests.cs" />
-    <Compile Include="$(CommonTestPath)System\Drawing\Helpers.cs"
-             Link="Common\System\Drawing\Helpers.cs" />
+    <Compile Include="$(CommonTestPath)System\Drawing\Helpers.cs" Link="Common\System\Drawing\Helpers.cs" />
   </ItemGroup>
   <ItemGroup>
+    <EmbeddedResource Include="Resources\AToZResx.resx">
+      <Generator>ResXFileCodeGenerator</Generator>
+      <LastGenOutput>AToZResx.Designer.cs</LastGenOutput>
+    </EmbeddedResource>
     <EmbeddedResource Include="Resources\TestResx.netstandard17.resources">
       <WithCulture>false</WithCulture>
       <Type>Non-Resx</Type>
@@ -40,23 +48,14 @@
       <Generator>ResXFileCodeGenerator</Generator>
       <LastGenOutput>TestResx.Designer.cs</LastGenOutput>
     </EmbeddedResource>
-    <EmbeddedResource Include="Resources\TestResx.es.resx"/>
-    <EmbeddedResource Include="Resources\TestResx.es-MX.resx"/>
-    <EmbeddedResource Include="Resources\TestResx.fr-FR.resx"/>
-    <EmbeddedResource Include="Resources\TestClassWithoutNeutralResources.fr.resx"/>
+    <EmbeddedResource Include="Resources\TestResx.es.resx" />
+    <EmbeddedResource Include="Resources\TestResx.es-MX.resx" />
+    <EmbeddedResource Include="Resources\TestResx.fr-FR.resx" />
+    <EmbeddedResource Include="Resources\TestClassWithoutNeutralResources.fr.resx" />
     <EmbeddedResource Include="Resources\CustomReader.resx" />
-    <Content Include="Resources\TestResx.netstandard17.resources"
-          Link="%(Filename)%(Extension)"
-          CopyToOutputDirectory="PreserveNewest"
-          Visible="false" />
-    <Content Include="Resources\bitmap.bmp"
-          Link="%(Filename)%(Extension)"
-          CopyToOutputDirectory="PreserveNewest"
-          Visible="false" />
-    <Content Include="Resources\icon.ico"
-          Link="%(Filename)%(Extension)"
-          CopyToOutputDirectory="PreserveNewest"
-          Visible="false" />
+    <Content Include="Resources\TestResx.netstandard17.resources" Link="%(Filename)%(Extension)" CopyToOutputDirectory="PreserveNewest" Visible="false" />
+    <Content Include="Resources\bitmap.bmp" Link="%(Filename)%(Extension)" CopyToOutputDirectory="PreserveNewest" Visible="false" />
+    <Content Include="Resources\icon.ico" Link="%(Filename)%(Extension)" CopyToOutputDirectory="PreserveNewest" Visible="false" />
   </ItemGroup>
   <ItemGroup>
     <ProjectReference Include="$(LibrariesProjectRoot)System.Drawing.Common\src\System.Drawing.Common.csproj" />

From a87ccd00d9b8575bf487675f290514d352a221f8 Mon Sep 17 00:00:00 2001
From: Michael Adelson <mike.adelson314@gmail.com>
Date: Sat, 3 Sep 2022 08:03:43 -0400
Subject: [PATCH 2/6] Remove trailing whitespace

---
 .../src/System/Resources/ResourceReader.cs                      | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
index 36846b6a27897..4749aae015aba 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
@@ -63,7 +63,7 @@ public sealed partial class
         // initialization must be protected by lock (this).
         private BinaryReader _store;
         // Used by RuntimeResourceSet and this class's enumerator.
-        // Accesses must be protected by lock(_resCache). 
+        // Accesses must be protected by lock(_resCache).
         internal Dictionary<string, ResourceLocator>? _resCache;
         private long _nameSectionOffset;  // Offset to name section of file.
         private long _dataSectionOffset;  // Offset to Data section of file.

From e063428674fb91d7c2ce36b759a17c52720c254c Mon Sep 17 00:00:00 2001
From: Michael Adelson <mike.adelson314@gmail.com>
Date: Tue, 8 Nov 2022 22:38:07 -0500
Subject: [PATCH 3/6] Address feedback from
 https://github.com/dotnet/runtime/pull/75054

---
 .../System/Resources/ResourceReader.Core.cs   |  4 --
 .../src/System/Resources/ResourceReader.cs    | 16 +++----
 .../tests/BinaryResourceWriterUnitTest.cs     | 48 ++++++++++++++++++-
 3 files changed, 53 insertions(+), 15 deletions(-)

diff --git a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.Core.cs b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.Core.cs
index f5a4b1a440720..2153d23ee97c6 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.Core.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.Core.cs
@@ -51,8 +51,6 @@ internal ResourceReader(Stream stream, Dictionary<string, ResourceLocator> resCa
             "the user to only get one error.")]
         private object DeserializeObject(int typeIndex)
         {
-            Debug.Assert(Monitor.IsEntered(this));
-
             if (!AllowCustomResourceTypes)
             {
                 throw new NotSupportedException(SR.ResourceManager_ReflectionNotAllowed);
@@ -91,8 +89,6 @@ private object DeserializeObject(int typeIndex)
             "Custom readers as well as custom objects on the resources file are not observable by the trimmer and so required assemblies, types and members may be removed.")]
         private bool InitializeBinaryFormatter()
         {
-            Debug.Assert(Monitor.IsEntered(this));
-
             // If BinaryFormatter support is disabled for the app, the linker will replace this entire
             // method body with "return false;", skipping all reflection code below.
 
diff --git a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
index 4749aae015aba..50f2a28d26065 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
@@ -324,7 +324,7 @@ internal int FindPosForResource(string name)
         private unsafe bool CompareStringEqualsName(string name)
         {
             Debug.Assert(_store != null, "ResourceReader is closed!");
-            Debug.Assert(Monitor.IsEntered(this));
+            Debug.Assert(Monitor.IsEntered(this)); // uses _store
 
             int byteLen = _store.Read7BitEncodedInt();
             if (byteLen < 0)
@@ -525,7 +525,7 @@ private unsafe string AllocateStringForNameIndex(int index, out int dataOffset)
         {
             Debug.Assert(_store != null, "ResourceReader is closed!");
             Debug.Assert(_version == 1, ".resources file was not a V1 .resources file!");
-            Debug.Assert(Monitor.IsEntered(this));
+            Debug.Assert(Monitor.IsEntered(this)); // uses _store
 
             try
             {
@@ -545,7 +545,7 @@ private unsafe string AllocateStringForNameIndex(int index, out int dataOffset)
 
         private object? _LoadObjectV1(int pos)
         {
-            Debug.Assert(Monitor.IsEntered(this));
+            Debug.Assert(Monitor.IsEntered(this)); // uses _store
 
             _store.BaseStream.Seek(_dataSectionOffset + pos, SeekOrigin.Begin);
             int typeIndex = _store.Read7BitEncodedInt();
@@ -603,7 +603,7 @@ private unsafe string AllocateStringForNameIndex(int index, out int dataOffset)
         {
             Debug.Assert(_store != null, "ResourceReader is closed!");
             Debug.Assert(_version >= 2, ".resources file was not a V2 (or higher) .resources file!");
-            Debug.Assert(Monitor.IsEntered(this));
+            Debug.Assert(Monitor.IsEntered(this)); // uses _store
 
             try
             {
@@ -623,7 +623,7 @@ private unsafe string AllocateStringForNameIndex(int index, out int dataOffset)
 
         private object? _LoadObjectV2(int pos, out ResourceTypeCode typeCode)
         {
-            Debug.Assert(Monitor.IsEntered(this));
+            Debug.Assert(Monitor.IsEntered(this)); // uses _store
 
             _store.BaseStream.Seek(_dataSectionOffset + pos, SeekOrigin.Begin);
             typeCode = (ResourceTypeCode)_store.Read7BitEncodedInt();
@@ -954,8 +954,6 @@ private void _ReadResources()
             "the user to only get one error.")]
         private Type FindType(int typeIndex)
         {
-            Debug.Assert(Monitor.IsEntered(this));
-
             if (!AllowCustomResourceTypes)
             {
                 throw new NotSupportedException(SR.ResourceManager_ReflectionNotAllowed);
@@ -973,7 +971,7 @@ private Type FindType(int typeIndex)
             "Custom readers as well as custom objects on the resources file are not observable by the trimmer and so required assemblies, types and members may be removed.")]
         private Type UseReflectionToGetType(int typeIndex)
         {
-            Debug.Assert(Monitor.IsEntered(this));
+            Debug.Assert(Monitor.IsEntered(this)); // uses _store
 
             long oldPos = _store.BaseStream.Position;
             try
@@ -1007,7 +1005,7 @@ private Type UseReflectionToGetType(int typeIndex)
         private string TypeNameFromTypeCode(ResourceTypeCode typeCode)
         {
             Debug.Assert(typeCode >= 0, "can't be negative");
-            Debug.Assert(Monitor.IsEntered(this));
+            Debug.Assert(Monitor.IsEntered(this)); // uses _store
 
             if (typeCode < ResourceTypeCode.StartOfUserTypes)
             {
diff --git a/src/libraries/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs b/src/libraries/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs
index 378a416210d62..8101393d0e21b 100644
--- a/src/libraries/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs
+++ b/src/libraries/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs
@@ -10,6 +10,8 @@
 using System.Linq;
 using System.Reflection;
 using System.Runtime.Serialization.Formatters.Binary;
+using System.Threading;
+using System.Threading.Tasks;
 using Xunit;
 
 namespace System.Resources.Extensions.Tests
@@ -149,7 +151,6 @@ public static void EmptyResources()
             Assert.Equal(writerBuffer, binaryWriterBuffer);
         }
 
-
         [Fact]
         public static void PrimitiveResources()
         {
@@ -498,6 +499,50 @@ 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.
+        /// This has historically been prone to thread-safety bugs because of the shared cache state and internal
+        /// method calls between RuntimeResourceSet and <see cref="DeserializingResourceReader"/>.
+        ///
+        /// Running with <paramref name="useEnumeratorEntry"/> TRUE replicates https://github.com/dotnet/runtime/issues/74868,
+        /// while running with FALSE replicates the error from https://github.com/dotnet/runtime/issues/74052.
+        /// </summary>
+        /// <param name="useEnumeratorEntry">
+        /// Whether to use <see cref="IDictionaryEnumerator.Entry"/> vs. <see cref="IDictionaryEnumerator.Key"/> when enumerating;
+        /// these follow fairly different code paths.
+        /// </param>]
+        [InlineData(false)]
+        [InlineData(true)]
+        public static void TestResourceManagerIsSafeForConcurrentAccessAndEnumeration(bool useEnumeratorEntry)
+        {
+            ResourceManager manager = new(
+                typeof(TestData).FullName,
+                typeof(TestData).Assembly,
+                typeof(DeserializingResourceReader).Assembly.GetType("System.Resources.Extensions.RuntimeResourceSet", throwOnError: true));
+
+            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)));
+
+            void WaitForBarrierThenEnumerateResources()
+            {
+                barrier.SignalAndWait();
+
+                ResourceSet set = manager.GetResourceSet(CultureInfo.InvariantCulture, createIfNotExists: true, tryParents: true);
+                IDictionaryEnumerator enumerator = set.GetEnumerator();
+                while (enumerator.MoveNext())
+                {
+                    object key = useEnumeratorEntry ? enumerator.Entry.Key : enumerator.Key;
+                    manager.GetObject((string)key);
+                    Thread.Sleep(1);
+                }
+            }
+        }
+
         private static void ResourceValueEquals(object expected, object actual)
         {
             if (actual is Bitmap bitmap)
@@ -535,5 +580,4 @@ private static void BitmapEquals(Bitmap left, Bitmap right)
             }
         }
     }
-
 }

From d42bd879610059a67f3eccf6c63cf2bb5b6e352b Mon Sep 17 00:00:00 2001
From: Michael Adelson <mike.adelson314@gmail.com>
Date: Wed, 16 Nov 2022 19:49:46 -0500
Subject: [PATCH 4/6] Add comment in response to
 https://github.com/dotnet/runtime/pull/75054#issuecomment-1317470280

---
 .../src/System/Resources/ResourceReader.cs                     | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
index 50f2a28d26065..d6da2fb52150a 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
@@ -171,6 +171,9 @@ private static unsafe int ReadUnalignedI4(int* p)
 
         private void SkipString()
         {
+            // Note: this method assumes that it is called either during object
+            // construction or within another method that locks on this.
+
             int stringLength = _store.Read7BitEncodedInt();
             if (stringLength < 0)
             {

From c111a0e96b5f4aff7fc4105b44ee93c20dd3ebdb Mon Sep 17 00:00:00 2001
From: Michael Adelson <mike.adelson314@gmail.com>
Date: Sun, 11 Dec 2022 14:20:48 -0500
Subject: [PATCH 5/6] Implement feedback from
 https://github.com/dotnet/runtime/pull/75054

---
 .../src/System/Resources/ResourceReader.cs           |  1 +
 .../src/System/Resources/RuntimeResourceSet.cs       |  2 ++
 .../tests/BinaryResourceWriterUnitTest.cs            |  2 +-
 .../tests/ResourceManagerTests.cs                    | 12 +++++++++---
 4 files changed, 13 insertions(+), 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
index d6da2fb52150a..e47dd90ade8bc 100644
--- a/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
+++ b/src/libraries/System.Private.CoreLib/src/System/Resources/ResourceReader.cs
@@ -1092,6 +1092,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
index 82b579f1801f6..246686f506f9b 100644
--- 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
@@ -278,6 +279,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
index 8101393d0e21b..7ce7b3958f211 100644
--- a/src/libraries/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs
+++ b/src/libraries/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs
@@ -499,7 +499,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.
@@ -513,6 +512,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
index c2965a8e86b28..ea288f9186e4e 100644
--- a/src/libraries/System.Resources.ResourceManager/tests/ResourceManagerTests.cs
+++ b/src/libraries/System.Resources.ResourceManager/tests/ResourceManagerTests.cs
@@ -243,9 +243,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()
             {

From c2b5ab4ad4919df995af3f7a01b5f7bc9944eb9e Mon Sep 17 00:00:00 2001
From: madelson <1269046+madelson@users.noreply.github.com>
Date: Sat, 7 Jan 2023 15:10:22 -0500
Subject: [PATCH 6/6] Increase timeout for
 TestResourceManagerIsSafeForConcurrentAccessAndEnumeration (#80330)

This raises the timeout to 30s, the same as what we have for the equivalent ResourceManager test (https://github.com/dotnet/runtime/blob/15fcb990fe17348ab6ddde0939200b900169920b/src/libraries/System.Resources.ResourceManager/tests/ResourceManagerTests.cs#L255).

fix #80277
---
 .../tests/BinaryResourceWriterUnitTest.cs                       | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/libraries/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs b/src/libraries/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs
index 7ce7b3958f211..14aa23e28815b 100644
--- a/src/libraries/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs
+++ b/src/libraries/System.Resources.Extensions/tests/BinaryResourceWriterUnitTest.cs
@@ -526,7 +526,7 @@ public static void TestResourceManagerIsSafeForConcurrentAccessAndEnumeration(bo
             using Barrier barrier = new(Threads);
             Task task = Task.WhenAll(Enumerable.Range(0, Threads).Select(_ => Task.Run(WaitForBarrierThenEnumerateResources)));
 
-            Assert.True(task.Wait(TimeSpan.FromSeconds(10)));
+            Assert.True(task.Wait(TimeSpan.FromSeconds(30)));
 
             void WaitForBarrierThenEnumerateResources()
             {