Merge 2914bec into 308cdb2

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

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;
@@ -249,11 +250,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 (RejectedExecutionException e) {
+          options
+              .getLogger()
+              .log(
+                  SentryLevel.ERROR,
+                  "Failed to call the executor. Time to full display span will not be finished automatically. Did you call Sentry.close()?",
+                  e);
+        }
       }
 
       // lets bind to the scope so other integrations can pick it up

sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java

@@ -34,9 +34,11 @@
 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;
+import org.jetbrains.annotations.TestOnly;
 
 final class AndroidTransactionProfiler implements ITransactionProfiler {
 
@@ -77,6 +79,8 @@ final class AndroidTransactionProfiler implements ITransactionProfiler {
       new ArrayDeque<>();
   private final @NotNull Map<String, ProfileMeasurement> measurementsMap = new HashMap<>();
 
+  private @Nullable ITransaction currentTransaction = null;
+
   public AndroidTransactionProfiler(
       final @NotNull Context context,
       final @NotNull SentryAndroidOptions sentryAndroidOptions,
@@ -140,7 +144,16 @@ private void init() {
 
   @Override
   public synchronized void onTransactionStart(final @NotNull ITransaction transaction) {
-    options.getExecutorService().submit(() -> onTransactionStartSafe(transaction));
+    try {
+      options.getExecutorService().submit(() -> onTransactionStartSafe(transaction));
+    } catch (RejectedExecutionException e) {
+      options
+          .getLogger()
+          .log(
+              SentryLevel.ERROR,
+              "Failed to call the executor. Profiling will not be started. Did you call Sentry.close()?",
+              e);
+    }
   }
 
   @SuppressLint("NewApi")
@@ -235,13 +248,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 (RejectedExecutionException e) {
+      options
+          .getLogger()
+          .log(
+              SentryLevel.ERROR,
+              "Failed to call the executor. Profiling will not be automatically finished. Did you call Sentry.close()?",
+              e);
+    }
 
     transactionStartNanos = SystemClock.elapsedRealtimeNanos();
     profileStartCpuMillis = Process.getElapsedCpuTime();
@@ -265,6 +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 (RejectedExecutionException e) {
+      options
+          .getLogger()
+          .log(
+              SentryLevel.ERROR,
+              "Failed to call the executor. Profiling could not be finished. Did you call Sentry.close()?",
+              e);
     }
     return null;
   }
@@ -349,6 +380,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 +515,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 +548,10 @@ private void putPerformanceCollectionDataInMeasurements(
       return null;
     }
   }
+
+  @TestOnly
+  @Nullable
+  ITransaction getCurrentTransaction() {
+    return currentTransaction;
+  }
 }

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()

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<Future<*>?>("scheduledFinish")
+        assertNull(scheduledJob)
+
+        // Calling transaction finish returns null, as the profiler was stopped
+        val profilingTraceData = profiler.onTransactionFinish(fixture.transaction1, null)
+        assertNull(profilingTraceData)
+    }
 }

sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidOptionsTest.kt

@@ -115,5 +115,7 @@ class SentryAndroidOptionsTest {
             transaction: ITransaction,
             performanceCollectionData: List<PerformanceCollectionData>?
         ): ProfilingTraceData? = null
+
+        override fun close() {}
     }
 }

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<EmptyActivity>()
+        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()
+    }
+}

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 <init> (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;
 }

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;
@@ -45,9 +46,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 (RejectedExecutionException e) {
+        options
+            .getLogger()
+            .log(
+                SentryLevel.ERROR,
+                "Failed to call the executor. Performance collector will not be automatically finished. Did you call Sentry.close()?",
+                e);
+      }
     }
     if (!isStarted.getAndSet(true)) {
       synchronized (timerLock) {
@@ -113,4 +123,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;
+        }
+      }
+    }
+  }
 }

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