-
Notifications
You must be signed in to change notification settings - Fork 747
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
Conversation
Codecov Report
@@ Coverage Diff @@
## master #628 +/- ##
=========================================
- Coverage 76.99% 76.9% -0.09%
=========================================
Files 64 64
Lines 5612 5582 -30
Branches 888 888
=========================================
- Hits 4321 4293 -28
+ Misses 1002 1000 -2
Partials 289 289
Continue to review full report at Codecov.
|
@Cronan I'm all for not using our own implementation of concurrent collection and use .NET types :) Here is one case to consider:
Is it possible that new elements are added to |
@denfromufa that line is just a hint to pre-allocate I think the only real difference here is that ConcurrentBag does not try to maintain any ordering of the elements you add. The ordering of the assemblies would dictate the resolution for classes which are present in multiple assemblies (not sure how frequent this actually is). Switching the order during the evaluation of the program could lead to inconsistent class definitions. |
@ArvidJB so what do you recommend? |
@ArvidJB @denfromufa ConcurrentQueue would preserve the ordering, and also allow threadsafe snapshot enumeration. |
this PR complies with point 1 from these new rules: |
What does this implement/fix?
The AssemblyList class has a bug where assembly loading race conditions can cause a LockRecursionException under certain circumstances. Replacing the custom implementation with a
ConcurrentBag<Assembly>
solves the problem.Does this close any currently open issues?
This closes #627
Any other comments?
The downside of ConcurrentBag is that it doesn't support accessing items via the index, and it is an unordered collection, neither of which matters in the way we are using it.
Checklist
Check all those that are applicable and complete.
AUTHORS
CHANGELOG