Skip to content

Fixes lock recursion bug in assembly loading #628

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 26, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
106 changes: 4 additions & 102 deletions src/runtime/assemblymanager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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;
Expand All @@ -60,7 +60,7 @@ internal static void Initialize()
try
{
ScanAssembly(a);
assemblies.Add(a);
assemblies.Enqueue(a);
}
catch (Exception ex)
{
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}
}
}
}
6 changes: 6 additions & 0 deletions src/tests/test_module.py
Original file line number Diff line number Diff line change
Expand Up @@ -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