Deduplicate events happening in multiple threads simultaneously (#2845)

romtsn · web-flow · commit f60279b3bb7f · 2023-07-19T09:58:27.000Z
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,5 +1,12 @@
 # Changelog
 
+## Unreleased
+
+### Fixes
+
+- Deduplicate events happening in multiple threads simultaneously (e.g. `OutOfMemoryError`) ([#2845](https://github.com/getsentry/sentry-java/pull/2845))
+  - This will improve Crash-Free Session Rate as we no longer will send multiple Session updates with `Crashed` status, but only the one that is relevant
+
 ## 6.26.0
 
 ### Features
diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java
@@ -6,6 +6,7 @@
 import android.content.Context;
 import android.content.pm.PackageInfo;
 import android.os.Build;
+import io.sentry.DeduplicateMultithreadedEventProcessor;
 import io.sentry.DefaultTransactionPerformanceCollector;
 import io.sentry.ILogger;
 import io.sentry.SendFireAndForgetEnvelopeSender;
@@ -125,6 +126,7 @@ static void initializeIntegrationsAndProcessors(
       options.setEnvelopeDiskCache(new AndroidEnvelopeCache(options));
     }
 
+    options.addEventProcessor(new DeduplicateMultithreadedEventProcessor(options));
     options.addEventProcessor(
         new DefaultAndroidEventProcessor(context, buildInfoProvider, options));
     options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker));
diff --git a/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.java b/sentry-samples/sentry-samples-android/src/main/java/io/sentry/samples/android/MainActivity.java
@@ -21,9 +21,12 @@
 import java.io.InputStream;
 import java.io.OutputStreamWriter;
 import java.io.Writer;
+import java.util.ArrayList;
 import java.util.Calendar;
 import java.util.Collections;
+import java.util.List;
 import java.util.Locale;
+import java.util.concurrent.CountDownLatch;
 import timber.log.Timber;
 
 public class MainActivity extends AppCompatActivity {
@@ -139,6 +142,28 @@ protected void onCreate(Bundle savedInstanceState) {
           Sentry.setUser(user);
         });
 
+    binding.outOfMemory.setOnClickListener(
+        view -> {
+          final CountDownLatch latch = new CountDownLatch(1);
+          for (int i = 0; i < 20; i++) {
+            new Thread(
+                    () -> {
+                      final List<String> data = new ArrayList<>();
+                      try {
+                        latch.await();
+                        for (int j = 0; j < 1_000_000; j++) {
+                          data.add(new String(new byte[1024 * 8]));
+                        }
+                      } catch (InterruptedException e) {
+                        e.printStackTrace();
+                      }
+                    })
+                .start();
+          }
+
+          latch.countDown();
+        });
+
     binding.nativeCrash.setOnClickListener(view -> NativeSample.crash());
 
     binding.nativeCapture.setOnClickListener(view -> NativeSample.message());
diff --git a/sentry-samples/sentry-samples-android/src/main/res/layout/activity_main.xml b/sentry-samples/sentry-samples-android/src/main/res/layout/activity_main.xml
@@ -64,6 +64,12 @@
       android:layout_height="wrap_content"
       android:text="@string/anr" />
 
+    <Button
+      android:id="@+id/out_of_memory"
+      android:layout_width="wrap_content"
+      android:layout_height="wrap_content"
+      android:text="@string/out_of_memory" />
+
     <Button
       android:id="@+id/native_crash"
       android:layout_width="wrap_content"
diff --git a/sentry-samples/sentry-samples-android/src/main/res/values/strings.xml b/sentry-samples/sentry-samples-android/src/main/res/values/strings.xml
@@ -1,6 +1,7 @@
 <resources>
   <string name="app_name">Sentry sample</string>
   <string name="crash_from_java">Crash from Java (UncaughtException)</string>
+  <string name="out_of_memory">Out of Memory (Mulithreaded)</string>
   <string name="send_message">Send Message</string>
   <string name="send_message_from_inner_fragment">Send Message from inner fragment</string>
   <string name="add_attachment">Add Attachment</string>
diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api
@@ -196,6 +196,11 @@ public final class io/sentry/DateUtils {
 	public static fun toUtilDateNotNull (Lio/sentry/SentryDate;)Ljava/util/Date;
 }
 
+public final class io/sentry/DeduplicateMultithreadedEventProcessor : io/sentry/EventProcessor {
+	public fun <init> (Lio/sentry/SentryOptions;)V
+	public fun process (Lio/sentry/SentryEvent;Lio/sentry/Hint;)Lio/sentry/SentryEvent;
+}
+
 public final class io/sentry/DefaultTransactionPerformanceCollector : io/sentry/TransactionPerformanceCollector {
 	public fun <init> (Lio/sentry/SentryOptions;)V
 	public fun close ()V
@@ -1564,6 +1569,7 @@ public final class io/sentry/SentryEvent : io/sentry/SentryBaseEvent, io/sentry/
 	public fun getThreads ()Ljava/util/List;
 	public fun getTimestamp ()Ljava/util/Date;
 	public fun getTransaction ()Ljava/lang/String;
+	public fun getUnhandledException ()Lio/sentry/protocol/SentryException;
 	public fun getUnknown ()Ljava/util/Map;
 	public fun isCrashed ()Z
 	public fun isErrored ()Z
@@ -2388,6 +2394,7 @@ public final class io/sentry/TypeCheckHint {
 	public static final field OPEN_FEIGN_RESPONSE Ljava/lang/String;
 	public static final field SENTRY_DART_SDK_NAME Ljava/lang/String;
 	public static final field SENTRY_DOTNET_SDK_NAME Ljava/lang/String;
+	public static final field SENTRY_EVENT_DROP_REASON Ljava/lang/String;
 	public static final field SENTRY_IS_FROM_HYBRID_SDK Ljava/lang/String;
 	public static final field SENTRY_JAVASCRIPT_SDK_NAME Ljava/lang/String;
 	public static final field SENTRY_SYNTHETIC_EXCEPTION Ljava/lang/String;
@@ -2671,6 +2678,12 @@ public abstract interface class io/sentry/hints/DiskFlushNotification {
 	public abstract fun markFlushed ()V
 }
 
+public final class io/sentry/hints/EventDropReason : java/lang/Enum {
+	public static final field MULTITHREADED_DEDUPLICATION Lio/sentry/hints/EventDropReason;
+	public static fun valueOf (Ljava/lang/String;)Lio/sentry/hints/EventDropReason;
+	public static fun values ()[Lio/sentry/hints/EventDropReason;
+}
+
 public abstract interface class io/sentry/hints/Flushable {
 	public abstract fun waitFlush ()Z
 }
@@ -4154,13 +4167,15 @@ public final class io/sentry/util/FileUtils {
 
 public final class io/sentry/util/HintUtils {
 	public static fun createWithTypeCheckHint (Ljava/lang/Object;)Lio/sentry/Hint;
+	public static fun getEventDropReason (Lio/sentry/Hint;)Lio/sentry/hints/EventDropReason;
 	public static fun getSentrySdkHint (Lio/sentry/Hint;)Ljava/lang/Object;
 	public static fun hasType (Lio/sentry/Hint;Ljava/lang/Class;)Z
 	public static fun isFromHybridSdk (Lio/sentry/Hint;)Z
 	public static fun runIfDoesNotHaveType (Lio/sentry/Hint;Ljava/lang/Class;Lio/sentry/util/HintUtils$SentryNullableConsumer;)V
 	public static fun runIfHasType (Lio/sentry/Hint;Ljava/lang/Class;Lio/sentry/util/HintUtils$SentryConsumer;)V
 	public static fun runIfHasType (Lio/sentry/Hint;Ljava/lang/Class;Lio/sentry/util/HintUtils$SentryConsumer;Lio/sentry/util/HintUtils$SentryHintFallback;)V
 	public static fun runIfHasTypeLogIfNot (Lio/sentry/Hint;Ljava/lang/Class;Lio/sentry/ILogger;Lio/sentry/util/HintUtils$SentryConsumer;)V
+	public static fun setEventDropReason (Lio/sentry/Hint;Lio/sentry/hints/EventDropReason;)V
 	public static fun setIsFromHybridSdk (Lio/sentry/Hint;Ljava/lang/String;)V
 	public static fun setTypeCheckHint (Lio/sentry/Hint;Ljava/lang/Object;)V
 	public static fun shouldApplyScopeData (Lio/sentry/Hint;)Z
diff --git a/sentry/src/main/java/io/sentry/DeduplicateMultithreadedEventProcessor.java b/sentry/src/main/java/io/sentry/DeduplicateMultithreadedEventProcessor.java
@@ -0,0 +1,65 @@
+package io.sentry;
+
+import io.sentry.hints.EventDropReason;
+import io.sentry.protocol.SentryException;
+import io.sentry.util.HintUtils;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.Map;
+import org.jetbrains.annotations.NotNull;
+import org.jetbrains.annotations.Nullable;
+
+/**
+ * An event processor that deduplicates crash events of the same type that are simultaneously from
+ * multiple threads. This can be the case for OutOfMemory errors or CursorWindowAllocationException,
+ * basically any error related to allocating memory when it's low.
+ */
+public final class DeduplicateMultithreadedEventProcessor implements EventProcessor {
+
+  private final @NotNull Map<String, Long> processedEvents =
+      Collections.synchronizedMap(new HashMap<>());
+
+  private final @NotNull SentryOptions options;
+
+  public DeduplicateMultithreadedEventProcessor(final @NotNull SentryOptions options) {
+    this.options = options;
+  }
+
+  @Override
+  public @Nullable SentryEvent process(final @NotNull SentryEvent event, final @NotNull Hint hint) {
+    if (!HintUtils.hasType(hint, UncaughtExceptionHandlerIntegration.UncaughtExceptionHint.class)) {
+      // only dedupe crashes coming from our exception handler, because custom errors/crashes might
+      // be sent on purpose
+      return event;
+    }
+
+    final SentryException exception = event.getUnhandledException();
+    if (exception == null) {
+      return event;
+    }
+
+    final String type = exception.getType();
+    if (type == null) {
+      return event;
+    }
+
+    final Long currentEventTid = exception.getThreadId();
+    if (currentEventTid == null) {
+      return event;
+    }
+
+    final Long tid = processedEvents.get(type);
+    if (tid != null && !tid.equals(currentEventTid)) {
+      options
+          .getLogger()
+          .log(
+              SentryLevel.INFO,
+              "Event %s has been dropped due to multi-threaded deduplication",
+              event.getEventId());
+      HintUtils.setEventDropReason(hint, EventDropReason.MULTITHREADED_DEDUPLICATION);
+      return null;
+    }
+    processedEvents.put(type, currentEventTid);
+    return event;
+  }
+}
diff --git a/sentry/src/main/java/io/sentry/SentryEvent.java b/sentry/src/main/java/io/sentry/SentryEvent.java
@@ -211,17 +211,20 @@ public void removeModule(final @NotNull String key) {
    * @return true if its crashed or false otherwise
    */
   public boolean isCrashed() {
+    return getUnhandledException() != null;
+  }
+
+  public @Nullable SentryException getUnhandledException() {
     if (exception != null) {
       for (SentryException e : exception.getValues()) {
         if (e.getMechanism() != null
             && e.getMechanism().isHandled() != null
             && !e.getMechanism().isHandled()) {
-          return true;
+          return e;
         }
       }
     }
-
-    return false;
+    return null;
   }
 
   /**
diff --git a/sentry/src/main/java/io/sentry/TypeCheckHint.java b/sentry/src/main/java/io/sentry/TypeCheckHint.java
@@ -10,6 +10,9 @@ public final class TypeCheckHint {
   @ApiStatus.Internal
   public static final String SENTRY_IS_FROM_HYBRID_SDK = "sentry:isFromHybridSdk";
 
+  @ApiStatus.Internal
+  public static final String SENTRY_EVENT_DROP_REASON = "sentry:eventDropReason";
+
   @ApiStatus.Internal public static final String SENTRY_JAVASCRIPT_SDK_NAME = "sentry.javascript";
 
   @ApiStatus.Internal public static final String SENTRY_DOTNET_SDK_NAME = "sentry.dotnet";
diff --git a/sentry/src/main/java/io/sentry/UncaughtExceptionHandlerIntegration.java b/sentry/src/main/java/io/sentry/UncaughtExceptionHandlerIntegration.java
@@ -3,6 +3,7 @@
 import com.jakewharton.nopen.annotation.Open;
 import io.sentry.exception.ExceptionMechanismException;
 import io.sentry.hints.BlockingFlushHint;
+import io.sentry.hints.EventDropReason;
 import io.sentry.hints.SessionEnd;
 import io.sentry.protocol.Mechanism;
 import io.sentry.protocol.SentryId;
@@ -98,7 +99,11 @@ public void uncaughtException(Thread thread, Throwable thrown) {
 
         final @NotNull SentryId sentryId = hub.captureEvent(event, hint);
         final boolean isEventDropped = sentryId.equals(SentryId.EMPTY_ID);
-        if (!isEventDropped) {
+        final EventDropReason eventDropReason = HintUtils.getEventDropReason(hint);
+        // in case the event has been dropped by multithreaded deduplicator, the other threads will
+        // crash the app without a chance to persist the main event so we have to special-case this
+        if (!isEventDropped
+            || EventDropReason.MULTITHREADED_DEDUPLICATION.equals(eventDropReason)) {
           // Block until the event is flushed to disk
           if (!exceptionHint.waitFlush()) {
             options
diff --git a/sentry/src/main/java/io/sentry/hints/EventDropReason.java b/sentry/src/main/java/io/sentry/hints/EventDropReason.java
@@ -0,0 +1,9 @@
+package io.sentry.hints;
+
+import org.jetbrains.annotations.ApiStatus;
+
+/** A reason for which an event was dropped, used for (not to confuse with ClientReports) */
+@ApiStatus.Internal
+public enum EventDropReason {
+  MULTITHREADED_DEDUPLICATION
+}
diff --git a/sentry/src/main/java/io/sentry/util/HintUtils.java b/sentry/src/main/java/io/sentry/util/HintUtils.java
@@ -2,6 +2,7 @@
 
 import static io.sentry.TypeCheckHint.SENTRY_DART_SDK_NAME;
 import static io.sentry.TypeCheckHint.SENTRY_DOTNET_SDK_NAME;
+import static io.sentry.TypeCheckHint.SENTRY_EVENT_DROP_REASON;
 import static io.sentry.TypeCheckHint.SENTRY_IS_FROM_HYBRID_SDK;
 import static io.sentry.TypeCheckHint.SENTRY_JAVASCRIPT_SDK_NAME;
 import static io.sentry.TypeCheckHint.SENTRY_TYPE_CHECK_HINT;
@@ -11,6 +12,7 @@
 import io.sentry.hints.ApplyScopeData;
 import io.sentry.hints.Backfillable;
 import io.sentry.hints.Cached;
+import io.sentry.hints.EventDropReason;
 import org.jetbrains.annotations.ApiStatus;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -33,6 +35,16 @@ public static boolean isFromHybridSdk(final @NotNull Hint hint) {
     return Boolean.TRUE.equals(hint.getAs(SENTRY_IS_FROM_HYBRID_SDK, Boolean.class));
   }
 
+  public static void setEventDropReason(
+      final @NotNull Hint hint, final @NotNull EventDropReason eventDropReason) {
+    hint.set(SENTRY_EVENT_DROP_REASON, eventDropReason);
+  }
+
+  @Nullable
+  public static EventDropReason getEventDropReason(final @NotNull Hint hint) {
+    return hint.getAs(SENTRY_EVENT_DROP_REASON, EventDropReason.class);
+  }
+
   public static Hint createWithTypeCheckHint(Object typeCheckHint) {
     Hint hint = new Hint();
     setTypeCheckHint(hint, typeCheckHint);
diff --git a/sentry/src/test/java/io/sentry/DeduplicateMultithreadedEventProcessorTest.kt b/sentry/src/test/java/io/sentry/DeduplicateMultithreadedEventProcessorTest.kt
diff --git a/sentry/src/test/java/io/sentry/UncaughtExceptionHandlerIntegrationTest.kt b/sentry/src/test/java/io/sentry/UncaughtExceptionHandlerIntegrationTest.kt

Original file line number	Diff line number	Diff line change
`@@ -211,17 +211,20 @@ public void removeModule(final @NotNull String key) {`
`211`	`211`	`* @return true if its crashed or false otherwise`
`212`	`212`	`*/`
`213`	`213`	`public boolean isCrashed() {`
	`214`	`+ return getUnhandledException() != null;`
	`215`	`+ }`
	`216`	`+`
	`217`	`+ public @Nullable SentryException getUnhandledException() {`
`214`	`218`	`if (exception != null) {`
`215`	`219`	`for (SentryException e : exception.getValues()) {`
`216`	`220`	`if (e.getMechanism() != null`
`217`	`221`	`&& e.getMechanism().isHandled() != null`
`218`	`222`	`&& !e.getMechanism().isHandled()) {`
`219`		`- return true;`
	`223`	`+ return e;`
`220`	`224`	`}`
`221`	`225`	`}`
`222`	`226`	`}`
`223`		`-`
`224`		`- return false;`
	`227`	`+ return null;`
`225`	`228`	`}`
`226`	`229`
`227`	`230`	`/**`