Use AutoClosableReentrantLock in favor of synchronized

Author: markushi

sentry/api/sentry.api

@@ -35,7 +35,7 @@ public abstract interface class io/sentry/BackfillingEventProcessor : io/sentry/
 public final class io/sentry/Baggage {
 	public fun <init> (Lio/sentry/Baggage;)V
 	public fun <init> (Lio/sentry/ILogger;)V
-	public fun <init> (Ljava/util/Map;Ljava/lang/Double;Ljava/lang/Double;Ljava/lang/String;ZZLio/sentry/ILogger;)V
+	public fun <init> (Ljava/util/concurrent/ConcurrentHashMap;Ljava/lang/Double;Ljava/lang/Double;Ljava/lang/String;ZZLio/sentry/ILogger;)V
 	public fun forceSetSampleRate (Ljava/lang/Double;)V
 	public fun freeze ()V
 	public static fun fromEvent (Lio/sentry/SentryEvent;Lio/sentry/SentryOptions;)Lio/sentry/Baggage;

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

@@ -4,6 +4,7 @@
 
 import io.sentry.protocol.SentryId;
 import io.sentry.protocol.TransactionNameSource;
+import io.sentry.util.AutoClosableReentrantLock;
 import io.sentry.util.SampleRateUtils;
 import io.sentry.util.StringUtils;
 import java.io.UnsupportedEncodingException;
@@ -14,7 +15,6 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Locale;
 import java.util.Map;
@@ -45,7 +45,9 @@ protected DecimalFormat initialValue() {
   private static final DecimalFormatterThreadLocal decimalFormatter =
       new DecimalFormatterThreadLocal();
 
-  private final @NotNull Map<String, String> keyValues;
+  private final @NotNull ConcurrentHashMap<String, String> keyValues;
+  private final @NotNull AutoClosableReentrantLock keyValuesLock = new AutoClosableReentrantLock();
+
   private @Nullable Double sampleRate;
   private @Nullable Double sampleRand;
 
@@ -100,7 +102,9 @@ public static Baggage fromHeader(
       final @Nullable String headerValue,
       final boolean includeThirdPartyValues,
       final @NotNull ILogger logger) {
-    final @NotNull Map<String, String> keyValues = new HashMap<>();
+
+    final @NotNull ConcurrentHashMap<String, String> keyValues = new ConcurrentHashMap<>();
+
     final @NotNull List<String> thirdPartyKeyValueStrings = new ArrayList<>();
     boolean shouldFreeze = false;
 
@@ -197,7 +201,7 @@ public static Baggage fromEvent(
 
   @ApiStatus.Internal
   public Baggage(final @NotNull ILogger logger) {
-    this(new HashMap<>(), null, null, null, true, false, logger);
+    this(new ConcurrentHashMap<>(), null, null, null, true, false, logger);
   }
 
   @ApiStatus.Internal
@@ -214,16 +218,14 @@ public Baggage(final @NotNull Baggage baggage) {
 
   @ApiStatus.Internal
   public Baggage(
-      final @NotNull Map<String, String> keyValues,
+      final @NotNull ConcurrentHashMap<String, String> keyValues,
       final @Nullable Double sampleRate,
       final @Nullable Double sampleRand,
       final @Nullable String thirdPartyHeader,
       final boolean isMutable,
       final boolean shouldFreeze,
       final @NotNull ILogger logger) {
-    // 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.keyValues = keyValues;
     this.sampleRate = sampleRate;
     this.sampleRand = sampleRand;
     this.logger = logger;
@@ -264,8 +266,8 @@ public String getThirdPartyHeader() {
     }
 
     final Set<String> keys;
-    synchronized (keyValues) {
-      keys = new TreeSet<>(keyValues.keySet());
+    try (final @NotNull ISentryLifecycleToken ignored = keyValuesLock.acquire()) {
+      keys = new TreeSet<>(Collections.list(keyValues.keys()));
     }
     keys.add(DSCKeys.SAMPLE_RATE);
     keys.add(DSCKeys.SAMPLE_RAND);
@@ -462,7 +464,7 @@ public void set(final @NotNull String key, final @Nullable String value) {
   @ApiStatus.Internal
   public @NotNull Map<String, Object> getUnknown() {
     final @NotNull Map<String, Object> unknown = new ConcurrentHashMap<>();
-    synchronized (keyValues) {
+    try (final @NotNull ISentryLifecycleToken ignored = keyValuesLock.acquire()) {
       for (final Map.Entry<String, String> keyValue : keyValues.entrySet()) {
         final @NotNull String key = keyValue.getKey();
         final @Nullable String value = keyValue.getValue();

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

@@ -125,7 +125,6 @@ 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) {