fix: Remove AppDomain.CurrentDomain.ProcessExit hook on shutdown

CHANGELOG.md

@@ -9,6 +9,7 @@
 ### Fixes
 
 - Crontab validation when capturing checkins ([#4314](https://github.com/getsentry/sentry-dotnet/pull/4314))
+- AppDomain.CurrentDomain.ProcessExit hook is now removed on shutdown ([#4323](https://github.com/getsentry/sentry-dotnet/pull/4323))
 
 ### Dependencies
 

src/Sentry/Integrations/AppDomainProcessExitIntegration.cs

@@ -3,7 +3,7 @@
 
 namespace Sentry.Integrations;
 
-internal class AppDomainProcessExitIntegration : ISdkIntegration
+internal class AppDomainProcessExitIntegration : ITidySdkIntegration
 {
     private readonly IAppDomain _appDomain;
     private IHub? _hub;
@@ -24,4 +24,9 @@ internal void HandleProcessExit(object? sender, EventArgs e)
         _options?.LogInfo("AppDomain process exited: Disposing SDK.");
         (_hub as IDisposable)?.Dispose();
     }
+
+    public void Cleanup()
+    {
+        _appDomain.ProcessExit -= HandleProcessExit;
+    }
 }

src/Sentry/Integrations/ITidySdkIntegration.cs

@@ -0,0 +1,17 @@
+namespace Sentry.Integrations;
+
+/// <summary>
+/// Since we can't add methods to <see cref="ISdkIntegration"/> without it being  a breaking change, this interface
+/// allows us to add some cleanup logic as an internal interface. We can move this to <see cref="ISdkIntegration"/>
+/// in the next major release.
+/// </summary>
+internal interface ITidySdkIntegration : ISdkIntegration
+{
+    /// <summary>
+    /// Performs any necessary cleanup for the integration
+    /// </summary>
+    /// <remarks>
+    /// Called when the Hub is disposed
+    /// </remarks>
+    public void Cleanup();
+}

src/Sentry/Internal/Hub.cs

@@ -1,5 +1,6 @@
 using Sentry.Extensibility;
 using Sentry.Infrastructure;
+using Sentry.Integrations;
 using Sentry.Protocol.Envelopes;
 using Sentry.Protocol.Metrics;
 
@@ -14,6 +15,7 @@ internal class Hub : IHub, IDisposable
     private readonly SentryOptions _options;
     private readonly RandomValuesFactory _randomValuesFactory;
     private readonly IReplaySession _replaySession;
+    private readonly List<ITidySdkIntegration> _integrationsToCleanup = new();
 
 #if MEMORY_DUMP_SUPPORTED
     private readonly MemoryMonitor? _memoryMonitor;
@@ -84,6 +86,10 @@ internal Hub(
         {
             options.LogDebug("Registering integration: '{0}'.", integration.GetType().Name);
             integration.Register(this, options);
+            if (integration is ITidySdkIntegration tidyIntegration)
+            {
+                _integrationsToCleanup.Add(tidyIntegration);
+            }
         }
     }
 
@@ -772,6 +778,18 @@ public void Dispose()
             return;
         }
 
+        foreach (var integration in _integrationsToCleanup)
+        {
+            try
+            {
+                integration.Cleanup();
+            }
+            catch (Exception e)
+            {
+                _options.LogError("Failure to cleanup integration {0}: {1}", integration.GetType().Name, e);
+            }
+        }
+
 #if MEMORY_DUMP_SUPPORTED
         _memoryMonitor?.Dispose();
 #endif

test/Sentry.Tests/HubTests.cs

@@ -1988,6 +1988,80 @@ public void CaptureUserFeedback_InvalidEmail_FeedbackDropped(string email)
         _fixture.Client.Received(1).CaptureUserFeedback(Arg.Is<UserFeedback>(f => f.Email.IsNull()));
 #pragma warning restore CS0618 // Type or member is obsolete
     }
+
+    [Fact]
+    public void Dispose_IntegrationsWithCleanup_CleanupCalled()
+    {
+        // Arrange
+        var cleaned = new List<ITidySdkIntegration>();
+        var integration1 = Substitute.For<ITidySdkIntegration>();
+        integration1.When(i => i.Cleanup()).Do(_ => cleaned.Add(integration1));
+        var integration2 = Substitute.For<ITidySdkIntegration>();
+        integration2.When(i => i.Cleanup()).Do(_ => cleaned.Add(integration2));
+        var integration3 = Substitute.For<ISdkIntegration>();
+        _fixture.Options.AddIntegration(integration1);
+        _fixture.Options.AddIntegration(integration2);
+        _fixture.Options.AddIntegration(integration3);
+        var hub = _fixture.GetSut();
+
+        // Act
+        hub.Dispose();
+
+        // Assert
+        cleaned.Should().Contain(integration1);
+        cleaned.Should().Contain(integration2);
+    }
+
+    [Fact]
+    public void Dispose_CleanupThrowsException_ExceptionHandledAndLogged()
+    {
+        // Arrange
+        var cleaned = new List<ITidySdkIntegration>();
+        var integration1 = Substitute.For<ITidySdkIntegration>();
+        integration1.When(i => i.Cleanup()).Do(_ => cleaned.Add(integration1));
+        var integration2 = Substitute.For<ITidySdkIntegration>();
+        integration2.When(i => i.Cleanup()).Do(_ => throw new InvalidOperationException("Cleanup failed"));
+        var integration3 = Substitute.For<ITidySdkIntegration>();
+        integration3.When(i => i.Cleanup()).Do(_ => cleaned.Add(integration3));
+        _fixture.Options.AddIntegration(integration1);
+        _fixture.Options.AddIntegration(integration2);
+        _fixture.Options.AddIntegration(integration3);
+        _fixture.Options.Debug = true;
+        _fixture.Options.DiagnosticLogger = Substitute.For<IDiagnosticLogger>();
+        _fixture.Options.DiagnosticLogger!.IsEnabled(Arg.Any<SentryLevel>()).Returns(true);
+        var hub = _fixture.GetSut();
+
+        // Act
+        hub.Dispose();
+
+        // Assert
+        cleaned.Should().Contain(integration1);
+        cleaned.Should().NotContain(integration2);
+        cleaned.Should().Contain(integration3);
+        _fixture.Options.DiagnosticLogger.Received(1).Log(
+            SentryLevel.Error,
+            Arg.Is<string>(s => s.Contains("Failure to cleanup integration")),
+            Arg.Any<InvalidOperationException>(),
+            Arg.Any<object[]>());
+    }
+
+    [Fact]
+    public void Dispose_CalledMultipleTimes_CleanupCalledOnlyOnce()
+    {
+        // Arrange
+        var cleaned = 0;
+        var integration = Substitute.For<ITidySdkIntegration>();
+        integration.When(i => i.Cleanup()).Do(_ => cleaned++);
+        _fixture.Options.AddIntegration(integration);
+        var hub = _fixture.GetSut();
+
+        // Act
+        hub.Dispose();
+        hub.Dispose();
+
+        // Assert
+        cleaned.Should().Be(1);
+    }
 }
 
 #if NET6_0_OR_GREATER