Replaced ConcurrentBag<T> with SynchronizedCollection<T>

[jamescrosswell]

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:

        <Sentry>  (13 usages found)
            <src>\<Sentry>\Scope.cs  (8 usages found)
                49 private readonly Lazy<ConcurrentBag<ISentryEventExceptionProcessor>> _lazyExceptionProcessors =
                57 private readonly Lazy<ConcurrentBag<ISentryEventProcessor>> _lazyEventProcessors =
                60 private readonly Lazy<ConcurrentBag<ISentryTransactionProcessor>> _lazyTransactionProcessors = _lazyTransactionProcessors.Value;
                224 private ConcurrentBag<SentryAttachment> _attachments = new();
            <src>\<Sentry>\SdkVersion.cs  (4 usages found)
                22 internal ConcurrentBag<SentryPackage> InternalPackages { get; set; } = new();
                23 internal ConcurrentBag<string> Integrations { get; set; } = new();
            <src>\<Sentry>\TransactionTracer.cs  (1 usage found)
                156 private readonly ConcurrentBag<Breadcrumb> _breadcrumbs = new();

#skip-changelog

[ericsampson]

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?

[jamescrosswell]

What about ConcurrentQueue?

We use ConcurrentBag<T> in a few different places. Where we bumped into a memory issue is using CB to store all the spans for a particular transaction tracer... in that scenario, we often have to look for a particular span based on some criteria (e.g. the span representing a database connection or the parent of the current span) so a queue definitely wouldn't work.

From a functional perspective, we just need a thread safe mutable collection... so List<T> is a good choice. I'm not actually sure how highly concurrent access would be to this collection. If someone was using Sentry to instrument a background worker that created a transaction and then spawned a bunch of threads that did stuff and created child spans for that transaction then yes, in that case concurrency might be an issue. The most common use case is probably more like the Sentry SDK creates a transaction (e.g. to represent an HTTP request) and then 10-20 child spans get created under that transaction (e.g. representing DB operations or outbound HTTP Client requests)... I don't think the performance of those kinds of workloads are likely to be concurrency bound (not within the context of a single transaction, in any case, which is what we're considering here).

#3432 works around the original memory leak... so we could just keep using ConcurrentBag<T>. That we had to deal with that memory leak at all is a concern though. The implementation of these concurrent data types is very complex and I think that makes them prone to errors that are hard to anticipate or reason about. We're seeing very unexpected/odd problems wherever they're used (we had another memory leak that was caused by ConcurrentQueue<T> for example) so if we can use simpler data structures, I'd like to.