diff --git a/CHANGELOG.md b/CHANGELOG.md index 80e9cef14d..72bcff1f29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Allow multiple UncaughtExceptionHandlerIntegrations to be active at the same time ([#4462](https://github.com/getsentry/sentry-java/pull/4462)) + ## 8.13.2 ### Fixes diff --git a/sentry/src/main/java/io/sentry/UncaughtExceptionHandlerIntegration.java b/sentry/src/main/java/io/sentry/UncaughtExceptionHandlerIntegration.java index 1cf4c151b0..dd3bc8e86b 100644 --- a/sentry/src/main/java/io/sentry/UncaughtExceptionHandlerIntegration.java +++ b/sentry/src/main/java/io/sentry/UncaughtExceptionHandlerIntegration.java @@ -10,6 +10,7 @@ import io.sentry.hints.TransactionEnd; import io.sentry.protocol.Mechanism; import io.sentry.protocol.SentryId; +import io.sentry.util.AutoClosableReentrantLock; import io.sentry.util.HintUtils; import io.sentry.util.Objects; import java.io.Closeable; @@ -28,6 +29,8 @@ public final class UncaughtExceptionHandlerIntegration /** Reference to the pre-existing uncaught exception handler. */ private @Nullable Thread.UncaughtExceptionHandler defaultExceptionHandler; + private static final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); + private @Nullable IScopes scopes; private @Nullable SentryOptions options; @@ -65,27 +68,33 @@ public final void register(final @NotNull IScopes scopes, final @NotNull SentryO this.options.isEnableUncaughtExceptionHandler()); if (this.options.isEnableUncaughtExceptionHandler()) { - final Thread.UncaughtExceptionHandler currentHandler = - threadAdapter.getDefaultUncaughtExceptionHandler(); - if (currentHandler != null) { - this.options - .getLogger() - .log( - SentryLevel.DEBUG, - "default UncaughtExceptionHandler class='" - + currentHandler.getClass().getName() - + "'"); - - if (currentHandler instanceof UncaughtExceptionHandlerIntegration) { - final UncaughtExceptionHandlerIntegration currentHandlerIntegration = - (UncaughtExceptionHandlerIntegration) currentHandler; - defaultExceptionHandler = currentHandlerIntegration.defaultExceptionHandler; - } else { - defaultExceptionHandler = currentHandler; + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + final Thread.UncaughtExceptionHandler currentHandler = + threadAdapter.getDefaultUncaughtExceptionHandler(); + if (currentHandler != null) { + this.options + .getLogger() + .log( + SentryLevel.DEBUG, + "default UncaughtExceptionHandler class='" + + currentHandler.getClass().getName() + + "'"); + if (currentHandler instanceof UncaughtExceptionHandlerIntegration) { + final UncaughtExceptionHandlerIntegration currentHandlerIntegration = + (UncaughtExceptionHandlerIntegration) currentHandler; + if (currentHandlerIntegration.scopes != null + && scopes.getGlobalScope() == currentHandlerIntegration.scopes.getGlobalScope()) { + defaultExceptionHandler = currentHandlerIntegration.defaultExceptionHandler; + } else { + defaultExceptionHandler = currentHandler; + } + } else { + defaultExceptionHandler = currentHandler; + } } - } - threadAdapter.setDefaultUncaughtExceptionHandler(this); + threadAdapter.setDefaultUncaughtExceptionHandler(this); + } this.options .getLogger() @@ -159,11 +168,36 @@ static Throwable getUnhandledThrowable( @Override public void close() { - if (this == threadAdapter.getDefaultUncaughtExceptionHandler()) { - threadAdapter.setDefaultUncaughtExceptionHandler(defaultExceptionHandler); + try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { + if (this == threadAdapter.getDefaultUncaughtExceptionHandler()) { + threadAdapter.setDefaultUncaughtExceptionHandler(defaultExceptionHandler); + + if (options != null) { + options + .getLogger() + .log(SentryLevel.DEBUG, "UncaughtExceptionHandlerIntegration removed."); + } + } else { + removeFromHandlerTree(threadAdapter.getDefaultUncaughtExceptionHandler()); + } + } + } - if (options != null) { - options.getLogger().log(SentryLevel.DEBUG, "UncaughtExceptionHandlerIntegration removed."); + private void removeFromHandlerTree(@Nullable Thread.UncaughtExceptionHandler currentHandler) { + if (currentHandler instanceof UncaughtExceptionHandlerIntegration) { + final UncaughtExceptionHandlerIntegration currentHandlerIntegration = + (UncaughtExceptionHandlerIntegration) currentHandler; + if (this == currentHandlerIntegration.defaultExceptionHandler) { + currentHandlerIntegration.defaultExceptionHandler = defaultExceptionHandler; + + if (options != null) { + options + .getLogger() + .log(SentryLevel.DEBUG, "UncaughtExceptionHandlerIntegration removed."); + } + + } else { + removeFromHandlerTree(currentHandlerIntegration.defaultExceptionHandler); } } } diff --git a/sentry/src/test/java/io/sentry/UncaughtExceptionHandlerIntegrationTest.kt b/sentry/src/test/java/io/sentry/UncaughtExceptionHandlerIntegrationTest.kt index 409d3b971b..2bff144824 100644 --- a/sentry/src/test/java/io/sentry/UncaughtExceptionHandlerIntegrationTest.kt +++ b/sentry/src/test/java/io/sentry/UncaughtExceptionHandlerIntegrationTest.kt @@ -19,6 +19,8 @@ import org.mockito.kotlin.whenever import java.io.ByteArrayOutputStream import java.io.PrintStream import java.nio.file.Files +import java.util.concurrent.CompletableFuture +import java.util.concurrent.Executors import kotlin.concurrent.thread import kotlin.test.Test import kotlin.test.assertEquals @@ -313,7 +315,7 @@ class UncaughtExceptionHandlerIntegrationTest { val integration2 = UncaughtExceptionHandlerIntegration(handler) integration2.register(fixture.scopes, fixture.options) - assertEquals(currentDefaultHandler, integration2) + assertEquals(integration2, currentDefaultHandler) integration2.close() assertEquals(null, currentDefaultHandler) @@ -344,4 +346,125 @@ class UncaughtExceptionHandlerIntegrationTest { assertEquals(initialUncaughtExceptionHandler, currentDefaultHandler) } + + @Test + fun `multiple registrations with different global scopes allowed`() { + val scopes2 = mock() + val initialUncaughtExceptionHandler = Thread.UncaughtExceptionHandler { _, _ -> } + + var currentDefaultHandler: Thread.UncaughtExceptionHandler? = initialUncaughtExceptionHandler + + val handler = mock() + whenever(handler.defaultUncaughtExceptionHandler).thenAnswer { currentDefaultHandler } + + whenever(handler.setDefaultUncaughtExceptionHandler(anyOrNull())).then { + currentDefaultHandler = it.getArgument(0) + null + } + + whenever(scopes2.globalScope).thenReturn(mock()) + + val integration1 = UncaughtExceptionHandlerIntegration(handler) + integration1.register(fixture.scopes, fixture.options) + + val integration2 = UncaughtExceptionHandlerIntegration(handler) + integration2.register(scopes2, fixture.options) + + assertEquals(currentDefaultHandler, integration2) + integration2.close() + + assertEquals(integration1, currentDefaultHandler) + integration1.close() + + assertEquals(initialUncaughtExceptionHandler, currentDefaultHandler) + } + + @Test + fun `multiple registrations with different global scopes allowed, closed out of order`() { + fixture.getSut() + val scopes2 = mock() + val initialUncaughtExceptionHandler = Thread.UncaughtExceptionHandler { _, _ -> } + + var currentDefaultHandler: Thread.UncaughtExceptionHandler? = initialUncaughtExceptionHandler + + val handler = mock() + whenever(handler.defaultUncaughtExceptionHandler).thenAnswer { currentDefaultHandler } + + whenever(handler.setDefaultUncaughtExceptionHandler(anyOrNull())).then { + currentDefaultHandler = it.getArgument(0) + null + } + + whenever(scopes2.globalScope).thenReturn(mock()) + + val integration1 = UncaughtExceptionHandlerIntegration(handler) + integration1.register(fixture.scopes, fixture.options) + + val integration2 = UncaughtExceptionHandlerIntegration(handler) + integration2.register(scopes2, fixture.options) + + assertEquals(currentDefaultHandler, integration2) + integration1.close() + + assertEquals(integration2, currentDefaultHandler) + integration2.close() + + assertEquals(initialUncaughtExceptionHandler, currentDefaultHandler) + } + + @Test + fun `multiple registrations async, closed async, one remains`() { + val executor = Executors.newFixedThreadPool(4) + fixture.getSut() + val scopes2 = mock() + val scopes3 = mock() + val scopes4 = mock() + val scopes5 = mock() + + val scopesList = listOf(fixture.scopes, scopes2, scopes3, scopes4, scopes5) + + val initialUncaughtExceptionHandler = Thread.UncaughtExceptionHandler { _, _ -> } + + var currentDefaultHandler: Thread.UncaughtExceptionHandler? = initialUncaughtExceptionHandler + + val handler = mock() + whenever(handler.defaultUncaughtExceptionHandler).thenAnswer { currentDefaultHandler } + + whenever(handler.setDefaultUncaughtExceptionHandler(anyOrNull())).then { + currentDefaultHandler = it.getArgument(0) + null + } + + whenever(scopes2.globalScope).thenReturn(mock()) + whenever(scopes3.globalScope).thenReturn(mock()) + whenever(scopes4.globalScope).thenReturn(mock()) + whenever(scopes5.globalScope).thenReturn(mock()) + + val integrations = scopesList.map { scope -> + CompletableFuture.supplyAsync( + { + UncaughtExceptionHandlerIntegration(handler).apply { + register(scope, fixture.options) + } + }, + executor + ) + } + + CompletableFuture.allOf(*integrations.toTypedArray()).get() + + val futures = integrations.minus(integrations[2]).reversed().map { integration -> + CompletableFuture.supplyAsync( + { + integration.get().close() + println(Thread.currentThread().name) + }, + executor + ) + } + + CompletableFuture.allOf(*futures.toTypedArray()).get() + + assertEquals(integrations[2].get(), currentDefaultHandler) + } }