fix: memory leak when profiling is enabled

src/Sentry/Internal/ConcurrentBagLite.cs

@@ -0,0 +1,73 @@
+namespace Sentry.Internal;
+
+/// <summary>
+/// A minimal replacement for <see cref="ConcurrentBag{T}"/>.
+///
+/// We're using this to avoid the same class of memory leak that <see cref="ConcurrentQueueLite{T}"/>
+/// was introduced to avoid. See https://github.com/getsentry/sentry-dotnet/issues/5113
+/// </summary>
+internal class ConcurrentBagLite<T> : IReadOnlyCollection<T>
+{
+    private readonly List<T> _items;
+
+    public ConcurrentBagLite()
+    {
+        _items = new List<T>();
+    }
+
+    public ConcurrentBagLite(IEnumerable<T> collection)
+    {
+        _items = new List<T>(collection);
+    }
+
+    public void Add(T item)
+    {
+        lock (_items)
+        {
+            _items.Add(item);
+        }
+    }
+
+    public int Count
+    {
+        get
+        {
+            lock (_items)
+            {
+                return _items.Count;
+            }
+        }
+    }
+
+    public bool IsEmpty => Count == 0;
+
+    public void Clear()
+    {
+        lock (_items)
+        {
+            _items.Clear();
+        }
+    }
+
+    public T[] ToArray()
+    {
+        lock (_items)
+        {
+            return _items.ToArray();
+        }
+    }
+
+    public IEnumerator<T> GetEnumerator()
+    {
+        // Return a snapshot to avoid holding the lock during iteration
+        // and to prevent InvalidOperationException if the collection is modified.
+        T[] snapshot;
+        lock (_items)
+        {
+            snapshot = _items.ToArray();
+        }
+        return ((IEnumerable<T>)snapshot).GetEnumerator();
+    }
+
+    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+}

src/Sentry/Internal/ConcurrentQueueLite.cs

@@ -6,7 +6,7 @@ namespace Sentry.Internal;
 /// We're using this due to a memory leak that happens when using ConcurrentQueue in the BackgroundWorker.
 /// See https://github.com/getsentry/sentry-dotnet/issues/2516
 /// </summary>
-internal class ConcurrentQueueLite<T>
+internal class ConcurrentQueueLite<T> : IReadOnlyCollection<T>
 {
     private readonly List<T> _queue = new();
 
@@ -74,4 +74,18 @@ public T[] ToArray()
             return _queue.ToArray();
         }
     }
+
+    public IEnumerator<T> GetEnumerator()
+    {
+        // Return a snapshot to avoid holding the lock during iteration
+        // and to prevent InvalidOperationException if the collection is modified.
+        T[] snapshot;
+        lock (_queue)
+        {
+            snapshot = _queue.ToArray();
+        }
+        return ((IEnumerable<T>)snapshot).GetEnumerator();
+    }
+
+    IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
 }

src/Sentry/Internal/UnsampledTransaction.cs

@@ -12,11 +12,7 @@ internal sealed class UnsampledTransaction : NoOpTransaction
     // Although it's a little bit wasteful to create separate individual class instances here when all we're going to
     // report to sentry is the span count (in the client report), SDK users may refer to things like
     // `ITransaction.Spans.Count`, so we create an actual collection
-#if NETCOREAPP2_0_OR_GREATER || NETSTANDARD2_1_OR_GREATER
-    private readonly ConcurrentBag<ISpan> _spans = [];
-#else
-    private ConcurrentBag<ISpan> _spans = [];
-#endif
+    private readonly ConcurrentBagLite<ISpan> _spans = new();
 
     private readonly IHub _hub;
     private readonly ITransactionContext _context;
@@ -117,10 +113,6 @@ public ISpan StartChild(string operation, SpanId spanId)
 
     private void ReleaseSpans()
     {
-#if NETCOREAPP2_0_OR_GREATER || NETSTANDARD2_1_OR_GREATER
         _spans.Clear();
-#else
-        _spans = [];
-#endif
     }
 }

