Synchronize Baggage keyValues

CHANGELOG.md

@@ -8,6 +8,7 @@
   - This ensures correct resource loading in environments like Spring Boot where the thread context classloader is used for resource loading.
 - Improve low memory breadcrumb capturing ([#4325](https://github.com/getsentry/sentry-java/pull/4325))
 - Fix do not initialize SDK for Jetpack Compose Preview builds ([#4324](https://github.com/getsentry/sentry-java/pull/4324))
+- Fix Synchronize Baggage values ([#4327](https://github.com/getsentry/sentry-java/pull/4327))
 
 ## 8.7.0
 

sentry/src/main/java/io/sentry/Baggage.java

@@ -13,6 +13,7 @@
 import java.text.DecimalFormatSymbols;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
@@ -44,13 +45,13 @@ protected DecimalFormat initialValue() {
   private static final DecimalFormatterThreadLocal decimalFormatter =
       new DecimalFormatterThreadLocal();
 
-  final @NotNull Map<String, String> keyValues;
-  @Nullable Double sampleRate;
-  @Nullable Double sampleRand;
+  private final @NotNull Map<String, String> keyValues;
+  private @Nullable Double sampleRate;
+  private @Nullable Double sampleRand;
 
-  final @Nullable String thirdPartyHeader;
+  private final @Nullable String thirdPartyHeader;
   private boolean mutable;
-  private boolean shouldFreeze;
+  private final boolean shouldFreeze;
   final @NotNull ILogger logger;
 
   @NotNull
@@ -217,10 +218,12 @@ public Baggage(
       final @Nullable Double sampleRate,
       final @Nullable Double sampleRand,
       final @Nullable String thirdPartyHeader,
-      boolean isMutable,
-      boolean shouldFreeze,
+      final boolean isMutable,
+      final boolean shouldFreeze,
       final @NotNull ILogger logger) {
-    this.keyValues = keyValues;
+    // TODO should we deep-copy the keyValues here?
+    // if so we could optimize this by only synchronizing in case isMutable is true
+    this.keyValues = Collections.synchronizedMap(keyValues);
     this.sampleRate = sampleRate;
     this.sampleRand = sampleRand;
     this.logger = logger;
@@ -260,7 +263,10 @@ public String getThirdPartyHeader() {
       separator = ",";
     }
 
-    final Set<String> keys = new TreeSet<>(keyValues.keySet());
+    final Set<String> keys;
+    synchronized (keyValues) {
+      keys = new TreeSet<>(keyValues.keySet());
+    }
     keys.add(DSCKeys.SAMPLE_RATE);
     keys.add(DSCKeys.SAMPLE_RAND);
 
@@ -440,38 +446,34 @@ public void setReplayId(final @Nullable String replayId) {
     set(DSCKeys.REPLAY_ID, replayId);
   }
 
-  @ApiStatus.Internal
-  public void set(final @NotNull String key, final @Nullable String value) {
-    set(key, value, false);
-  }
-
   /**
-   * Sets / updates a value
+   * Sets / updates a value, but only if the baggage is still mutable.
    *
    * @param key key
    * @param value value to set
-   * @param force ignores mutability of this baggage and sets the value anyways
    */
-  private void set(final @NotNull String key, final @Nullable String value, final boolean force) {
-    if (mutable || force) {
+  @ApiStatus.Internal
+  public void set(final @NotNull String key, final @Nullable String value) {
+    if (mutable) {
       this.keyValues.put(key, value);
     }
   }
 
   @ApiStatus.Internal
   public @NotNull Map<String, Object> getUnknown() {
     final @NotNull Map<String, Object> unknown = new ConcurrentHashMap<>();
-    for (Map.Entry<String, String> keyValue : this.keyValues.entrySet()) {
-      final @NotNull String key = keyValue.getKey();
-      final @Nullable String value = keyValue.getValue();
-      if (!DSCKeys.ALL.contains(key)) {
-        if (value != null) {
-          final @NotNull String unknownKey = key.replaceFirst(SENTRY_BAGGAGE_PREFIX, "");
-          unknown.put(unknownKey, value);
+    synchronized (keyValues) {
+      for (final Map.Entry<String, String> keyValue : keyValues.entrySet()) {
+        final @NotNull String key = keyValue.getKey();
+        final @Nullable String value = keyValue.getValue();
+        if (!DSCKeys.ALL.contains(key)) {
+          if (value != null) {
+            final @NotNull String unknownKey = key.replaceFirst(SENTRY_BAGGAGE_PREFIX, "");
+            unknown.put(unknownKey, value);
+          }
         }
       }
     }
-
     return unknown;
   }
 

sentry/src/main/java/io/sentry/SpanContext.java

@@ -125,6 +125,7 @@ public SpanContext(final @NotNull SpanContext spanContext) {
     if (copiedUnknown != null) {
       this.unknown = copiedUnknown;
     }
+    // TODO should this be a deep copy instead?
     this.baggage = spanContext.baggage;
     final Map<String, Object> copiedData = CollectionUtils.newConcurrentHashMap(spanContext.data);
     if (copiedData != null) {