| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
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.
|
Sorry, something went wrong.
Sorry, something went wrong.
|
@Cronan I'm all for not using our own implementation of concurrent collection and use .NET types :) Here is one case to consider: var names = new List<AssemblyName>(assemblies.Count); Is it possible that new elements are added to assemblies concurrently before the next line with iteration starts and hence .Count is not correct for the size of names anymore? |
Sorry, something went wrong.
|
@denfromufa that line is just a hint to pre-allocate assemblies.Count slots in the List and if you get that wrong the only implication is that it needs to reallocate the internal array in the List. 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. |
Sorry, something went wrong.
|
@ArvidJB @denfromufa ConcurrentQueue would preserve the ordering, and also allow threadsafe snapshot enumeration. |
Sorry, something went wrong.
|
this PR complies with point 1 from these new rules: |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
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.