-
-
Notifications
You must be signed in to change notification settings - Fork 206
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
Replaced ConcurrentBag<T> with SynchronizedCollection<T> #3434
Conversation
I'd be a little surprised if SyncronizedCollection is the right choice, if the test case was more representative of realworld use. What about ConcurrentQueue? |
We use From a functional perspective, we just need a thread safe mutable collection... so #3432 works around the original memory leak... so we could just keep using |
Resolves #3433
This is an experiment for a more comprehensive fix to #3432.
Important
The initial port of SychronizedCollection (from the WCF libraries) seemed to have some issues when running queries against the IEnumerable interface... various of our tests failed, indicating it wasn't a good idea to use the IEnumerable interfaces in a multithreaded environment. As a quick fix, so that we can check to see if ConcurrentBag is related to memory issues one of our SDK customers is seeing, any IEnumerable queries need to be run against
SychronizedCollection<T>.ToReadOnlyCollection()
, which creates a copy of all of the items in the list inside a lock. This will be very slow but is a quick and dirty way to address the thread safety issues.We definitely don't want to merge that code into our main branch, but it should allow us to put an alpha release together for testing purposes.
TODO
If ConcurrentBag turns out to be the cause of some of our memory issues and if we can build a more robust replacement for ConcurrentBag, we could also consider replacing some/all of the following:
#skip-changelog