diff --git a/CHANGELOG.md b/CHANGELOG.md index 550ebac042..a9adceeed1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - Custom ISentryEventProcessors are now run for native iOS events ([#4318](https://github.com/getsentry/sentry-dotnet/pull/4318)) - Crontab validation when capturing checkins ([#4314](https://github.com/getsentry/sentry-dotnet/pull/4314)) - Native AOT: link to static `lzma` on Linux/MUSL ([#4326](https://github.com/getsentry/sentry-dotnet/pull/4326)) +- AppDomain.CurrentDomain.ProcessExit hook is now removed on shutdown ([#4323](https://github.com/getsentry/sentry-dotnet/pull/4323)) ### Dependencies diff --git a/src/Sentry/Integrations/AppDomainProcessExitIntegration.cs b/src/Sentry/Integrations/AppDomainProcessExitIntegration.cs index 9fd9caebe2..07b222b4dd 100644 --- a/src/Sentry/Integrations/AppDomainProcessExitIntegration.cs +++ b/src/Sentry/Integrations/AppDomainProcessExitIntegration.cs @@ -3,7 +3,7 @@ namespace Sentry.Integrations; -internal class AppDomainProcessExitIntegration : ISdkIntegration +internal class AppDomainProcessExitIntegration : ISdkIntegration, IDisposable { 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 Dispose() + { + _appDomain.ProcessExit -= HandleProcessExit; + } } diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index ff9d5de43a..67318abc54 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/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 _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 IDisposable disposableIntegration) + { + _integrationsToCleanup.Add(disposableIntegration); + } } } @@ -772,6 +778,18 @@ public void Dispose() return; } + foreach (var integration in _integrationsToCleanup) + { + try + { + integration.Dispose(); + } + catch (Exception e) + { + _options.LogError("Failed to dispose integration {0}: {1}", integration.GetType().Name, e); + } + } + #if MEMORY_DUMP_SUPPORTED _memoryMonitor?.Dispose(); #endif diff --git a/test/Sentry.Tests/HubTests.cs b/test/Sentry.Tests/HubTests.cs index 864a92dd2d..85bf533df9 100644 --- a/test/Sentry.Tests/HubTests.cs +++ b/test/Sentry.Tests/HubTests.cs @@ -1988,6 +1988,100 @@ public void CaptureUserFeedback_InvalidEmail_FeedbackDropped(string email) _fixture.Client.Received(1).CaptureUserFeedback(Arg.Is(f => f.Email.IsNull())); #pragma warning restore CS0618 // Type or member is obsolete } + + private class TestDisposableIntegration : ISdkIntegration, IDisposable + { + public int Registered { get; private set; } + public int Disposed { get; private set; } + + public void Register(IHub hub, SentryOptions options) + { + Registered++; + } + + protected virtual void Cleanup() + { + Disposed++; + } + + public void Dispose() + { + Cleanup(); + } + } + + private class TestFlakyDisposableIntegration : TestDisposableIntegration + { + protected override void Cleanup() + { + throw new InvalidOperationException("Cleanup failed"); + } + } + + [Fact] + public void Dispose_IntegrationsWithCleanup_CleanupCalled() + { + // Arrange + var integration1 = new TestDisposableIntegration(); + var integration2 = Substitute.For(); + var integration3 = new TestDisposableIntegration(); + _fixture.Options.AddIntegration(integration1); + _fixture.Options.AddIntegration(integration2); + _fixture.Options.AddIntegration(integration3); + var hub = _fixture.GetSut(); + + // Act + hub.Dispose(); + + // Assert + integration1.Disposed.Should().Be(1); + integration3.Disposed.Should().Be(1); + } + + [Fact] + public void Dispose_CleanupThrowsException_ExceptionHandledAndLogged() + { + // Arrange + var integration1 = new TestDisposableIntegration(); + var integration2 = new TestFlakyDisposableIntegration(); + var integration3 = new TestDisposableIntegration(); + _fixture.Options.AddIntegration(integration1); + _fixture.Options.AddIntegration(integration2); + _fixture.Options.AddIntegration(integration3); + _fixture.Options.Debug = true; + _fixture.Options.DiagnosticLogger = Substitute.For(); + _fixture.Options.DiagnosticLogger!.IsEnabled(Arg.Any()).Returns(true); + var hub = _fixture.GetSut(); + + // Act + hub.Dispose(); + + // Assert + integration1.Disposed.Should().Be(1); + integration2.Disposed.Should().Be(0); + integration3.Disposed.Should().Be(1); + _fixture.Options.DiagnosticLogger.Received(1).Log( + SentryLevel.Error, + Arg.Is(s => s.Contains("Failed to dispose integration")), + Arg.Any(), + Arg.Any()); + } + + [Fact] + public void Dispose_CalledMultipleTimes_CleanupCalledOnlyOnce() + { + // Arrange + var integration = new TestDisposableIntegration(); + _fixture.Options.AddIntegration(integration); + var hub = _fixture.GetSut(); + + // Act + hub.Dispose(); + hub.Dispose(); + + // Assert + integration.Disposed.Should().Be(1); + } } #if NET6_0_OR_GREATER