From eedb57dff27884ee4d3539ce3eda283408aea3ae Mon Sep 17 00:00:00 2001
From: Pavel Pochobut <pavel.pochobut@gmail.com>
Date: Wed, 4 Feb 2015 21:49:32 +0300
Subject: [PATCH] Fixed deadlock in System.Resources.ResourceSet

ResourceSet internal enumerator uses the following lock order:
1. lock _resCache (as _reader._resCache)
       in System.Resources.ResourceSet+ResourceEnumerator.Entry
2. lock reader (this)
       in the body on of _reader.AllocateStringForNameIndex

RuntimeResourceSet.GetObject uses the following lock order:
1. Lock on reader
2. Lock on _resCache
    (on the same instance as passed to the reader ctor)

That causes deadlock when one thread tries to enumerate the ResourceSet
while another thread reads a specific object from the same set.

Added lock in System.Resources.ResourceSet+ResourceEnumerator.Entry,
so both code paths locks on reader first.

Fix Connect Issue ID #580259
---
 .../src/System/Resources/ResourceReader.cs    | 34 ++++++++++---------
 1 file changed, 18 insertions(+), 16 deletions(-)

diff --git a/src/mscorlib/src/System/Resources/ResourceReader.cs b/src/mscorlib/src/System/Resources/ResourceReader.cs
index 4430ca6328e5..a811fae19476 100644
--- a/src/mscorlib/src/System/Resources/ResourceReader.cs
+++ b/src/mscorlib/src/System/Resources/ResourceReader.cs
@@ -1373,22 +1373,24 @@ public DictionaryEntry Entry {
 
                     String key;
                     Object value = null;
-                    lock (_reader._resCache) {
-                        key = _reader.AllocateStringForNameIndex(_currentName, out _dataPosition);
-                        ResourceLocator locator;
-                        if (_reader._resCache.TryGetValue(key, out 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.
+                    lock (_reader) { // locks should be taken in the same order as in RuntimeResourceSet.GetObject to avoid deadlock
+                        lock (_reader._resCache) {
+                            key = _reader.AllocateStringForNameIndex(_currentName, out _dataPosition); // AllocateStringForNameIndex could lock on _reader
+                            ResourceLocator locator;
+                            if (_reader._resCache.TryGetValue(key, out 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.
+                            }
                         }
                     }
                     return new DictionaryEntry(key, value);