clone method - race condition free

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

@@ -10,6 +10,7 @@ import io.sentry.core.protocol.Contexts
 import io.sentry.core.protocol.Device
 import java.io.StringReader
 import java.io.StringWriter
+import java.util.Date
 import java.util.TimeZone
 import java.util.UUID
 import kotlin.test.Test
@@ -28,8 +29,7 @@ class AndroidSerializerTest {
 
     @Test
     fun `when serializing SentryEvent-SentryId object, it should become a event_id json without dashes`() {
-        val sentryEvent = generateEmptySentryEvent()
-        sentryEvent.timestamp = null
+        val sentryEvent = generateEmptySentryEvent(null)
 
         val actual = serializeToString(sentryEvent)
 
@@ -50,10 +50,9 @@ class AndroidSerializerTest {
 
     @Test
     fun `when serializing SentryEvent-Date, it should become a timestamp json ISO format`() {
-        val sentryEvent = generateEmptySentryEvent()
         val dateIsoFormat = "2000-12-31T23:59:58Z"
+        val sentryEvent = generateEmptySentryEvent(DateUtils.getDateTime(dateIsoFormat))
         sentryEvent.eventId = null
-        sentryEvent.timestamp = DateUtils.getDateTime(dateIsoFormat)
 
         val expected = "{\"timestamp\":\"$dateIsoFormat\"}"
 
@@ -64,11 +63,8 @@ class AndroidSerializerTest {
 
     @Test
     fun `when deserializing timestamp, it should become a SentryEvent-Date`() {
-        val sentryEvent = generateEmptySentryEvent()
         val dateIsoFormat = "2000-12-31T23:59:58Z"
-        sentryEvent.eventId = null
         val expected = DateUtils.getDateTime(dateIsoFormat)
-        sentryEvent.timestamp = expected
 
         val jsonEvent = "{\"timestamp\":\"$dateIsoFormat\"}"
 
@@ -81,7 +77,6 @@ class AndroidSerializerTest {
     fun `when deserializing unknown properties, it should be added to unknown field`() {
         val sentryEvent = generateEmptySentryEvent()
         sentryEvent.eventId = null
-        sentryEvent.timestamp = null
 
         val jsonEvent = "{\"string\":\"test\",\"int\":1,\"boolean\":true}"
 
@@ -96,7 +91,6 @@ class AndroidSerializerTest {
     fun `when deserializing unknown properties with nested objects, it should be added to unknown field`() {
         val sentryEvent = generateEmptySentryEvent()
         sentryEvent.eventId = null
-        sentryEvent.timestamp = null
 
         val objects = hashMapOf<String, Any>()
         objects["int"] = 1
@@ -118,9 +112,8 @@ class AndroidSerializerTest {
 
     @Test
     fun `when serializing unknown field, it should become unknown as json format`() {
-        val sentryEvent = generateEmptySentryEvent()
+        val sentryEvent = generateEmptySentryEvent(null)
         sentryEvent.eventId = null
-        sentryEvent.timestamp = null
 
         val objects = hashMapOf<String, Any>()
         objects["int"] = 1
@@ -140,9 +133,8 @@ class AndroidSerializerTest {
 
     @Test
     fun `when serializing a TimeZone, it should become a timezone ID string`() {
-        val sentryEvent = generateEmptySentryEvent()
+        val sentryEvent = generateEmptySentryEvent(null)
         sentryEvent.eventId = null
-        sentryEvent.timestamp = null
         val device = Device()
         device.timezone = TimeZone.getTimeZone("Europe/Vienna")
         val contexts = Contexts()
@@ -160,7 +152,6 @@ class AndroidSerializerTest {
     fun `when deserializing a timezone ID string, it should become a Device-TimeZone`() {
         val sentryEvent = generateEmptySentryEvent()
         sentryEvent.eventId = null
-        sentryEvent.timestamp = null
 
         val jsonEvent = "{\"contexts\":{\"device\":{\"timezone\":\"Europe/Vienna\"}}}"
 
@@ -171,9 +162,8 @@ class AndroidSerializerTest {
 
     @Test
     fun `when serializing a DeviceOrientation, it should become an orientation string`() {
-        val sentryEvent = generateEmptySentryEvent()
+        val sentryEvent = generateEmptySentryEvent(null)
         sentryEvent.eventId = null
-        sentryEvent.timestamp = null
         val device = Device()
         device.orientation = Device.DeviceOrientation.LANDSCAPE
         val contexts = Contexts()
@@ -191,7 +181,6 @@ class AndroidSerializerTest {
     fun `when deserializing an orientation string, it should become a DeviceOrientation`() {
         val sentryEvent = generateEmptySentryEvent()
         sentryEvent.eventId = null
-        sentryEvent.timestamp = null
 
         val jsonEvent = "{\"contexts\":{\"device\":{\"orientation\":\"landscape\"}}}"
 
@@ -202,9 +191,8 @@ class AndroidSerializerTest {
 
     @Test
     fun `when serializing a SentryLevel, it should become a sentry level string`() {
-        val sentryEvent = generateEmptySentryEvent()
+        val sentryEvent = generateEmptySentryEvent(null)
         sentryEvent.eventId = null
-        sentryEvent.timestamp = null
         sentryEvent.level = SentryLevel.DEBUG
 
         val expected = "{\"level\":\"debug\"}"
@@ -218,7 +206,6 @@ class AndroidSerializerTest {
     fun `when deserializing a sentry level string, it should become a SentryLevel`() {
         val sentryEvent = generateEmptySentryEvent()
         sentryEvent.eventId = null
-        sentryEvent.timestamp = null
 
         val jsonEvent = "{\"level\":\"debug\"}"
 
@@ -234,8 +221,8 @@ class AndroidSerializerTest {
         assertNull(event.user)
     }
 
-    private fun generateEmptySentryEvent(): SentryEvent {
-        return SentryEvent().apply {
+    private fun generateEmptySentryEvent(date: Date? = null): SentryEvent {
+        return SentryEvent(date).apply {
             contexts = null
         }
     }

sentry-core/src/main/java/io/sentry/core/Breadcrumb.java

@@ -10,15 +10,15 @@
 
 public final class Breadcrumb implements Cloneable, IUnknownPropertiesConsumer {
 
-  private Date timestamp;
+  private final Date timestamp;
   private String message;
   private String type;
   private Map<String, String> data;
   private String category;
   private SentryLevel level;
   private Map<String, Object> unknown;
 
-  Breadcrumb(Date timestamp) {
+  Breadcrumb(final Date timestamp) {
     this.timestamp = timestamp;
   }
 
@@ -35,10 +35,6 @@ public Date getTimestamp() {
     return (Date) timestamp.clone();
   }
 
-  void setTimestamp(Date timestamp) {
-    this.timestamp = timestamp;
-  }
-
   public String getMessage() {
     return message;
   }
@@ -92,14 +88,13 @@ Map<String, Object> getUnknown() {
 
   @Override
   public Breadcrumb clone() throws CloneNotSupportedException {
-    Breadcrumb clone = (Breadcrumb) super.clone();
-
-    clone.timestamp = timestamp != null ? (Date) timestamp.clone() : null;
+    final Breadcrumb clone = (Breadcrumb) super.clone();
 
-    if (data != null) {
-      Map<String, String> dataClone = new ConcurrentHashMap<>();
+    final Map<String, String> dataRef = data;
+    if (dataRef != null) {
+      final Map<String, String> dataClone = new ConcurrentHashMap<>();
 
-      for (Map.Entry<String, String> item : data.entrySet()) {
+      for (Map.Entry<String, String> item : dataRef.entrySet()) {
         if (item != null) {
           dataClone.put(item.getKey(), item.getValue());
         }
@@ -110,10 +105,11 @@ public Breadcrumb clone() throws CloneNotSupportedException {
       clone.data = null;
     }
 
-    if (unknown != null) {
-      Map<String, Object> unknownClone = new HashMap<>();
+    final Map<String, Object> unknownRef = unknown;
+    if (unknownRef != null) {
+      final Map<String, Object> unknownClone = new HashMap<>();
 
-      for (Map.Entry<String, Object> item : unknown.entrySet()) {
+      for (Map.Entry<String, Object> item : unknownRef.entrySet()) {
         if (item != null) {
           unknownClone.put(item.getKey(), item.getValue()); // shallow copy
         }
@@ -124,7 +120,9 @@ public Breadcrumb clone() throws CloneNotSupportedException {
       clone.unknown = null;
     }
 
-    clone.level = level != null ? SentryLevel.valueOf(level.name().toUpperCase(Locale.ROOT)) : null;
+    final SentryLevel levelRef = level;
+    clone.level =
+        levelRef != null ? SentryLevel.valueOf(levelRef.name().toUpperCase(Locale.ROOT)) : null;
 
     return clone;
   }

sentry-core/src/main/java/io/sentry/core/Hub.java

@@ -503,7 +503,7 @@ public void flush(long timeoutMills) {
       options.getLogger().log(SentryLevel.WARNING, "Disabled Hub cloned.");
     }
     // Clone will be invoked in parallel
-    Hub clone = new Hub(this.options, null);
+    final Hub clone = new Hub(this.options, null);
     for (StackItem item : this.stack) {
       Scope clonedScope;
       try {
@@ -513,7 +513,7 @@ public void flush(long timeoutMills) {
         options.getLogger().log(SentryLevel.ERROR, "Clone not supported");
         clonedScope = new Scope(options.getMaxBreadcrumbs(), options.getBeforeBreadcrumb());
       }
-      StackItem cloneItem = new StackItem(item.client, clonedScope);
+      final StackItem cloneItem = new StackItem(item.client, clonedScope);
       clone.stack.push(cloneItem);
     }
     return clone;

sentry-core/src/main/java/io/sentry/core/Scope.java

@@ -8,6 +8,7 @@
 import java.util.Map;
 import java.util.Queue;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
@@ -21,17 +22,17 @@ public final class Scope implements Cloneable {
   private @NotNull Map<String, Object> extra = new ConcurrentHashMap<>();
   private final int maxBreadcrumb;
   private @Nullable final SentryOptions.BeforeBreadcrumbCallback beforeBreadcrumbCallback;
-  private @NotNull List<EventProcessor> eventProcessors = new ArrayList<>();
+  private @NotNull List<EventProcessor> eventProcessors = new CopyOnWriteArrayList<>();
 
   public Scope(
-      int maxBreadcrumb,
+      final int maxBreadcrumb,
       @Nullable final SentryOptions.BeforeBreadcrumbCallback beforeBreadcrumbCallback) {
     this.maxBreadcrumb = maxBreadcrumb;
     this.beforeBreadcrumbCallback = beforeBreadcrumbCallback;
     this.breadcrumbs = createBreadcrumbsList(this.maxBreadcrumb);
   }
 
-  public Scope(int maxBreadcrumb) {
+  public Scope(final int maxBreadcrumb) {
     this(maxBreadcrumb, null);
   }
 
@@ -150,52 +151,52 @@ public void removeExtra(@NotNull String key) {
 
   @Override
   public Scope clone() throws CloneNotSupportedException {
-    Scope clone = (Scope) super.clone();
-    clone.level = level != null ? SentryLevel.valueOf(level.name().toUpperCase(Locale.ROOT)) : null;
-    clone.user = user != null ? user.clone() : null;
-    clone.fingerprint = fingerprint != null ? new ArrayList<>(fingerprint) : null;
-    clone.eventProcessors = eventProcessors != null ? new ArrayList<>(eventProcessors) : null;
-
-    if (breadcrumbs != null) {
-      Queue<Breadcrumb> breadcrumbsClone = createBreadcrumbsList(maxBreadcrumb);
-
-      for (Breadcrumb item : breadcrumbs) {
-        Breadcrumb breadcrumbClone = item.clone();
-        breadcrumbsClone.add(breadcrumbClone);
-      }
-      clone.breadcrumbs = breadcrumbsClone;
-    } else {
-      clone.breadcrumbs = null;
-    }
+    final Scope clone = (Scope) super.clone();
 
-    if (tags != null) {
-      Map<String, String> tagsClone = new ConcurrentHashMap<>();
+    final SentryLevel levelRef = level;
+    clone.level =
+        levelRef != null ? SentryLevel.valueOf(levelRef.name().toUpperCase(Locale.ROOT)) : null;
 
-      for (Map.Entry<String, String> item : tags.entrySet()) {
-        if (item != null) {
-          tagsClone.put(item.getKey(), item.getValue());
-        }
-      }
+    final User userRef = user;
+    clone.user = userRef != null ? userRef.clone() : null;
 
-      clone.tags = tagsClone;
-    } else {
-      clone.tags = null;
+    clone.fingerprint = new ArrayList<>(fingerprint);
+    clone.eventProcessors = new CopyOnWriteArrayList<>(eventProcessors);
+
+    final Queue<Breadcrumb> breadcrumbsRef = breadcrumbs;
+
+    Queue<Breadcrumb> breadcrumbsClone = createBreadcrumbsList(maxBreadcrumb);
+
+    for (Breadcrumb item : breadcrumbsRef) {
+      final Breadcrumb breadcrumbClone = item.clone();
+      breadcrumbsClone.add(breadcrumbClone);
     }
+    clone.breadcrumbs = breadcrumbsClone;
 
-    if (extra != null) {
-      Map<String, Object> extraClone = new HashMap<>();
+    final Map<String, String> tagsRef = tags;
 
-      for (Map.Entry<String, Object> item : extra.entrySet()) {
-        if (item != null) {
-          extraClone.put(item.getKey(), item.getValue());
-        }
+    final Map<String, String> tagsClone = new ConcurrentHashMap<>();
+
+    for (Map.Entry<String, String> item : tagsRef.entrySet()) {
+      if (item != null) {
+        tagsClone.put(item.getKey(), item.getValue());
       }
+    }
+
+    clone.tags = tagsClone;
+
+    final Map<String, Object> extraRef = extra;
 
-      clone.extra = extraClone;
-    } else {
-      clone.extra = null;
+    Map<String, Object> extraClone = new HashMap<>();
+
+    for (Map.Entry<String, Object> item : extraRef.entrySet()) {
+      if (item != null) {
+        extraClone.put(item.getKey(), item.getValue());
+      }
     }
 
+    clone.extra = extraClone;
+
     return clone;
   }
 

sentry-core/src/main/java/io/sentry/core/SentryEvent.java

@@ -12,7 +12,7 @@
 
 public final class SentryEvent implements IUnknownPropertiesConsumer {
   private SentryId eventId;
-  private Date timestamp;
+  private final Date timestamp;
   private transient Throwable throwable;
   private Message message;
   private String serverName;
@@ -37,7 +37,7 @@ public final class SentryEvent implements IUnknownPropertiesConsumer {
   private Map<String, String> modules;
   private DebugMeta debugMeta;
 
-  SentryEvent(SentryId eventId, Date timestamp) {
+  SentryEvent(SentryId eventId, final Date timestamp) {
     this.eventId = eventId;
     this.timestamp = timestamp;
   }
@@ -51,6 +51,11 @@ public SentryEvent() {
     this(new SentryId(), DateUtils.getCurrentDateTime());
   }
 
+  @TestOnly
+  public SentryEvent(final Date timestamp) {
+    this(new SentryId(), timestamp);
+  }
+
   public SentryId getEventId() {
     return eventId;
   }
@@ -135,11 +140,6 @@ public void setEventId(SentryId eventId) {
     this.eventId = eventId;
   }
 
-  @TestOnly
-  public void setTimestamp(Date timestamp) {
-    this.timestamp = timestamp;
-  }
-
   public void setThrowable(Throwable throwable) {
     this.throwable = throwable;
   }

sentry-core/src/main/java/io/sentry/core/protocol/App.java

@@ -28,7 +28,8 @@ public void setAppIdentifier(String appIdentifier) {
   }
 
   public Date getAppStartTime() {
-    return appStartTime != null ? (Date) appStartTime.clone() : null;
+    final Date appStartTimeRef = appStartTime;
+    return appStartTimeRef != null ? (Date) appStartTimeRef.clone() : null;
   }
 
   public void setAppStartTime(Date appStartTime) {