src/Sentry/Scope.cs

@@ -51,29 +51,29 @@ internal SentryId LastEventId
     /// </summary>
     internal bool HasEvaluated => _hasEvaluated;
 
-    private readonly Lazy<ConcurrentBag<ISentryEventExceptionProcessor>> _lazyExceptionProcessors =
+    private readonly Lazy<ConcurrentBagLite<ISentryEventExceptionProcessor>> _lazyExceptionProcessors =
         new(LazyThreadSafetyMode.PublicationOnly);
 
     /// <summary>
     /// A list of exception processors.
     /// </summary>
-    internal ConcurrentBag<ISentryEventExceptionProcessor> ExceptionProcessors => _lazyExceptionProcessors.Value;
+    internal ConcurrentBagLite<ISentryEventExceptionProcessor> ExceptionProcessors => _lazyExceptionProcessors.Value;
 
-    private readonly Lazy<ConcurrentBag<ISentryEventProcessor>> _lazyEventProcessors =
+    private readonly Lazy<ConcurrentBagLite<ISentryEventProcessor>> _lazyEventProcessors =
         new(LazyThreadSafetyMode.PublicationOnly);
 
-    private readonly Lazy<ConcurrentBag<ISentryTransactionProcessor>> _lazyTransactionProcessors =
+    private readonly Lazy<ConcurrentBagLite<ISentryTransactionProcessor>> _lazyTransactionProcessors =
         new(LazyThreadSafetyMode.PublicationOnly);
 
     /// <summary>
     /// A list of event processors.
     /// </summary>
-    internal ConcurrentBag<ISentryEventProcessor> EventProcessors => _lazyEventProcessors.Value;
+    internal ConcurrentBagLite<ISentryEventProcessor> EventProcessors => _lazyEventProcessors.Value;
 
     /// <summary>
     /// A list of event processors.
     /// </summary>
-    internal ConcurrentBag<ISentryTransactionProcessor> TransactionProcessors => _lazyTransactionProcessors.Value;
+    internal ConcurrentBagLite<ISentryTransactionProcessor> TransactionProcessors => _lazyTransactionProcessors.Value;
 
     /// <summary>
     /// An event that fires when the scope evaluates.
@@ -259,11 +259,7 @@ public ITransactionTracer? Transaction
     /// <inheritdoc />
     public IReadOnlyList<string> Fingerprint { get; set; } = Array.Empty<string>();
 
-#if NETSTANDARD2_0 || NETFRAMEWORK
-    private ConcurrentQueue<Breadcrumb> _breadcrumbs = new();
-#else
-    private readonly ConcurrentQueue<Breadcrumb> _breadcrumbs = new();
-#endif
+    private readonly ConcurrentQueueLite<Breadcrumb> _breadcrumbs = new();
 
     /// <inheritdoc />
     public IReadOnlyCollection<Breadcrumb> Breadcrumbs => _breadcrumbs;
@@ -278,11 +274,7 @@ public ITransactionTracer? Transaction
     /// <inheritdoc />
     public IReadOnlyDictionary<string, string> Tags => _tags;
 
-#if NETSTANDARD2_0 || NETFRAMEWORK
-    private ConcurrentBag<SentryAttachment> _attachments = new();
-#else
-    private readonly ConcurrentBag<SentryAttachment> _attachments = new();
-#endif
+    private readonly ConcurrentBagLite<SentryAttachment> _attachments = new();
 
     /// <summary>
     /// Attachments.
