Added TryGetDumpLocation tests

src/Sentry/Internal/GarbageCollectionMonitor.cs

@@ -1,17 +1,17 @@
 namespace Sentry.Internal;
 
 /// <summary>
-/// Simple class to determine when GarbageCollection occurs
+/// Simple class to detect when Full Garbage Collection occurs
 /// </summary>
-internal sealed class GarbageCollectionMonitor(Action onGarbageCollected)
+internal sealed class GarbageCollectionMonitor
 {
     private const int MaxGenerationThreshold = 10;
     private const int LargeObjectHeapThreshold = 10;
 
-    public void Start(CancellationToken cancellationToken) =>
-        Task.Run(() => MonitorGarbageCollection(cancellationToken), cancellationToken);
+    public static void Start(Action onGarbageCollected, CancellationToken cancellationToken) =>
+        Task.Run(() => MonitorGarbageCollection(onGarbageCollected, cancellationToken), cancellationToken);
 
-    private void MonitorGarbageCollection(CancellationToken cancellationToken)
+    private static void MonitorGarbageCollection(Action onGarbageCollected, CancellationToken cancellationToken)
     {
         GC.RegisterForFullGCNotification(MaxGenerationThreshold, LargeObjectHeapThreshold);
 

src/Sentry/Internal/Hub.cs

@@ -70,7 +70,7 @@ internal Hub(
 #if NET6_0_OR_GREATER && !(IOS || ANDROID)
         if (options.AutomaticHeapDumpMemoryThreshold is { } threshold)
         {
-            _memoryMonitor = new MemoryMonitor(threshold, options, CaptureHeapDump);
+            _memoryMonitor = new MemoryMonitor(options, threshold, CaptureHeapDump);
         }
 #endif
 
@@ -692,6 +692,10 @@ public void Dispose()
             return;
         }
 
+#if NET6_0_OR_GREATER && !(IOS || ANDROID)
+        _memoryMonitor?.Dispose();
+#endif
+
         try
         {
             CurrentClient.FlushAsync(_options.ShutdownTimeout).ConfigureAwait(false).GetAwaiter().GetResult();

src/Sentry/Internal/MemoryMonitor.cs

@@ -11,18 +11,17 @@
 
 namespace Sentry.Internal;
 
-internal class MemoryMonitor
+internal class MemoryMonitor : IDisposable
 {
     private readonly SentryOptions _options;
-    private readonly long _thresholdBytes;
     private readonly long _totalMemory;
+    internal readonly long _thresholdBytes;
     private bool _dumpTriggered;
-    private GarbageCollectionMonitor _gcMonitor;
-    CancellationTokenSource _cancellationTokenSource = new();
+    private readonly CancellationTokenSource _cancellationTokenSource = new();
 
     private Action<string> OnDumpCollected { get; }
 
-    public MemoryMonitor(short thresholdPercentage, SentryOptions options, Action<string> onDumpCollected)
+    public MemoryMonitor(SentryOptions options, short thresholdPercentage, Action<string> onDumpCollected)
     {
         if (thresholdPercentage is < 0 or > 100)
         {
@@ -37,8 +36,7 @@ public MemoryMonitor(short thresholdPercentage, SentryOptions options, Action<st
         _thresholdBytes = (long)Math.Ceiling(portion * _totalMemory);
         _options.LogInfo("Automatic heap dump enabled if memory usage exceeds {0:N0} bytes ({1}%)", _thresholdBytes, thresholdPercentage);
 
-        _gcMonitor = new GarbageCollectionMonitor(CheckMemoryUsage);
-        _gcMonitor.Start(_cancellationTokenSource.Token);
+        GarbageCollectionMonitor.Start(CheckMemoryUsage, _cancellationTokenSource.Token);
     }
 
     private void CheckMemoryUsage()
@@ -54,16 +52,22 @@ private void CheckMemoryUsage()
         // Trigger the event if the threshold is exceeded
         if (usedMemory > _thresholdBytes && !_dumpTriggered)
         {
+            _dumpTriggered = true;
             _options.LogDebug("Total Memory: {0:N0} bytes", _totalMemory);
             _options.LogDebug("Threshold: {0:N0} bytes", _thresholdBytes);
             _options.LogDebug("Memory used: {0:N0} bytes ({1:N2}%)", usedMemory, usedMemoryPercentage);
-            _dumpTriggered = true;
             CaptureMemoryDump();
         }
     }
 
     internal void CaptureMemoryDump()
     {
+        if (_options.DisableFileWrite)
+        {
+            _options.LogDebug("File write has been disabled via the options. Unable to create memory dump.");
+            return;
+        }
+
         var dumpFile = TryGetDumpLocation();
         if (dumpFile is null)
         {
@@ -102,14 +106,8 @@ internal void CaptureMemoryDump()
         OnDumpCollected(dumpFile);
     }
 
-    private string? TryGetDumpLocation()
+    internal string? TryGetDumpLocation()
     {
-        if (_options.DisableFileWrite)
-        {
-            _options.LogDebug("File write has been disabled via the options. Unable to create memory dump.");
-            return null;
-        }
-
         try
         {
             var rootPath = _options.CacheDirectoryPath ??
@@ -119,7 +117,7 @@ internal void CaptureMemoryDump()
 
             if (!fileSystem.CreateDirectory(directoryPath))
             {
-                _options.LogDebug("Failed to create a directory for memory dump ({0}).", directoryPath);
+                _options.LogWarning("Failed to create a directory for memory dump ({0}).", directoryPath);
                 return null;
             }
             _options.LogDebug("Created directory for heap dump ({0}).", directoryPath);
@@ -129,7 +127,8 @@ internal void CaptureMemoryDump()
             var filePath = Path.Combine(directoryPath, $"{timestamp}_{processId}.gcdump");
             if (fileSystem.FileExists(filePath))
             {
-                fileSystem.DeleteFile(filePath);
+                _options.LogWarning("Duplicate dump file detected.");
+                return null;
             }
 
             return filePath;
@@ -141,6 +140,11 @@ internal void CaptureMemoryDump()
             return null;
         }
     }
+
+    public void Dispose()
+    {
+        _cancellationTokenSource.Cancel();
+    }
 }
 
 #endif

test/Sentry.Testing/SentryOptionsExtensions.cs

@@ -0,0 +1,46 @@
+namespace Sentry.Testing;
+
+/// <summary>
+/// We most frequently call IDiagnosticLogger via extension methods on the SentryOptions class, which obscures what
+/// we're expecting to receive via mocks when writing tests. This class provides some test extensions that mirror the
+/// extensions provided for SentryOptions to make it easier to write tests that match the code they're testing.
+/// </summary>
+public static class SentryOptionsExtensions
+{
+    private static SentryOptions ReceivedLog(this SentryOptions substitute, SentryLevel level)
+    {
+        substitute.DiagnosticLogger.Received().Log(level, Arg.Any<string>(), null, Arg.Any<object[]>());
+        return substitute;
+    }
+
+    private static SentryOptions ReceivedLog(this SentryOptions substitute, SentryLevel level, string message, params object[] args)
+    {
+        substitute.DiagnosticLogger.Received().Log(level, message, null, args);
+        return substitute;
+    }
+
+    public static SentryOptions ReceivedLogInfo(this SentryOptions substitute)
+        => ReceivedLog(substitute, SentryLevel.Info);
+
+    public static SentryOptions ReceivedLogInfo(this SentryOptions substitute, string message, params object[] args)
+        => ReceivedLog(substitute, SentryLevel.Info, message, args);
+
+    public static SentryOptions ReceivedLogWarning(this SentryOptions substitute)
+        => ReceivedLog(substitute, SentryLevel.Warning);
+
+    public static SentryOptions ReceivedLogWarning(this SentryOptions substitute, string message, params object[] args)
+        => ReceivedLog(substitute, SentryLevel.Warning, message, args);
+
+    public static SentryOptions ReceivedLogError(this SentryOptions substitute)
+        => ReceivedLogError(substitute, string.Empty);
+
+    public static SentryOptions ReceivedLogError(this SentryOptions substitute, string message, params object[] args)
+        => ReceivedLog(substitute, SentryLevel.Error, message, args);
+
+    public static SentryOptions ReceivedLogError(this SentryOptions substitute, Exception exception, string message,
+        params object[] args)
+    {
+        substitute.DiagnosticLogger.Received().Log(SentryLevel.Error, message, exception, Arg.Any<object[]>());
+        return substitute;
+    }
+}

test/Sentry.Tests/Internals/MemoryMonitorTests.cs

@@ -0,0 +1,93 @@
+#if NET6_0_OR_GREATER && !(IOS || ANDROID)
+namespace Sentry.Tests.Internals;
+
+public class MemoryMonitorTests
+{
+    private class Fixture
+    {
+        public SentryOptions Options { get; set; } = new()
+        {
+            Dsn = ValidDsn,
+            Debug = true,
+            DiagnosticLogger = Substitute.For<IDiagnosticLogger>()
+        };
+        public Action<string> OnDumpCollected { get; set; } = _ => { };
+        public short ThresholdPercentage { get; set; } = 5;
+
+        public MemoryMonitor GetSut()
+        {
+            Options.DiagnosticLogger?.IsEnabled(Arg.Any<SentryLevel>()).Returns(true);
+            return new(Options, ThresholdPercentage, OnDumpCollected);
+        }
+    }
+
+    private readonly Fixture _fixture = new();
+
+    [Fact]
+    public void TryGetDumpLocation_DirectoryCreationFails_ReturnsNull()
+    {
+        // Arrange
+        _fixture.Options.FileSystem = Substitute.For<IFileSystem>();
+        _fixture.Options.FileSystem.CreateDirectory(Arg.Any<string>()).Returns(false);
+        using var sut = _fixture.GetSut();
+
+        // Act
+        var result = sut.TryGetDumpLocation();
+
+        // Assert
+        result.Should().BeNull();
+        _fixture.Options.FileSystem.Received().CreateDirectory(Arg.Any<string>());
+        _fixture.Options.FileSystem.DidNotReceive().FileExists(Arg.Any<string>());
+        _fixture.Options.ReceivedLogWarning("Failed to create a directory for memory dump ({0}).", Arg.Any<string>());
+    }
+
+    [Fact]
+    public void TryGetDumpLocation_DumpFileExists_ReturnsNull()
+    {
+        // Arrange
+        _fixture.Options.FileSystem = Substitute.For<IFileSystem>();
+        _fixture.Options.FileSystem.CreateDirectory(Arg.Any<string>()).Returns(true);
+        _fixture.Options.FileSystem.FileExists(Arg.Any<string>()).Returns(true);
+        using var sut = _fixture.GetSut();
+
+        // Act
+        var result = sut.TryGetDumpLocation();
+
+        // Assert
+        result.Should().BeNull();
+        _fixture.Options.FileSystem.Received().CreateDirectory(Arg.Any<string>());
+        _fixture.Options.FileSystem.Received().FileExists(Arg.Any<string>());
+        _fixture.Options.ReceivedLogWarning("Duplicate dump file detected.");
+    }
+
+    [Fact]
+    public void TryGetDumpLocation_Exception_LogsError()
+    {
+        // Arrange
+        _fixture.Options.FileSystem = Substitute.For<IFileSystem>();
+        _fixture.Options.FileSystem.CreateDirectory(Arg.Any<string>()).Throws(_ => new Exception());
+        using var sut = _fixture.GetSut();
+
+        // Act
+        var result = sut.TryGetDumpLocation();
+
+        // Assert
+        result.Should().BeNull();
+        _fixture.Options.ReceivedLogError(Arg.Any<Exception>(), "Failed to resolve appropriate memory dump location.");
+    }
+
+    [Fact]
+    public void TryGetDumpLocation_ReturnsFilePath()
+    {
+        // Arrange
+        _fixture.Options.FileSystem = new FakeFileSystem();
+        using var sut = _fixture.GetSut();
+
+        // Act
+        var result = sut.TryGetDumpLocation();
+
+        // Assert
+        result.Should().NotBeNull();
+    }
+}
+#endif