Merge pull request #628 from Cronan/lock_recursion

den-run-ai · web-flow · commit df2aa64f192d · 2018-02-25T19:22:23.000-06:00
Fixes lock recursion bug in assembly loading
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -29,6 +29,7 @@ This document follows the conventions laid out in [Keep a CHANGELOG][].
 -   Fixed 'clrmethod' for python 2 (#492)
 -   Fixed double calling of constructor when deriving from .NET class (#495)
 -   Fixed `clr.GetClrType` when iterating over `System` members (#607) 
+-   Fixed `LockRecursionException` when loading assemblies (#627)
 
 
 ## [2.3.0][] - 2017-03-11
diff --git a/src/runtime/assemblymanager.cs b/src/runtime/assemblymanager.cs
@@ -26,7 +26,7 @@ internal class AssemblyManager
         private static Dictionary<string, int> probed;
 
         // modified from event handlers below, potentially triggered from different .NET threads
-        private static AssemblyList assemblies;
+        private static ConcurrentQueue<Assembly> assemblies;
         internal static List<string> pypath;
 
         private AssemblyManager()
@@ -43,7 +43,7 @@ internal static void Initialize()
             namespaces = new ConcurrentDictionary<string, ConcurrentDictionary<Assembly, string>>();
             probed = new Dictionary<string, int>(32);
             //generics = new Dictionary<string, Dictionary<string, string>>();
-            assemblies = new AssemblyList(16);
+            assemblies = new ConcurrentQueue<Assembly>();
             pypath = new List<string>(16);
 
             AppDomain domain = AppDomain.CurrentDomain;
@@ -60,7 +60,7 @@ internal static void Initialize()
                 try
                 {
                     ScanAssembly(a);
-                    assemblies.Add(a);
+                    assemblies.Enqueue(a);
                 }
                 catch (Exception ex)
                 {
@@ -91,7 +91,7 @@ internal static void Shutdown()
         private static void AssemblyLoadHandler(object ob, AssemblyLoadEventArgs args)
         {
             Assembly assembly = args.LoadedAssembly;
-            assemblies.Add(assembly);
+            assemblies.Enqueue(assembly);
             ScanAssembly(assembly);
         }
 
@@ -461,103 +461,5 @@ public static Type LookupType(string qname)
             }
             return null;
         }
-
-        /// <summary>
-        /// Wrapper around List&lt;Assembly&gt; for thread safe access
-        /// </summary>
-        private class AssemblyList : IEnumerable<Assembly>
-        {
-            private readonly List<Assembly> _list;
-            private readonly ReaderWriterLockSlim _lock;
-
-            public AssemblyList(int capacity)
-            {
-                _list = new List<Assembly>(capacity);
-                _lock = new ReaderWriterLockSlim();
-            }
-
-            public int Count
-            {
-                get
-                {
-                    _lock.EnterReadLock();
-                    try
-                    {
-                        return _list.Count;
-                    }
-                    finally
-                    {
-                        _lock.ExitReadLock();
-                    }
-                }
-            }
-
-            public void Add(Assembly assembly)
-            {
-                _lock.EnterWriteLock();
-                try
-                {
-                    _list.Add(assembly);
-                }
-                finally
-                {
-                    _lock.ExitWriteLock();
-                }
-            }
-
-            public IEnumerator GetEnumerator()
-            {
-                return ((IEnumerable<Assembly>)this).GetEnumerator();
-            }
-
-            /// <summary>
-            /// Enumerator wrapping around <see cref="AssemblyList._list" />'s enumerator.
-            /// Acquires and releases a read lock on <see cref="AssemblyList._lock" /> during enumeration
-            /// </summary>
-            private class Enumerator : IEnumerator<Assembly>
-            {
-                private readonly AssemblyList _assemblyList;
-
-                private readonly IEnumerator<Assembly> _listEnumerator;
-
-                public Enumerator(AssemblyList assemblyList)
-                {
-                    _assemblyList = assemblyList;
-                    _assemblyList._lock.EnterReadLock();
-                    _listEnumerator = _assemblyList._list.GetEnumerator();
-                }
-
-                public void Dispose()
-                {
-                    _listEnumerator.Dispose();
-                    _assemblyList._lock.ExitReadLock();
-                }
-
-                public bool MoveNext()
-                {
-                    return _listEnumerator.MoveNext();
-                }
-
-                public void Reset()
-                {
-                    _listEnumerator.Reset();
-                }
-
-                public Assembly Current
-                {
-                    get { return _listEnumerator.Current; }
-                }
-
-                object IEnumerator.Current
-                {
-                    get { return Current; }
-                }
-            }
-
-            IEnumerator<Assembly> IEnumerable<Assembly>.GetEnumerator()
-            {
-                return new Enumerator(this);
-            }
-        }
     }
 }
diff --git a/src/tests/test_module.py b/src/tests/test_module.py
@@ -387,3 +387,9 @@ def test_assembly_load_thread_safety():
         from System.Collections.Generic import Dictionary
         _ = Dictionary[Guid, DateTime]()
     ModuleTest.JoinThreads()
+
+def test_assembly_load_recursion_bug():
+    """Test fix for recursion bug documented in #627"""
+    from System.Configuration import ConfigurationManager
+    content = dir(ConfigurationManager)
+    assert len(content) > 5, content