@@ -435,11 +427,7 @@ public void Clear()
     /// </summary>
     public void ClearAttachments()
     {
-#if NETSTANDARD2_0 || NETFRAMEWORK
-        Interlocked.Exchange(ref _attachments, new());
-#else
         _attachments.Clear();
-#endif
         if (Options.EnableScopeSync)
         {
             Options.ScopeObserver?.ClearAttachments();
@@ -451,12 +439,7 @@ public void ClearAttachments()
     /// </summary>
     public void ClearBreadcrumbs()
     {
-#if NETSTANDARD2_0 || NETFRAMEWORK
-        // No Clear method on ConcurrentQueue for these target frameworks
-        Interlocked.Exchange(ref _breadcrumbs, new());
-#else
         _breadcrumbs.Clear();
-#endif
     }
 
     /// <summary>

src/Sentry/SdkVersion.cs

@@ -1,4 +1,5 @@
 using Sentry.Extensibility;
+using Sentry.Internal;
 using Sentry.Internal.Extensions;
 using Sentry.Reflection;
 
@@ -19,8 +20,8 @@ public sealed class SdkVersion : ISentryJsonSerializable
 
     internal static SdkVersion Instance => InstanceLazy.Value;
 
-    internal ConcurrentBag<SentryPackage> InternalPackages { get; set; } = new();
-    internal ConcurrentBag<string> Integrations { get; set; } = new();
+    internal ConcurrentBagLite<SentryPackage> InternalPackages { get; set; } = new();
+    internal ConcurrentBagLite<string> Integrations { get; set; } = new();
 
     /// <summary>
     /// SDK packages.
@@ -104,8 +105,8 @@ public static SdkVersion FromJson(JsonElement json)
 
         return new SdkVersion
         {
-            InternalPackages = new ConcurrentBag<SentryPackage>(packages),
-            Integrations = new ConcurrentBag<string>(integrations),
+            InternalPackages = new ConcurrentBagLite<SentryPackage>(packages),
+            Integrations = new ConcurrentBagLite<string>(integrations),
             Name = name,
             Version = version
         };

src/Sentry/TransactionTracer.cs

@@ -148,7 +148,7 @@ public IReadOnlyList<string> Fingerprint
         set => _fingerprint = value;
     }
 
-    private readonly ConcurrentBag<Breadcrumb> _breadcrumbs = new();
+    private readonly ConcurrentBagLite<Breadcrumb> _breadcrumbs = new();
 
     /// <inheritdoc />
     public IReadOnlyCollection<Breadcrumb> Breadcrumbs => _breadcrumbs;
@@ -165,11 +165,7 @@ public IReadOnlyList<string> Fingerprint
     /// <inheritdoc />
     public IReadOnlyDictionary<string, string> Tags => _tags;
 
-#if NETCOREAPP2_0_OR_GREATER || NETSTANDARD2_1_OR_GREATER
-    private readonly ConcurrentBag<ISpan> _spans = new();
-#else
-    private ConcurrentBag<ISpan> _spans = new();
-#endif
+    private readonly ConcurrentBagLite<ISpan> _spans = new();
 
     /// <inheritdoc />
     public IReadOnlyCollection<ISpan> Spans => _spans;
@@ -428,11 +424,7 @@ public string? Origin
 
     private void ReleaseSpans()
     {
-#if NETCOREAPP2_0_OR_GREATER || NETSTANDARD2_1_OR_GREATER
         _spans.Clear();
-#else
-        _spans = new ConcurrentBag<ISpan>();
-#endif
         _activeSpanTracker.Clear();
     }
 

test/Sentry.DiagnosticSource.Tests/SentrySqlListenerTests.cs

@@ -120,11 +120,10 @@ public void OnNext_KnownKey_GetSpanInvoked(string key, bool addConnectionSpan)
                 }));
 
         // Assert
-        var spans = fixture.Spans.Where(s => s.Operation != "abc");
+        var spans = fixture.Spans.Where(s => s.Operation != "abc").ToList();
         Assert.NotEmpty(spans);
 
-        var firstSpan = fixture.Spans.OrderByDescending(x => x.StartTimestamp).First();
-        Assert.True(GetValidator(key)(firstSpan));
+        Assert.True(GetValidator(key)(spans[0]));
     }
 
     [Theory]