fix: thread-unsafe date parsing (#130)

* fix: thread-unsafe date parsing

* bump version

Author: typotter

README.md

@@ -13,7 +13,7 @@ or [JVM](https://github.com/Eppo-exp/java-server-sdk) SDKs.
 
 ```groovy
 dependencies {
-  implementation 'cloud.eppo:sdk-common-jvm:3.10.0'
+  implementation 'cloud.eppo:sdk-common-jvm:3.11.3'
 }
 ```
 

build.gradle

@@ -6,7 +6,7 @@ plugins {
 }
 
 group = 'cloud.eppo'
-version = '3.11.2'
+version = '3.11.3'
 ext.isReleaseVersion = !version.endsWith("SNAPSHOT")
 
 java {

src/main/java/cloud/eppo/Utils.java

@@ -13,7 +13,7 @@
 import org.slf4j.LoggerFactory;
 
 public final class Utils {
-  private static final SimpleDateFormat UTC_ISO_DATE_FORMAT = buildUtcIsoDateFormat();
+  private static final ThreadLocal<SimpleDateFormat> UTC_ISO_DATE_FORMAT = buildUtcIsoDateFormat();
   private static final Logger log = LoggerFactory.getLogger(Utils.class);
   private static final ThreadLocal<MessageDigest> md = buildMd5MessageDigest();
 
@@ -31,6 +31,21 @@ protected MessageDigest initialValue() {
     };
   }
 
+  @SuppressWarnings("AnonymousHasLambdaAlternative")
+  private static ThreadLocal<SimpleDateFormat> buildUtcIsoDateFormat() {
+    return new ThreadLocal<SimpleDateFormat>() {
+      @Override
+      protected SimpleDateFormat initialValue() {
+        // Note: we don't use DateTimeFormatter.ISO_DATE so that this supports older Android
+        // versions
+        SimpleDateFormat dateFormat =
+            new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'", Locale.US);
+        dateFormat.setTimeZone(java.util.TimeZone.getTimeZone("UTC"));
+        return dateFormat;
+      }
+    };
+  }
+
   public static void throwIfEmptyOrNull(String input, String errorMessage) {
     if (input == null || input.isEmpty()) {
       throw new IllegalArgumentException(errorMessage);
@@ -84,7 +99,7 @@ public static Date parseUtcISODateNode(JsonNode isoDateStringElement) {
     String isoDateString = isoDateStringElement.asText();
     Date result = null;
     try {
-      result = UTC_ISO_DATE_FORMAT.parse(isoDateString);
+      result = UTC_ISO_DATE_FORMAT.get().parse(isoDateString);
     } catch (ParseException e) {
       // We expect to fail parsing if the date is base 64 encoded
       // Thus we'll leave the result null for now and try again with the decoded value
@@ -94,7 +109,7 @@ public static Date parseUtcISODateNode(JsonNode isoDateStringElement) {
       // Date may be encoded
       String decodedIsoDateString = base64Decode(isoDateString);
       try {
-        result = UTC_ISO_DATE_FORMAT.parse(decodedIsoDateString);
+        result = UTC_ISO_DATE_FORMAT.get().parse(decodedIsoDateString);
       } catch (ParseException e) {
         log.warn("Date \"{}\" not in ISO date format", isoDateString);
       }
@@ -104,7 +119,7 @@ public static Date parseUtcISODateNode(JsonNode isoDateStringElement) {
   }
 
   public static String getISODate(Date date) {
-    return UTC_ISO_DATE_FORMAT.format(date);
+    return UTC_ISO_DATE_FORMAT.get().format(date);
   }
 
   public static String base64Encode(String input) {
@@ -125,11 +140,4 @@ public static String base64Decode(String input) {
     }
     return new String(decodedBytes);
   }
-
-  private static SimpleDateFormat buildUtcIsoDateFormat() {
-    // Note: we don't use DateTimeFormatter.ISO_DATE so that this supports older Android versions
-    SimpleDateFormat dateFormat = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'", Locale.US);
-    dateFormat.setTimeZone(java.util.TimeZone.getTimeZone("UTC"));
-    return dateFormat;
-  }
 }

src/test/java/cloud/eppo/UtilsTest.java

@@ -7,10 +7,12 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import java.util.Date;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 import org.junit.jupiter.api.Test;
 
 public class UtilsTest {
@@ -90,4 +92,90 @@ public void testParseUtcISODateNode() throws JsonProcessingException {
     assertNull(parsedDate);
     assertNull(parseUtcISODateNode(null));
   }
+
+  @Test
+  public void testDateParsingThreadSafety() throws InterruptedException {
+    final AtomicBoolean collisionDetected = new AtomicBoolean(false);
+    final AtomicInteger unexpectedExceptions = new AtomicInteger(0);
+    final AtomicInteger incorrectParseResults = new AtomicInteger(0);
+
+    int numThreads = 20; // Spawn 20 threads
+    int iterationsPerThread = 100; // Each thread will parse 100 dates
+    ExecutorService pool = Executors.newFixedThreadPool(numThreads);
+
+    CountDownLatch startLatch = new CountDownLatch(1);
+    CountDownLatch finishLatch = new CountDownLatch(numThreads);
+
+    // Expected date: 2024-05-01T16:13:26.651Z -> 1714580006651L
+    final String testDateString = "\"2024-05-01T16:13:26.651Z\"";
+    final long expectedTimestamp = 1714580006651L;
+
+    try {
+      for (int i = 0; i < numThreads; i++) {
+        pool.execute(
+            () -> {
+              try {
+                // Wait for all threads to start simultaneously
+                startLatch.await();
+
+                ObjectMapper mapper = new ObjectMapper();
+
+                for (int j = 0; j < iterationsPerThread; j++) {
+                  try {
+                    JsonNode jsonNode = mapper.readTree(testDateString);
+                    Date parsedDate = parseUtcISODateNode(jsonNode);
+
+                    if (parsedDate == null || parsedDate.getTime() != expectedTimestamp) {
+                      incorrectParseResults.incrementAndGet();
+                      collisionDetected.set(true);
+                    }
+
+                    // Also test the reverse operation
+                    Date originalDate = new Date(expectedTimestamp);
+                    String formattedDate = getISODate(originalDate);
+                    if (!formattedDate.equals("2024-05-01T16:13:26.651Z")) {
+                      incorrectParseResults.incrementAndGet();
+                      collisionDetected.set(true);
+                    }
+
+                  } catch (Exception e) {
+                    unexpectedExceptions.incrementAndGet();
+                  }
+                }
+              } catch (InterruptedException e) {
+                Thread.currentThread().interrupt();
+              } finally {
+                finishLatch.countDown();
+              }
+            });
+      }
+
+      // Start all threads simultaneously to maximize contention
+      startLatch.countDown();
+
+      // Wait for all threads to complete
+      assertTrue(
+          finishLatch.await(30, TimeUnit.SECONDS), "Test threads did not complete within timeout");
+
+    } finally {
+      pool.shutdown();
+      assertTrue(
+          pool.awaitTermination(5, TimeUnit.SECONDS),
+          "Thread pool did not shutdown within timeout");
+    }
+
+    // Print diagnostic information
+    System.out.println("Unexpected exceptions: " + unexpectedExceptions.get());
+    System.out.println("Incorrect parse results: " + incorrectParseResults.get());
+    System.out.println("Total operations: " + (numThreads * iterationsPerThread * 2));
+
+    String failureMessage =
+        "SimpleDateFormat thread-safety issue detected! "
+            + "Exceptions: "
+            + unexpectedExceptions.get()
+            + ", Incorrect results: "
+            + incorrectParseResults.get();
+    assertFalse(collisionDetected.get(), failureMessage);
+    assertEquals(0, unexpectedExceptions.get(), failureMessage);
+  }
 }