diff --git a/sentry/src/main/java/io/sentry/Breadcrumb.java b/sentry/src/main/java/io/sentry/Breadcrumb.java index d122d1459b..c670393725 100644 --- a/sentry/src/main/java/io/sentry/Breadcrumb.java +++ b/sentry/src/main/java/io/sentry/Breadcrumb.java @@ -34,8 +34,10 @@ public final class Breadcrumb implements JsonUnknown, JsonSerializable, Comparab /** The type of breadcrumb. */ private @Nullable String type; + private static final @NotNull Map EMPTY_DATA = Collections.emptyMap(); + /** Data associated with this breadcrumb. */ - private @NotNull Map data = new ConcurrentHashMap<>(); + private volatile @NotNull Map data = EMPTY_DATA; /** Dotted strings that indicate what the crumb is or where it comes from. */ private @Nullable String category; @@ -78,9 +80,11 @@ public Breadcrumb(final long timestamp) { this.type = breadcrumb.type; this.category = breadcrumb.category; this.origin = breadcrumb.origin; - final Map dataClone = CollectionUtils.newConcurrentHashMap(breadcrumb.data); - if (dataClone != null) { - this.data = dataClone; + if (!breadcrumb.data.isEmpty()) { + final Map dataClone = CollectionUtils.newConcurrentHashMap(breadcrumb.data); + if (dataClone != null) { + this.data = dataClone; + } } this.unknown = CollectionUtils.newConcurrentHashMap(breadcrumb.unknown); this.level = breadcrumb.level; @@ -100,7 +104,7 @@ public static Breadcrumb fromMap( @NotNull Date timestamp = DateUtils.getCurrentDateTime(); String message = null; String type = null; - @NotNull Map data = new ConcurrentHashMap<>(); + Map data = null; String category = null; String origin = null; SentryLevel level = null; @@ -129,6 +133,9 @@ public static Breadcrumb fromMap( if (untypedData != null) { for (Map.Entry dataEntry : untypedData.entrySet()) { if (dataEntry.getKey() instanceof String && dataEntry.getValue() != null) { + if (data == null) { + data = new ConcurrentHashMap<>(); + } data.put((String) dataEntry.getKey(), dataEntry.getValue()); } else { options @@ -166,7 +173,9 @@ public static Breadcrumb fromMap( final Breadcrumb breadcrumb = new Breadcrumb(timestamp); breadcrumb.message = message; breadcrumb.type = type; - breadcrumb.data = data; + if (data != null) { + breadcrumb.data = data; + } breadcrumb.category = category; breadcrumb.origin = origin; breadcrumb.level = level; @@ -494,7 +503,7 @@ public static Breadcrumb fromMap( breadcrumb.setData("view.tag", viewTag); } for (final Map.Entry entry : additionalData.entrySet()) { - breadcrumb.getData().put(entry.getKey(), entry.getValue()); + breadcrumb.setData(entry.getKey(), entry.getValue()); } breadcrumb.setLevel(SentryLevel.INFO); return breadcrumb; @@ -598,6 +607,20 @@ public void setType(@Nullable String type) { this.type = type; } + private @NotNull Map getOrCreateData() { + Map currentData = data; + if (currentData == EMPTY_DATA) { + synchronized (this) { + currentData = data; + if (currentData == EMPTY_DATA) { + currentData = new ConcurrentHashMap<>(); + data = currentData; + } + } + } + return currentData; + } + /** * Returns the data map * @@ -636,7 +659,7 @@ public void setData(@Nullable String key, @Nullable Object value) { if (value == null) { removeData(key); } else { - data.put(key, value); + getOrCreateData().put(key, value); } } @@ -649,7 +672,10 @@ public void removeData(@Nullable String key) { if (key == null) { return; } - data.remove(key); + final Map currentData = data; + if (currentData != EMPTY_DATA) { + currentData.remove(key); + } } /** @@ -859,7 +885,7 @@ public static final class Deserializer implements JsonDeserializer { @NotNull Date timestamp = DateUtils.getCurrentDateTime(); String message = null; String type = null; - @NotNull Map data = new ConcurrentHashMap<>(); + Map data = null; String category = null; String origin = null; SentryLevel level = null; @@ -884,7 +910,7 @@ public static final class Deserializer implements JsonDeserializer { Map deserializedData = CollectionUtils.newConcurrentHashMap( (Map) reader.nextObjectOrNull()); - if (deserializedData != null) { + if (deserializedData != null && !deserializedData.isEmpty()) { data = deserializedData; } break; @@ -913,7 +939,9 @@ public static final class Deserializer implements JsonDeserializer { Breadcrumb breadcrumb = new Breadcrumb(timestamp); breadcrumb.message = message; breadcrumb.type = type; - breadcrumb.data = data; + if (data != null) { + breadcrumb.data = data; + } breadcrumb.category = category; breadcrumb.origin = origin; breadcrumb.level = level; diff --git a/sentry/src/test/java/io/sentry/BreadcrumbTest.kt b/sentry/src/test/java/io/sentry/BreadcrumbTest.kt index 30c322641b..6c99115a83 100644 --- a/sentry/src/test/java/io/sentry/BreadcrumbTest.kt +++ b/sentry/src/test/java/io/sentry/BreadcrumbTest.kt @@ -1,6 +1,9 @@ package io.sentry import java.util.Date +import java.util.concurrent.CountDownLatch +import java.util.concurrent.Executors +import java.util.concurrent.TimeUnit import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -329,6 +332,30 @@ class BreadcrumbTest { breadcrumb.removeData(null) } + @Test + fun `concurrent first writes keep all data entries`() { + val breadcrumb = Breadcrumb() + val count = 32 + val executor = Executors.newFixedThreadPool(count) + val start = CountDownLatch(1) + val futures = + (0 until count).map { index -> + executor.submit { + start.await() + breadcrumb.setData("key-$index", index) + } + } + + start.countDown() + futures.forEach { it.get(5, TimeUnit.SECONDS) } + executor.shutdown() + + assertEquals(count, breadcrumb.data.size) + for (index in 0 until count) { + assertEquals(index, breadcrumb.data["key-$index"]) + } + } + class TestKey(val id: Long) { override fun toString(): String = id.toString() }