Prevent OOM by disabling TransactionPerformanceCollector for now

CHANGELOG.md

@@ -16,6 +16,7 @@
 ### Fixes
 
 - Expand guard against CVE-2018-9492 "Privilege Escalation via Content Provider" ([#2482](https://github.com/getsentry/sentry-java/pull/2482))
+- Prevent OOM by disabling TransactionPerformanceCollector for now ([#2498](https://github.com/getsentry/sentry-java/pull/2498))
 
 ## 6.12.1
 

sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt

@@ -88,9 +88,9 @@ class EnvelopeTests : BaseUiTest() {
                 val slowFrames = profilingTraceData.measurementsMap[ProfileMeasurement.ID_SLOW_FRAME_RENDERS]
                 val frozenFrames = profilingTraceData.measurementsMap[ProfileMeasurement.ID_FROZEN_FRAME_RENDERS]
                 val frameRates = profilingTraceData.measurementsMap[ProfileMeasurement.ID_SCREEN_FRAME_RATES]!!
-                val memoryStats = profilingTraceData.measurementsMap[ProfileMeasurement.ID_MEMORY_FOOTPRINT]!!
-                val memoryNativeStats = profilingTraceData.measurementsMap[ProfileMeasurement.ID_MEMORY_NATIVE_FOOTPRINT]!!
-                val cpuStats = profilingTraceData.measurementsMap[ProfileMeasurement.ID_CPU_USAGE]!!
+//                val memoryStats = profilingTraceData.measurementsMap[ProfileMeasurement.ID_MEMORY_FOOTPRINT]!!
+//                val memoryNativeStats = profilingTraceData.measurementsMap[ProfileMeasurement.ID_MEMORY_NATIVE_FOOTPRINT]!!
+//                val cpuStats = profilingTraceData.measurementsMap[ProfileMeasurement.ID_CPU_USAGE]!!
                 // Slow and frozen frames can be null (in case there were none)
                 if (slowFrames != null) {
                     assertEquals(ProfileMeasurement.UNIT_NANOSECONDS, slowFrames.unit)
@@ -101,12 +101,12 @@ class EnvelopeTests : BaseUiTest() {
                 // There could be no slow/frozen frames, but we expect at least one frame rate
                 assertEquals(ProfileMeasurement.UNIT_HZ, frameRates.unit)
                 assertTrue(frameRates.values.isNotEmpty())
-                assertEquals(ProfileMeasurement.UNIT_BYTES, memoryStats.unit)
-                assertTrue(memoryStats.values.isNotEmpty())
-                assertEquals(ProfileMeasurement.UNIT_BYTES, memoryNativeStats.unit)
-                assertTrue(memoryNativeStats.values.isNotEmpty())
-                assertEquals(ProfileMeasurement.UNIT_PERCENT, cpuStats.unit)
-                assertTrue(cpuStats.values.isNotEmpty())
+//                assertEquals(ProfileMeasurement.UNIT_BYTES, memoryStats.unit)
+//                assertTrue(memoryStats.values.isNotEmpty())
+//                assertEquals(ProfileMeasurement.UNIT_BYTES, memoryNativeStats.unit)
+//                assertTrue(memoryNativeStats.values.isNotEmpty())
+//                assertEquals(ProfileMeasurement.UNIT_PERCENT, cpuStats.unit)
+//                assertTrue(cpuStats.values.isNotEmpty())
 
                 // We should find the transaction id that started the profiling in the list of transactions
                 val transactionData = profilingTraceData.transactions

sentry/api/sentry.api

@@ -183,6 +183,12 @@ public final class io/sentry/DateUtils {
 	public static fun toUtilDateNotNull (Lio/sentry/SentryDate;)Ljava/util/Date;
 }
 
+public final class io/sentry/DefaultTransactionPerformanceCollector : io/sentry/TransactionPerformanceCollector {
+	public fun <init> (Lio/sentry/SentryOptions;)V
+	public fun start (Lio/sentry/ITransaction;)V
+	public fun stop (Lio/sentry/ITransaction;)Lio/sentry/PerformanceCollectionData;
+}
+
 public final class io/sentry/DiagnosticLogger : io/sentry/ILogger {
 	public fun <init> (Lio/sentry/SentryOptions;Lio/sentry/ILogger;)V
 	public fun getLogger ()Lio/sentry/ILogger;
@@ -853,6 +859,12 @@ public final class io/sentry/NoOpTransaction : io/sentry/ITransaction {
 	public fun traceContext ()Lio/sentry/TraceContext;
 }
 
+public final class io/sentry/NoOpTransactionPerformanceCollector : io/sentry/TransactionPerformanceCollector {
+	public static fun getInstance ()Lio/sentry/NoOpTransactionPerformanceCollector;
+	public fun start (Lio/sentry/ITransaction;)V
+	public fun stop (Lio/sentry/ITransaction;)Lio/sentry/PerformanceCollectionData;
+}
+
 public final class io/sentry/NoOpTransactionProfiler : io/sentry/ITransactionProfiler {
 	public static fun getInstance ()Lio/sentry/NoOpTransactionProfiler;
 	public fun onTransactionFinish (Lio/sentry/ITransaction;Lio/sentry/PerformanceCollectionData;)Lio/sentry/ProfilingTraceData;
@@ -2055,10 +2067,9 @@ public final class io/sentry/TransactionOptions {
 	public fun setWaitForChildren (Z)V
 }
 
-public final class io/sentry/TransactionPerformanceCollector {
-	public fun <init> (Lio/sentry/SentryOptions;)V
-	public fun start (Lio/sentry/ITransaction;)V
-	public fun stop (Lio/sentry/ITransaction;)Lio/sentry/PerformanceCollectionData;
+public abstract interface class io/sentry/TransactionPerformanceCollector {
+	public abstract fun start (Lio/sentry/ITransaction;)V
+	public abstract fun stop (Lio/sentry/ITransaction;)Lio/sentry/PerformanceCollectionData;
 }
 
 public final class io/sentry/TypeCheckHint {

sentry/src/main/java/io/sentry/DefaultTransactionPerformanceCollector.java

@@ -0,0 +1,105 @@
+package io.sentry;
+
+import io.sentry.util.Objects;
+import java.util.List;
+import java.util.Map;
+import java.util.Timer;
+import java.util.TimerTask;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicBoolean;
+import org.jetbrains.annotations.ApiStatus;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+@ApiStatus.Internal
+public final class DefaultTransactionPerformanceCollector
+    implements TransactionPerformanceCollector {
+  private static final long TRANSACTION_COLLECTION_INTERVAL_MILLIS = 100;
+  private static final long TRANSACTION_COLLECTION_TIMEOUT_MILLIS = 30000;
+  private final @NotNull Object timerLock = new Object();
+  private volatile @Nullable Timer timer = null;
+  private final @NotNull Map<String, PerformanceCollectionData> performanceDataMap =
+      new ConcurrentHashMap<>();
+  private final @NotNull List<ICollector> collectors;
+  private final @NotNull SentryOptions options;
+  private final @NotNull AtomicBoolean isStarted = new AtomicBoolean(false);
+
+  public DefaultTransactionPerformanceCollector(final @NotNull SentryOptions options) {
+    this.options = Objects.requireNonNull(options, "The options object is required.");
+    this.collectors = options.getCollectors();
+  }
+
+  @Override
+  @SuppressWarnings("FutureReturnValueIgnored")
+  public void start(final @NotNull ITransaction transaction) {
+    if (collectors.isEmpty()) {
+      options
+          .getLogger()
+          .log(
+              SentryLevel.INFO,
+              "No collector found. Performance stats will not be captured during transactions.");
+      return;
+    }
+
+    if (!performanceDataMap.containsKey(transaction.getEventId().toString())) {
+      performanceDataMap.put(transaction.getEventId().toString(), new PerformanceCollectionData());
+      options
+          .getExecutorService()
+          .schedule(
+              () -> {
+                PerformanceCollectionData data = stop(transaction);
+                if (data != null) {
+                  performanceDataMap.put(transaction.getEventId().toString(), data);
+                }
+              },
+              TRANSACTION_COLLECTION_TIMEOUT_MILLIS);
+    }
+    if (!isStarted.getAndSet(true)) {
+      synchronized (timerLock) {
+        for (ICollector collector : collectors) {
+          collector.setup();
+        }
+        if (timer == null) {
+          timer = new Timer(true);
+        }
+        // We schedule the timer to start after a delay, so we let some time pass between setup()
+        // and collect() calls.
+        // This way ICollectors that collect average stats based on time intervals, like
+        // AndroidCpuCollector, can have an actual time interval to evaluate.
+        timer.scheduleAtFixedRate(
+            new TimerTask() {
+              @Override
+              public void run() {
+                synchronized (timerLock) {
+                  for (ICollector collector : collectors) {
+                    collector.collect(performanceDataMap.values());
+                  }
+                  // We commit data after calling all collectors.
+                  // This way we avoid issues caused by having multiple cpu or memory collectors.
+                  for (PerformanceCollectionData data : performanceDataMap.values()) {
+                    data.commitData();
+                  }
+                }
+              }
+            },
+            TRANSACTION_COLLECTION_INTERVAL_MILLIS,
+            TRANSACTION_COLLECTION_INTERVAL_MILLIS);
+      }
+    }
+  }
+
+  @Override
+  public @Nullable PerformanceCollectionData stop(final @NotNull ITransaction transaction) {
+    synchronized (timerLock) {
+      PerformanceCollectionData data =
+          performanceDataMap.remove(transaction.getEventId().toString());
+      if (performanceDataMap.isEmpty() && isStarted.getAndSet(false)) {
+        if (timer != null) {
+          timer.cancel();
+          timer = null;
+        }
+      }
+      return data;
+    }
+  }
+}

sentry/src/main/java/io/sentry/NoOpTransactionPerformanceCollector.java

@@ -0,0 +1,24 @@
+package io.sentry;
+
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+public final class NoOpTransactionPerformanceCollector implements TransactionPerformanceCollector {
+
+  private static final NoOpTransactionPerformanceCollector instance =
+      new NoOpTransactionPerformanceCollector();
+
+  public static NoOpTransactionPerformanceCollector getInstance() {
+    return instance;
+  }
+
+  private NoOpTransactionPerformanceCollector() {}
+
+  @Override
+  public void start(@NotNull ITransaction transaction) {}
+
+  @Override
+  public @Nullable PerformanceCollectionData stop(@NotNull ITransaction transaction) {
+    return null;
+  }
+}

sentry/src/main/java/io/sentry/SentryOptions.java

@@ -397,7 +397,7 @@ public class SentryOptions {
 
   /** Performance collector that collect performance stats while transactions run. */
   private final @NotNull TransactionPerformanceCollector transactionPerformanceCollector =
-      new TransactionPerformanceCollector(this);
+      NoOpTransactionPerformanceCollector.getInstance();
 
   /**
    * Adds an event processor

sentry/src/main/java/io/sentry/TransactionPerformanceCollector.java

@@ -1,102 +1,12 @@
 package io.sentry;
 
-import io.sentry.util.Objects;
-import java.util.List;
-import java.util.Map;
-import java.util.Timer;
-import java.util.TimerTask;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.atomic.AtomicBoolean;
-import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-@ApiStatus.Internal
-public final class TransactionPerformanceCollector {
-  private static final long TRANSACTION_COLLECTION_INTERVAL_MILLIS = 100;
-  private static final long TRANSACTION_COLLECTION_TIMEOUT_MILLIS = 30000;
-  private final @NotNull Object timerLock = new Object();
-  private volatile @Nullable Timer timer = null;
-  private final @NotNull Map<String, PerformanceCollectionData> performanceDataMap =
-      new ConcurrentHashMap<>();
-  private final @NotNull List<ICollector> collectors;
-  private final @NotNull SentryOptions options;
-  private final @NotNull AtomicBoolean isStarted = new AtomicBoolean(false);
+public interface TransactionPerformanceCollector {
 
-  public TransactionPerformanceCollector(final @NotNull SentryOptions options) {
-    this.options = Objects.requireNonNull(options, "The options object is required.");
-    this.collectors = options.getCollectors();
-  }
+  void start(@NotNull ITransaction transaction);
 
-  @SuppressWarnings("FutureReturnValueIgnored")
-  public void start(final @NotNull ITransaction transaction) {
-    if (collectors.isEmpty()) {
-      options
-          .getLogger()
-          .log(
-              SentryLevel.INFO,
-              "No collector found. Performance stats will not be captured during transactions.");
-      return;
-    }
-
-    if (!performanceDataMap.containsKey(transaction.getEventId().toString())) {
-      performanceDataMap.put(transaction.getEventId().toString(), new PerformanceCollectionData());
-      options
-          .getExecutorService()
-          .schedule(
-              () -> {
-                PerformanceCollectionData data = stop(transaction);
-                if (data != null) {
-                  performanceDataMap.put(transaction.getEventId().toString(), data);
-                }
-              },
-              TRANSACTION_COLLECTION_TIMEOUT_MILLIS);
-    }
-    if (!isStarted.getAndSet(true)) {
-      synchronized (timerLock) {
-        for (ICollector collector : collectors) {
-          collector.setup();
-        }
-        if (timer == null) {
-          timer = new Timer(true);
-        }
-        // We schedule the timer to start after a delay, so we let some time pass between setup()
-        // and collect() calls.
-        // This way ICollectors that collect average stats based on time intervals, like
-        // AndroidCpuCollector, can have an actual time interval to evaluate.
-        timer.scheduleAtFixedRate(
-            new TimerTask() {
-              @Override
-              public void run() {
-                synchronized (timerLock) {
-                  for (ICollector collector : collectors) {
-                    collector.collect(performanceDataMap.values());
-                  }
-                  // We commit data after calling all collectors.
-                  // This way we avoid issues caused by having multiple cpu or memory collectors.
-                  for (PerformanceCollectionData data : performanceDataMap.values()) {
-                    data.commitData();
-                  }
-                }
-              }
-            },
-            TRANSACTION_COLLECTION_INTERVAL_MILLIS,
-            TRANSACTION_COLLECTION_INTERVAL_MILLIS);
-      }
-    }
-  }
-
-  public @Nullable PerformanceCollectionData stop(final @NotNull ITransaction transaction) {
-    synchronized (timerLock) {
-      PerformanceCollectionData data =
-          performanceDataMap.remove(transaction.getEventId().toString());
-      if (performanceDataMap.isEmpty() && isStarted.getAndSet(false)) {
-        if (timer != null) {
-          timer.cancel();
-          timer = null;
-        }
-      }
-      return data;
-    }
-  }
+  @Nullable
+  PerformanceCollectionData stop(@NotNull ITransaction transaction);
 }

sentry/src/test/java/io/sentry/SentryTracerTest.kt

@@ -34,7 +34,7 @@ class SentryTracerTest {
             options.environment = "environment"
             options.release = "release@3.0.0"
             hub = spy(Hub(options))
-            transactionPerformanceCollector = spy(TransactionPerformanceCollector(options))
+            transactionPerformanceCollector = spy(DefaultTransactionPerformanceCollector(options))
             hub.bindClient(mock())
         }
 

sentry/src/test/java/io/sentry/TransactionPerformanceCollectorTest.kt

@@ -24,7 +24,7 @@ import kotlin.test.assertTrue
 
 class TransactionPerformanceCollectorTest {
 
-    private val className = "io.sentry.TransactionPerformanceCollector"
+    private val className = "io.sentry.DefaultTransactionPerformanceCollector"
     private val ctorTypes: Array<Class<*>> = arrayOf(SentryOptions::class.java)
     private val fixture = Fixture()
 
@@ -70,7 +70,7 @@ class TransactionPerformanceCollectorTest {
             }
             transaction1 = SentryTracer(TransactionContext("", ""), hub)
             transaction2 = SentryTracer(TransactionContext("", ""), hub)
-            val collector = TransactionPerformanceCollector(options)
+            val collector = DefaultTransactionPerformanceCollector(options)
             val timer: Timer? = collector.getProperty("timer") ?: Timer(true)
             mockTimer = spy(timer)
             collector.injectForField("timer", mockTimer)