From 54b93c4a9c49020ee44c67d8b7f5e38d55f220fc Mon Sep 17 00:00:00 2001
From: Tyler Gregg <greggt@amazon.com>
Date: Wed, 1 Nov 2023 12:25:54 -0700
Subject: [PATCH] Fixes a bug that caused concurrent field access of cloned,
 read-only structs with more than 5 members to be non-deterministic.

---
 .../amazon/ion/impl/lite/IonStructLite.java   |  8 +++++
 test/com/amazon/ion/CloneTest.java            | 29 +++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/src/com/amazon/ion/impl/lite/IonStructLite.java b/src/com/amazon/ion/impl/lite/IonStructLite.java
index ed32ef6a45..c6ea1fca9a 100644
--- a/src/com/amazon/ion/impl/lite/IonStructLite.java
+++ b/src/com/amazon/ion/impl/lite/IonStructLite.java
@@ -101,6 +101,14 @@ private void build_field_map()
             _field_map.put(v._fieldName, ii); // this causes the map to have the largest index value stored
         }
     }
+
+    @Override
+    public void makeReadOnly() {
+        // Eagerly initialize the fields map to prevent potential data races https://github.com/amazon-ion/ion-java/issues/629
+        fieldMapIsActive(_child_count);
+        super.makeReadOnly();
+    }
+
     private void add_field(String fieldName, int newFieldIdx)
     {
         Integer idx = _field_map.get(fieldName);
diff --git a/test/com/amazon/ion/CloneTest.java b/test/com/amazon/ion/CloneTest.java
index 7dfe5df275..9950de5a7e 100644
--- a/test/com/amazon/ion/CloneTest.java
+++ b/test/com/amazon/ion/CloneTest.java
@@ -20,6 +20,10 @@
 import com.amazon.ion.system.IonSystemBuilder;
 import org.junit.jupiter.api.Test;
 
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+
 import static com.amazon.ion.impl._Private_Utils.newSymbolToken;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.MatcherAssert.assertThat;
@@ -28,6 +32,7 @@
 import static org.hamcrest.Matchers.sameInstance;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 
 public class CloneTest
@@ -261,6 +266,30 @@ public void modifyAfterCloneDoesNotChangeOriginal() {
         assertNotEquals(original, clone);
     }
 
+    @Test
+    public void readOnlyIonStructMultithreadedTest() {
+        // See: https://github.com/amazon-ion/ion-java/issues/629
+        String ionStr = "{a:1,b:2,c:3,d:4,e:5,f:6}";
+
+        IonStruct ionValue = (IonStruct)system().singleValue(ionStr);
+        ionValue.makeReadOnly();
+
+        for (int i=0; i<100; i++) {
+            IonStruct clone = ionValue.clone();
+            clone.makeReadOnly();
+
+            List<CompletableFuture<Void>> waiting = new ArrayList<>();
+            for (int j = 0; j < 4; j++) {
+                waiting.add(CompletableFuture.runAsync(() -> {
+                    for (int k = 0; k <= 100; k++) {
+                        assertNotNull(clone.get("a"));
+                    }
+                }));
+            }
+            waiting.forEach(CompletableFuture::join);
+        }
+    }
+
     /**
      * @return the singleton IonSystem
      */