From 960fd20709168a1bcae5fba17612fcfcdd66038e Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 27 Apr 2023 17:10:14 +0200 Subject: [PATCH 1/4] all calls to ISentryExecutorService are now wrapped in a try catch block closing the hub will close the profiler and the performance collector added ui test for double init --- .../core/ActivityLifecycleIntegration.java | 19 ++++-- .../core/AndroidTransactionProfiler.java | 59 ++++++++++++++++--- .../core/AndroidTransactionProfilerTest.kt | 22 +++++++ .../android/core/SentryAndroidOptionsTest.kt | 2 + .../io/sentry/uitest/android/SdkInitTests.kt | 31 ++++++++++ sentry/api/sentry.api | 5 ++ ...efaultTransactionPerformanceCollector.java | 31 +++++++++- sentry/src/main/java/io/sentry/Hub.java | 15 +++++ .../io/sentry/ISentryExecutorService.java | 8 ++- .../java/io/sentry/ITransactionProfiler.java | 3 + .../NoOpTransactionPerformanceCollector.java | 3 + .../io/sentry/NoOpTransactionProfiler.java | 3 + sentry/src/main/java/io/sentry/Sentry.java | 31 ++++++---- .../TransactionPerformanceCollector.java | 5 ++ ...aultTransactionPerformanceCollectorTest.kt | 14 +++++ sentry/src/test/java/io/sentry/HubTest.kt | 28 ++++++++- .../io/sentry/NoOpTransactionProfilerTest.kt | 4 ++ .../test/java/io/sentry/SentryTracerTest.kt | 1 + 18 files changed, 254 insertions(+), 30 deletions(-) create mode 100644 sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index 8c20dce0eda..69ea79aaef6 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -249,11 +249,20 @@ private void startTracing(final @NotNull Activity activity) { ttfdSpan = transaction.startChild( TTFD_OP, getTtfdDesc(activityName), ttidStartTime, Instrumenter.SENTRY); - ttfdAutoCloseFuture = - options - .getExecutorService() - .schedule( - () -> finishExceededTtfdSpan(ttidSpanMap.get(activity)), TTFD_TIMEOUT_MILLIS); + try { + ttfdAutoCloseFuture = + options + .getExecutorService() + .schedule( + () -> finishExceededTtfdSpan(ttidSpanMap.get(activity)), TTFD_TIMEOUT_MILLIS); + } catch (Throwable e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + "Failed to call the executor. Time to full display span will not be finished automatically.", + e); + } } // lets bind to the scope so other integrations can pick it up diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index 912ea9829c8..b1ec04406db 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -37,6 +37,7 @@ import java.util.concurrent.TimeUnit; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; final class AndroidTransactionProfiler implements ITransactionProfiler { @@ -77,6 +78,8 @@ final class AndroidTransactionProfiler implements ITransactionProfiler { new ArrayDeque<>(); private final @NotNull Map measurementsMap = new HashMap<>(); + private @Nullable ITransaction currentTransaction = null; + public AndroidTransactionProfiler( final @NotNull Context context, final @NotNull SentryAndroidOptions sentryAndroidOptions, @@ -140,7 +143,13 @@ private void init() { @Override public synchronized void onTransactionStart(final @NotNull ITransaction transaction) { - options.getExecutorService().submit(() -> onTransactionStartSafe(transaction)); + try { + options.getExecutorService().submit(() -> onTransactionStartSafe(transaction)); + } catch (Throwable e) { + options + .getLogger() + .log(SentryLevel.ERROR, "Failed to call the executor. Profiling will not be started", e); + } } @SuppressLint("NewApi") @@ -235,13 +244,24 @@ public void onFrameMetricCollected( } }); + currentTransaction = transaction; + // We stop profiling after a timeout to avoid huge profiles to be sent - scheduledFinish = - options - .getExecutorService() - .schedule( - () -> timedOutProfilingData = onTransactionFinish(transaction, true, null), - PROFILING_TIMEOUT_MILLIS); + try { + scheduledFinish = + options + .getExecutorService() + .schedule( + () -> timedOutProfilingData = onTransactionFinish(transaction, true, null), + PROFILING_TIMEOUT_MILLIS); + } catch (Throwable e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + "Failed to call the executor. Profiling will not be automatically finished", + e); + } transactionStartNanos = SystemClock.elapsedRealtimeNanos(); profileStartCpuMillis = Process.getElapsedCpuTime(); @@ -265,6 +285,11 @@ public void onFrameMetricCollected( options.getLogger().log(SentryLevel.ERROR, "Error finishing profiling: ", e); } catch (InterruptedException e) { options.getLogger().log(SentryLevel.ERROR, "Error finishing profiling: ", e); + } catch (Throwable e) { + options + .getLogger() + .log( + SentryLevel.ERROR, "Failed to call the executor. Profiling could not be finished", e); } return null; } @@ -349,6 +374,7 @@ public void onFrameMetricCollected( currentProfilingTransactionData = null; // We clear the counter in case of a timeout transactionsCounter = 0; + currentTransaction = null; if (scheduledFinish != null) { scheduledFinish.cancel(true); @@ -483,6 +509,19 @@ private void putPerformanceCollectionDataInMeasurements( } } + @Override + public void close() { + // we cancel any scheduled work + if (scheduledFinish != null) { + scheduledFinish.cancel(true); + scheduledFinish = null; + } + // we stop profiling + if (currentTransaction != null) { + onTransactionFinish(currentTransaction, true, null); + } + } + /** * Get MemoryInfo object representing the memory state of the application. * @@ -503,4 +542,10 @@ private void putPerformanceCollectionDataInMeasurements( return null; } } + + @TestOnly + @Nullable + ITransaction getCurrentTransaction() { + return currentTransaction; + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index ab5bc2fb296..304f723d08f 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -17,6 +17,7 @@ import io.sentry.TransactionContext import io.sentry.android.core.internal.util.SentryFrameMetricsCollector import io.sentry.profilemeasurements.ProfileMeasurement import io.sentry.test.getCtor +import io.sentry.test.getProperty import org.junit.runner.RunWith import org.mockito.kotlin.any import org.mockito.kotlin.mock @@ -150,8 +151,11 @@ class AndroidTransactionProfilerTest { @Test fun `profiler profiles current transaction`() { val profiler = fixture.getSut(context) + assertNull(profiler.currentTransaction) profiler.onTransactionStart(fixture.transaction1) + assertNotNull(profiler.currentTransaction) val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1, null) + assertNull(profiler.currentTransaction) assertNotNull(profilingTraceData) assertEquals(profilingTraceData.transactionId, fixture.transaction1.eventId.toString()) @@ -401,4 +405,22 @@ class AndroidTransactionProfilerTest { data.measurementsMap[ProfileMeasurement.ID_MEMORY_NATIVE_FOOTPRINT]!!.values.map { it.value } ) } + + @Test + fun `profiler stops profiling, clear current transaction and scheduled job on close`() { + val profiler = fixture.getSut(context) + profiler.onTransactionStart(fixture.transaction1) + assertNotNull(profiler.currentTransaction) + + profiler.close() + assertNull(profiler.currentTransaction) + + // The timeout scheduled job should be cleared + val scheduledJob = profiler.getProperty?>("scheduledFinish") + assertNull(scheduledJob) + + // Calling transaction finish returns null, as the profiler was stopped + val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1, null) + assertNull(profilingTraceData) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt index 5cee3e792de..f6a4a995060 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt @@ -115,5 +115,7 @@ class SentryAndroidOptionsTest { transaction: ITransaction, performanceCollectionData: List? ): ProfilingTraceData? = null + + override fun close() {} } } diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt new file mode 100644 index 00000000000..81455e4d0b3 --- /dev/null +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt @@ -0,0 +1,31 @@ +package io.sentry.uitest.android + +import androidx.lifecycle.Lifecycle +import androidx.test.core.app.launchActivity +import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.Sentry +import io.sentry.android.core.SentryAndroidOptions +import org.junit.runner.RunWith +import kotlin.test.Test + +@RunWith(AndroidJUnit4::class) +class SdkInitTests : BaseUiTest() { + + @Test + fun doubleInitDoesNotThrow() { + initSentry(false) { options: SentryAndroidOptions -> + options.tracesSampleRate = 1.0 + options.profilesSampleRate = 1.0 + } + val transaction = Sentry.startTransaction("e2etests", "testInit") + val sampleScenario = launchActivity() + initSentry(false) { options: SentryAndroidOptions -> + options.tracesSampleRate = 1.0 + options.profilesSampleRate = 1.0 + } + transaction.finish() + sampleScenario.moveToState(Lifecycle.State.DESTROYED) + val transaction2 = Sentry.startTransaction("e2etests", "testInit") + transaction2.finish() + } +} diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index b44704316b2..96e72f63d37 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -188,6 +188,7 @@ public final class io/sentry/DateUtils { public final class io/sentry/DefaultTransactionPerformanceCollector : io/sentry/TransactionPerformanceCollector { public fun (Lio/sentry/SentryOptions;)V + public fun close ()V public fun start (Lio/sentry/ITransaction;)V public fun stop (Lio/sentry/ITransaction;)Ljava/util/List; } @@ -597,6 +598,7 @@ public abstract interface class io/sentry/ITransaction : io/sentry/ISpan { } public abstract interface class io/sentry/ITransactionProfiler { + public abstract fun close ()V public abstract fun onTransactionFinish (Lio/sentry/ITransaction;Ljava/util/List;)Lio/sentry/ProfilingTraceData; public abstract fun onTransactionStart (Lio/sentry/ITransaction;)V } @@ -905,12 +907,14 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction { } public final class io/sentry/NoOpTransactionPerformanceCollector : io/sentry/TransactionPerformanceCollector { + public fun close ()V public static fun getInstance ()Lio/sentry/NoOpTransactionPerformanceCollector; public fun start (Lio/sentry/ITransaction;)V public fun stop (Lio/sentry/ITransaction;)Ljava/util/List; } public final class io/sentry/NoOpTransactionProfiler : io/sentry/ITransactionProfiler { + public fun close ()V public static fun getInstance ()Lio/sentry/NoOpTransactionProfiler; public fun onTransactionFinish (Lio/sentry/ITransaction;Ljava/util/List;)Lio/sentry/ProfilingTraceData; public fun onTransactionStart (Lio/sentry/ITransaction;)V @@ -2151,6 +2155,7 @@ public final class io/sentry/TransactionOptions : io/sentry/SpanOptions { } public abstract interface class io/sentry/TransactionPerformanceCollector { + public abstract fun close ()V public abstract fun start (Lio/sentry/ITransaction;)V public abstract fun stop (Lio/sentry/ITransaction;)Ljava/util/List; } diff --git a/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java b/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java index b4ffec08721..d4017df356b 100644 --- a/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java +++ b/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java @@ -45,9 +45,18 @@ public void start(final @NotNull ITransaction transaction) { if (!performanceDataMap.containsKey(transaction.getEventId().toString())) { performanceDataMap.put(transaction.getEventId().toString(), new ArrayList<>()); // We schedule deletion of collected performance data after a timeout - options - .getExecutorService() - .schedule(() -> stop(transaction), TRANSACTION_COLLECTION_TIMEOUT_MILLIS); + try { + options + .getExecutorService() + .schedule(() -> stop(transaction), TRANSACTION_COLLECTION_TIMEOUT_MILLIS); + } catch (Throwable e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + "Failed to call the executor. Performance collector will not be automatically finished", + e); + } } if (!isStarted.getAndSet(true)) { synchronized (timerLock) { @@ -113,4 +122,20 @@ public void run() { } return data; } + + @Override + public void close() { + performanceDataMap.clear(); + options + .getLogger() + .log(SentryLevel.DEBUG, "stop collecting all performance info for transactions"); + if (isStarted.getAndSet(false)) { + synchronized (timerLock) { + if (timer != null) { + timer.cancel(); + timer = null; + } + } + } + } } diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index e22259195da..151d2c11ed6 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -339,6 +339,21 @@ public void close() { ((Closeable) integration).close(); } } + + withScope( + scope -> { + ITransaction transaction = scope.getTransaction(); + if (transaction != null) { + for (Span span : transaction.getSpans()) { + span.setSpanFinishedCallback(null); + span.finish(SpanStatus.CANCELLED); + } + transaction.finish(SpanStatus.CANCELLED); + } + scope.clear(); + }); + options.getTransactionProfiler().close(); + options.getTransactionPerformanceCollector().close(); options.getExecutorService().close(options.getShutdownTimeoutMillis()); // Close the top-most client diff --git a/sentry/src/main/java/io/sentry/ISentryExecutorService.java b/sentry/src/main/java/io/sentry/ISentryExecutorService.java index 7f6c81cedce..74d294bffee 100644 --- a/sentry/src/main/java/io/sentry/ISentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/ISentryExecutorService.java @@ -2,6 +2,7 @@ import java.util.concurrent.Callable; import java.util.concurrent.Future; +import java.util.concurrent.RejectedExecutionException; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -16,7 +17,7 @@ public interface ISentryExecutorService { * @return a Future of the Runnable */ @NotNull - Future submit(final @NotNull Runnable runnable); + Future submit(final @NotNull Runnable runnable) throws RejectedExecutionException; /** * Submits a Callable to the ThreadExecutor @@ -25,10 +26,11 @@ public interface ISentryExecutorService { * @return a Future of the Callable */ @NotNull - Future submit(final @NotNull Callable callable); + Future submit(final @NotNull Callable callable) throws RejectedExecutionException; @NotNull - Future schedule(final @NotNull Runnable runnable, final long delayMillis); + Future schedule(final @NotNull Runnable runnable, final long delayMillis) + throws RejectedExecutionException; /** * Closes the ThreadExecutor and awaits for the timeout diff --git a/sentry/src/main/java/io/sentry/ITransactionProfiler.java b/sentry/src/main/java/io/sentry/ITransactionProfiler.java index 44676e6f727..a5eb1c18843 100644 --- a/sentry/src/main/java/io/sentry/ITransactionProfiler.java +++ b/sentry/src/main/java/io/sentry/ITransactionProfiler.java @@ -14,4 +14,7 @@ public interface ITransactionProfiler { ProfilingTraceData onTransactionFinish( @NotNull ITransaction transaction, @Nullable List performanceCollectionData); + + /** Cancel the profiler and stops it. Used on SDK close. */ + void close(); } diff --git a/sentry/src/main/java/io/sentry/NoOpTransactionPerformanceCollector.java b/sentry/src/main/java/io/sentry/NoOpTransactionPerformanceCollector.java index 02edeb10944..ea8ef769852 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransactionPerformanceCollector.java +++ b/sentry/src/main/java/io/sentry/NoOpTransactionPerformanceCollector.java @@ -22,4 +22,7 @@ public void start(@NotNull ITransaction transaction) {} public @Nullable List stop(@NotNull ITransaction transaction) { return null; } + + @Override + public void close() {} } diff --git a/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java b/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java index 9f51ea33a97..f4c5ac07d8a 100644 --- a/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java +++ b/sentry/src/main/java/io/sentry/NoOpTransactionProfiler.java @@ -23,4 +23,7 @@ public void onTransactionStart(@NotNull ITransaction transaction) {} @Nullable List performanceCollectionData) { return null; } + + @Override + public void close() {} } diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 4b420a19a70..f3c8e8c88e2 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -280,17 +280,26 @@ private static boolean initConfigurations(final @NotNull SentryOptions options) profilingTracesDir.mkdirs(); final File[] oldTracesDirContent = profilingTracesDir.listFiles(); - options - .getExecutorService() - .submit( - () -> { - if (oldTracesDirContent == null) return; - // Method trace files are normally deleted at the end of traces, but if that fails - // for some reason we try to clear any old files here. - for (File f : oldTracesDirContent) { - FileUtils.deleteRecursively(f); - } - }); + try { + options + .getExecutorService() + .submit( + () -> { + if (oldTracesDirContent == null) return; + // Method trace files are normally deleted at the end of traces, but if that fails + // for some reason we try to clear any old files here. + for (File f : oldTracesDirContent) { + FileUtils.deleteRecursively(f); + } + }); + } catch (Throwable e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + "Failed to call the executor. Old profiles will not be deleted", + e); + } } final @NotNull IModulesLoader modulesLoader = options.getModulesLoader(); diff --git a/sentry/src/main/java/io/sentry/TransactionPerformanceCollector.java b/sentry/src/main/java/io/sentry/TransactionPerformanceCollector.java index 013667d2cfd..eec2c35fee3 100644 --- a/sentry/src/main/java/io/sentry/TransactionPerformanceCollector.java +++ b/sentry/src/main/java/io/sentry/TransactionPerformanceCollector.java @@ -1,6 +1,7 @@ package io.sentry; import java.util.List; +import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -10,4 +11,8 @@ public interface TransactionPerformanceCollector { @Nullable List stop(@NotNull ITransaction transaction); + + /** Cancel the collector and stops it. Used on SDK close. */ + @ApiStatus.Internal + void close(); } diff --git a/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt b/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt index b87de67f68f..c1feee7d358 100644 --- a/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt +++ b/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt @@ -226,6 +226,20 @@ class DefaultTransactionPerformanceCollectorTest { verify(threadCheckerCollector, atLeast(1)).collect(any()) } + @Test + fun `when close, timer is stopped and data is cleared`() { + val collector = fixture.getSut() + collector.start(fixture.transaction1) + collector.close() + + // Timer was canceled + verify(fixture.mockTimer)!!.scheduleAtFixedRate(any(), any(), eq(100)) + verify(fixture.mockTimer)!!.cancel() + + // Data was cleared + assertNull(collector.stop(fixture.transaction1)) + } + inner class ThreadCheckerCollector : ICollector { override fun setup() { if (mainThreadChecker.isMainThread) { diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt index 9e6d564caeb..842e8f3bc87 100644 --- a/sentry/src/test/java/io/sentry/HubTest.kt +++ b/sentry/src/test/java/io/sentry/HubTest.kt @@ -1530,16 +1530,42 @@ class HubTest { } @Test - fun `Hub should close the sentry executor processor on close call`() { + fun `Hub should close the sentry executor processor, profiler and performance collector on close call`() { val executor = mock() + val profiler = mock() + val performanceCollector = mock() val options = SentryOptions().apply { dsn = "https://key@sentry.io/proj" cacheDirPath = file.absolutePath executorService = executor + setTransactionProfiler(profiler) + transactionPerformanceCollector = performanceCollector } val sut = Hub(options) sut.close() verify(executor).close(any()) + verify(profiler).close() + verify(performanceCollector).close() + } + + @Test + fun `Hub should cancel current transaction bound to the scope and its spans`() { + val hub = generateHub { + it.tracesSampleRate = 1.0 + } + val transaction = hub.startTransaction("test", "test", true) + val span = transaction.startChild("span1") + val span2 = transaction.startChild("span1") + assertFalse(transaction.isFinished) + assertFalse(span.isFinished) + assertFalse(span2.isFinished) + hub.close() + assertTrue(transaction.isFinished) + assertTrue(span.isFinished) + assertTrue(span2.isFinished) + assertEquals(SpanStatus.CANCELLED, transaction.status) + assertEquals(SpanStatus.CANCELLED, span.status) + assertEquals(SpanStatus.CANCELLED, span2.status) } @Test diff --git a/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt b/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt index 288380f05ed..e210226c341 100644 --- a/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt +++ b/sentry/src/test/java/io/sentry/NoOpTransactionProfilerTest.kt @@ -14,4 +14,8 @@ class NoOpTransactionProfilerTest { fun `onTransactionFinish does returns null`() { assertNull(profiler.onTransactionFinish(NoOpTransaction.getInstance(), null)) } + + @Test + fun `close does not throw`() = + profiler.close() } diff --git a/sentry/src/test/java/io/sentry/SentryTracerTest.kt b/sentry/src/test/java/io/sentry/SentryTracerTest.kt index 87ffb165d8f..fafe1f40090 100644 --- a/sentry/src/test/java/io/sentry/SentryTracerTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTracerTest.kt @@ -1069,6 +1069,7 @@ class SentryTracerTest { val mockPerformanceCollector = object : TransactionPerformanceCollector { override fun start(transaction: ITransaction) {} override fun stop(transaction: ITransaction): MutableList = data + override fun close() {} } val transaction = fixture.getSut(optionsConfiguration = { it.profilesSampleRate = 1.0 From 6af6003def3186e742cfd26f81ada20d408261fa Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Thu, 27 Apr 2023 17:14:59 +0200 Subject: [PATCH 2/4] updated changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6ea74a7a042..a169005898d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +### Unreleased + +## Fixes + +- Fix crash on double SDK init ([#2679](https://github.com/getsentry/sentry-java/pull/2679)) + ## 6.18.0 ### Features From 2914becee332e0ec0627376e0c3de7f42e67f15e Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 28 Apr 2023 10:07:11 +0200 Subject: [PATCH 3/4] updated exception description --- .../core/ActivityLifecycleIntegration.java | 5 +++-- .../core/AndroidTransactionProfiler.java | 18 ++++++++++++------ .../core/SendCachedEnvelopeIntegration.java | 8 ++++++++ ...DefaultTransactionPerformanceCollector.java | 5 +++-- ...CachedEnvelopeFireAndForgetIntegration.java | 8 ++++++++ sentry/src/main/java/io/sentry/Sentry.java | 5 +++-- 6 files changed, 37 insertions(+), 12 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java index 69ea79aaef6..b0bb8fa3b06 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/ActivityLifecycleIntegration.java @@ -37,6 +37,7 @@ import java.util.Map; import java.util.WeakHashMap; import java.util.concurrent.Future; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -255,12 +256,12 @@ private void startTracing(final @NotNull Activity activity) { .getExecutorService() .schedule( () -> finishExceededTtfdSpan(ttidSpanMap.get(activity)), TTFD_TIMEOUT_MILLIS); - } catch (Throwable e) { + } catch (RejectedExecutionException e) { options .getLogger() .log( SentryLevel.ERROR, - "Failed to call the executor. Time to full display span will not be finished automatically.", + "Failed to call the executor. Time to full display span will not be finished automatically. Did you call Sentry.close()?", e); } } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index b1ec04406db..b82edb93143 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -34,6 +34,7 @@ import java.util.UUID; import java.util.concurrent.ExecutionException; import java.util.concurrent.Future; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -145,10 +146,13 @@ private void init() { public synchronized void onTransactionStart(final @NotNull ITransaction transaction) { try { options.getExecutorService().submit(() -> onTransactionStartSafe(transaction)); - } catch (Throwable e) { + } catch (RejectedExecutionException e) { options .getLogger() - .log(SentryLevel.ERROR, "Failed to call the executor. Profiling will not be started", e); + .log( + SentryLevel.ERROR, + "Failed to call the executor. Profiling will not be started. Did you call Sentry.close()?", + e); } } @@ -254,12 +258,12 @@ public void onFrameMetricCollected( .schedule( () -> timedOutProfilingData = onTransactionFinish(transaction, true, null), PROFILING_TIMEOUT_MILLIS); - } catch (Throwable e) { + } catch (RejectedExecutionException e) { options .getLogger() .log( SentryLevel.ERROR, - "Failed to call the executor. Profiling will not be automatically finished", + "Failed to call the executor. Profiling will not be automatically finished. Did you call Sentry.close()?", e); } @@ -285,11 +289,13 @@ public void onFrameMetricCollected( options.getLogger().log(SentryLevel.ERROR, "Error finishing profiling: ", e); } catch (InterruptedException e) { options.getLogger().log(SentryLevel.ERROR, "Error finishing profiling: ", e); - } catch (Throwable e) { + } catch (RejectedExecutionException e) { options .getLogger() .log( - SentryLevel.ERROR, "Failed to call the executor. Profiling could not be finished", e); + SentryLevel.ERROR, + "Failed to call the executor. Profiling could not be finished. Did you call Sentry.close()?", + e); } return null; } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java index 4f150907967..bda83358223 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SendCachedEnvelopeIntegration.java @@ -7,6 +7,7 @@ import io.sentry.SentryOptions; import io.sentry.util.Objects; import java.util.concurrent.Future; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import org.jetbrains.annotations.NotNull; @@ -74,6 +75,13 @@ public void register(@NotNull IHub hub, @NotNull SentryOptions options) { } } androidOptions.getLogger().log(SentryLevel.DEBUG, "SendCachedEnvelopeIntegration installed."); + } catch (RejectedExecutionException e) { + androidOptions + .getLogger() + .log( + SentryLevel.ERROR, + "Failed to call the executor. Cached events will not be sent. Did you call Sentry.close()?", + e); } catch (Throwable e) { androidOptions .getLogger() diff --git a/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java b/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java index d4017df356b..eb11f2a779d 100644 --- a/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java +++ b/sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java @@ -7,6 +7,7 @@ import java.util.Timer; import java.util.TimerTask; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.RejectedExecutionException; import java.util.concurrent.atomic.AtomicBoolean; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; @@ -49,12 +50,12 @@ public void start(final @NotNull ITransaction transaction) { options .getExecutorService() .schedule(() -> stop(transaction), TRANSACTION_COLLECTION_TIMEOUT_MILLIS); - } catch (Throwable e) { + } catch (RejectedExecutionException e) { options .getLogger() .log( SentryLevel.ERROR, - "Failed to call the executor. Performance collector will not be automatically finished", + "Failed to call the executor. Performance collector will not be automatically finished. Did you call Sentry.close()?", e); } } diff --git a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java index 2646d9be398..01f9b960183 100644 --- a/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java +++ b/sentry/src/main/java/io/sentry/SendCachedEnvelopeFireAndForgetIntegration.java @@ -2,6 +2,7 @@ import io.sentry.util.Objects; import java.io.File; +import java.util.concurrent.RejectedExecutionException; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -88,6 +89,13 @@ public final void register(final @NotNull IHub hub, final @NotNull SentryOptions .getLogger() .log(SentryLevel.DEBUG, "SendCachedEventFireAndForgetIntegration installed."); addIntegrationToSdkVersion(); + } catch (RejectedExecutionException e) { + options + .getLogger() + .log( + SentryLevel.ERROR, + "Failed to call the executor. Cached events will not be sent. Did you call Sentry.close()?", + e); } catch (Throwable e) { options .getLogger() diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index f3c8e8c88e2..01fb3d67563 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -19,6 +19,7 @@ import java.lang.reflect.InvocationTargetException; import java.util.Arrays; import java.util.List; +import java.util.concurrent.RejectedExecutionException; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @@ -292,12 +293,12 @@ private static boolean initConfigurations(final @NotNull SentryOptions options) FileUtils.deleteRecursively(f); } }); - } catch (Throwable e) { + } catch (RejectedExecutionException e) { options .getLogger() .log( SentryLevel.ERROR, - "Failed to call the executor. Old profiles will not be deleted", + "Failed to call the executor. Old profiles will not be deleted. Did you call Sentry.close()?", e); } } From 7e1be4f3a6808c3bdaefe90e73aad0cf6410f728 Mon Sep 17 00:00:00 2001 From: stefanosiano Date: Fri, 28 Apr 2023 15:25:45 +0200 Subject: [PATCH 4/4] added ISentryExecutorService.close internal method profiling is now stopped on executorService exceptions transaction bound to the scope is not cancelled automatically anymore on Sentry.close() ISentryExecutorService is re-init in Sentry.init if it was shutdown --- .../core/AndroidTransactionProfiler.java | 8 ++-- .../core/ActivityLifecycleIntegrationTest.kt | 2 + .../core/AndroidTransactionProfilerTest.kt | 1 + .../io/sentry/uitest/android/SdkInitTests.kt | 46 +++++++++++++++++++ sentry/api/sentry.api | 1 + sentry/src/main/java/io/sentry/Hub.java | 13 +----- .../io/sentry/ISentryExecutorService.java | 7 +++ .../io/sentry/NoOpSentryExecutorService.java | 5 ++ sentry/src/main/java/io/sentry/Sentry.java | 8 ++++ .../java/io/sentry/SentryExecutorService.java | 7 +++ ...aultTransactionPerformanceCollectorTest.kt | 1 + sentry/src/test/java/io/sentry/HubTest.kt | 20 -------- .../io/sentry/SentryExecutorServiceTest.kt | 17 +++++++ sentry/src/test/java/io/sentry/SentryTest.kt | 32 +++++++++++++ 14 files changed, 133 insertions(+), 35 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java index b82edb93143..9322bf50e11 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java @@ -285,10 +285,10 @@ public void onFrameMetricCollected( .getExecutorService() .submit(() -> onTransactionFinish(transaction, false, performanceCollectionData)) .get(); - } catch (ExecutionException e) { - options.getLogger().log(SentryLevel.ERROR, "Error finishing profiling: ", e); - } catch (InterruptedException e) { + } catch (ExecutionException | InterruptedException e) { options.getLogger().log(SentryLevel.ERROR, "Error finishing profiling: ", e); + // We stop profiling even on exceptions, so profiles don't run indefinitely + onTransactionFinish(transaction, false, null); } catch (RejectedExecutionException e) { options .getLogger() @@ -296,6 +296,8 @@ public void onFrameMetricCollected( SentryLevel.ERROR, "Failed to call the executor. Profiling could not be finished. Did you call Sentry.close()?", e); + // We stop profiling even on exceptions, so profiles don't run indefinitely + onTransactionFinish(transaction, false, null); } return null; } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt index 5ed4bd2bd7c..1cdfbc3fc3c 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/ActivityLifecycleIntegrationTest.kt @@ -1097,6 +1097,7 @@ class ActivityLifecycleIntegrationTest { return FutureTask {} } override fun close(timeoutMillis: Long) {} + override fun isClosed() = false } fixture.options.tracesSampleRate = 1.0 fixture.options.isEnableTimeToFullDisplayTracing = true @@ -1321,6 +1322,7 @@ class ActivityLifecycleIntegrationTest { return FutureTask {} } override fun close(timeoutMillis: Long) {} + override fun isClosed() = false } fixture.options.tracesSampleRate = 1.0 fixture.options.isEnableTimeToFullDisplayTracing = true diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index 304f723d08f..b72854e7cd7 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -76,6 +76,7 @@ class AndroidTransactionProfilerTest { return FutureTask {} } override fun close(timeoutMillis: Long) {} + override fun isClosed() = false } val options = spy(SentryAndroidOptions()).apply { diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt index 81455e4d0b3..39bc21c4e9d 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/SdkInitTests.kt @@ -3,10 +3,14 @@ package io.sentry.uitest.android import androidx.lifecycle.Lifecycle import androidx.test.core.app.launchActivity import androidx.test.ext.junit.runners.AndroidJUnit4 +import io.sentry.ProfilingTraceData import io.sentry.Sentry import io.sentry.android.core.SentryAndroidOptions +import io.sentry.assertEnvelopeItem +import io.sentry.protocol.SentryTransaction import org.junit.runner.RunWith import kotlin.test.Test +import kotlin.test.assertEquals @RunWith(AndroidJUnit4::class) class SdkInitTests : BaseUiTest() { @@ -28,4 +32,46 @@ class SdkInitTests : BaseUiTest() { val transaction2 = Sentry.startTransaction("e2etests", "testInit") transaction2.finish() } + + @Test + fun doubleInitWithSameOptionsDoesNotThrow() { + val options = SentryAndroidOptions() + + initSentry(true) { + it.tracesSampleRate = 1.0 + it.profilesSampleRate = 1.0 + // We use the same executorService before and after closing the SDK + it.executorService = options.executorService + it.isDebug = true + } + val transaction = Sentry.startTransaction("e2etests", "testInit") + val sampleScenario = launchActivity() + initSentry(true) { + it.tracesSampleRate = 1.0 + it.profilesSampleRate = 1.0 + // We use the same executorService before and after closing the SDK + it.executorService = options.executorService + it.isDebug = true + } + relayIdlingResource.increment() + transaction.finish() + sampleScenario.moveToState(Lifecycle.State.DESTROYED) + val transaction2 = Sentry.startTransaction("e2etests2", "testInit") + transaction2.finish() + + relay.assert { + findEnvelope { + assertEnvelopeItem(it.items.toList()).transaction == "e2etests2" + }.assert { + val transactionItem: SentryTransaction = it.assertItem() + // Profiling uses executorService, so if the executorService is shutdown it would fail + val profilingTraceData: ProfilingTraceData = it.assertItem() + it.assertNoOtherItems() + assertEquals("e2etests2", transactionItem.transaction) + assertEquals("e2etests2", profilingTraceData.transactionName) + } + assertNoOtherEnvelopes() + assertNoOtherRequests() + } + } } diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index 96e72f63d37..666fc6080b6 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -532,6 +532,7 @@ public abstract interface class io/sentry/ISentryClient { public abstract interface class io/sentry/ISentryExecutorService { public abstract fun close (J)V + public abstract fun isClosed ()Z public abstract fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future; public abstract fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future; public abstract fun submit (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future; diff --git a/sentry/src/main/java/io/sentry/Hub.java b/sentry/src/main/java/io/sentry/Hub.java index 151d2c11ed6..dcc80fa2877 100644 --- a/sentry/src/main/java/io/sentry/Hub.java +++ b/sentry/src/main/java/io/sentry/Hub.java @@ -340,18 +340,7 @@ public void close() { } } - withScope( - scope -> { - ITransaction transaction = scope.getTransaction(); - if (transaction != null) { - for (Span span : transaction.getSpans()) { - span.setSpanFinishedCallback(null); - span.finish(SpanStatus.CANCELLED); - } - transaction.finish(SpanStatus.CANCELLED); - } - scope.clear(); - }); + withScope(scope -> scope.clear()); options.getTransactionProfiler().close(); options.getTransactionPerformanceCollector().close(); options.getExecutorService().close(options.getShutdownTimeoutMillis()); diff --git a/sentry/src/main/java/io/sentry/ISentryExecutorService.java b/sentry/src/main/java/io/sentry/ISentryExecutorService.java index 74d294bffee..9bdef8db2b7 100644 --- a/sentry/src/main/java/io/sentry/ISentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/ISentryExecutorService.java @@ -38,4 +38,11 @@ Future schedule(final @NotNull Runnable runnable, final long delayMillis) * @param timeoutMillis the timeout in millis */ void close(long timeoutMillis); + + /** + * Check if there was a previous call to the close() method. + * + * @return If the executorService was previously closed + */ + boolean isClosed(); } diff --git a/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java b/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java index 232dfaf112e..735c0dc29a3 100644 --- a/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/NoOpSentryExecutorService.java @@ -31,4 +31,9 @@ private NoOpSentryExecutorService() {} @Override public void close(long timeoutMillis) {} + + @Override + public boolean isClosed() { + return false; + } } diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 01fb3d67563..c3eea1d9b1c 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -217,6 +217,14 @@ private static synchronized void init( hub.close(); + // If the executorService passed in the init is the same that was previously closed, we have to + // set a new one + final ISentryExecutorService sentryExecutorService = options.getExecutorService(); + // If the passed executor service was previously called we set a new one + if (sentryExecutorService.isClosed()) { + options.setExecutorService(new SentryExecutorService()); + } + // when integrations are registered on Hub ctor and async integrations are fired, // it might and actually happened that integrations called captureSomething // and hub was still NoOp. diff --git a/sentry/src/main/java/io/sentry/SentryExecutorService.java b/sentry/src/main/java/io/sentry/SentryExecutorService.java index 89e343d4758..ba0ae4fac56 100644 --- a/sentry/src/main/java/io/sentry/SentryExecutorService.java +++ b/sentry/src/main/java/io/sentry/SentryExecutorService.java @@ -52,4 +52,11 @@ public void close(final long timeoutMillis) { } } } + + @Override + public boolean isClosed() { + synchronized (executorService) { + return executorService.isShutdown(); + } + } } diff --git a/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt b/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt index c1feee7d358..edcbf32811f 100644 --- a/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt +++ b/sentry/src/test/java/io/sentry/DefaultTransactionPerformanceCollectorTest.kt @@ -47,6 +47,7 @@ class DefaultTransactionPerformanceCollectorTest { return FutureTask {} } override fun close(timeoutMillis: Long) {} + override fun isClosed() = false } val mockCpuCollector: ICollector = object : ICollector { diff --git a/sentry/src/test/java/io/sentry/HubTest.kt b/sentry/src/test/java/io/sentry/HubTest.kt index 842e8f3bc87..62ad641372f 100644 --- a/sentry/src/test/java/io/sentry/HubTest.kt +++ b/sentry/src/test/java/io/sentry/HubTest.kt @@ -1548,26 +1548,6 @@ class HubTest { verify(performanceCollector).close() } - @Test - fun `Hub should cancel current transaction bound to the scope and its spans`() { - val hub = generateHub { - it.tracesSampleRate = 1.0 - } - val transaction = hub.startTransaction("test", "test", true) - val span = transaction.startChild("span1") - val span2 = transaction.startChild("span1") - assertFalse(transaction.isFinished) - assertFalse(span.isFinished) - assertFalse(span2.isFinished) - hub.close() - assertTrue(transaction.isFinished) - assertTrue(span.isFinished) - assertTrue(span2.isFinished) - assertEquals(SpanStatus.CANCELLED, transaction.status) - assertEquals(SpanStatus.CANCELLED, span.status) - assertEquals(SpanStatus.CANCELLED, span2.status) - } - @Test fun `when tracesSampleRate and tracesSampler are not set on SentryOptions, startTransaction returns NoOp`() { val hub = generateHub { diff --git a/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt b/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt index f06379008bf..1cd00109004 100644 --- a/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt +++ b/sentry/src/test/java/io/sentry/SentryExecutorServiceTest.kt @@ -10,6 +10,7 @@ import java.util.concurrent.Executors import java.util.concurrent.ScheduledExecutorService import java.util.concurrent.atomic.AtomicBoolean import kotlin.test.Test +import kotlin.test.assertFalse import kotlin.test.assertTrue class SentryExecutorServiceTest { @@ -87,4 +88,20 @@ class SentryExecutorServiceTest { await.untilFalse(atomicBoolean) sentryExecutor.close(15000) } + + @Test + fun `SentryExecutorService isClosed returns true if executor is shutdown`() { + val executor = mock() + val sentryExecutor = SentryExecutorService(executor) + whenever(executor.isShutdown).thenReturn(true) + assertTrue(sentryExecutor.isClosed) + } + + @Test + fun `SentryExecutorService isClosed returns false if executor is not shutdown`() { + val executor = mock() + val sentryExecutor = SentryExecutorService(executor) + whenever(executor.isShutdown).thenReturn(false) + assertFalse(sentryExecutor.isClosed) + } } diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index a84cf32dcc0..5c95eb46eda 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -13,6 +13,7 @@ import org.mockito.kotlin.argThat import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.verify +import org.mockito.kotlin.whenever import java.io.File import java.nio.file.Files import java.util.concurrent.CompletableFuture @@ -21,6 +22,7 @@ import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse +import kotlin.test.assertNotEquals import kotlin.test.assertNotNull import kotlin.test.assertNull import kotlin.test.assertTrue @@ -500,6 +502,36 @@ class SentryTest { verify(hub).reportFullyDisplayed() } + @Test + fun `ignores executorService if it is closed`() { + var sentryOptions: SentryOptions? = null + val executorService = mock() + whenever(executorService.isClosed).thenReturn(true) + + Sentry.init { + it.dsn = dsn + it.executorService = executorService + sentryOptions = it + } + + assertNotEquals(executorService, sentryOptions!!.executorService) + } + + @Test + fun `accept executorService if it is not closed`() { + var sentryOptions: SentryOptions? = null + val executorService = mock() + whenever(executorService.isClosed).thenReturn(false) + + Sentry.init { + it.dsn = dsn + it.executorService = executorService + sentryOptions = it + } + + assertEquals(executorService, sentryOptions!!.executorService) + } + private class CustomMainThreadChecker : IMainThreadChecker { override fun isMainThread(threadId: Long): Boolean = false }