Merge branch 'master' into empty-bais

src/com/amazon/ion/impl/IonCursorBinary.java

@@ -527,7 +527,7 @@ private boolean fillAt(long index, long numberOfBytes) {
             refillableState.bytesRequested = numberOfBytes + (index - offset);
             if (ensureCapacity(refillableState.bytesRequested)) {
                 // Fill all the free space, not just the shortfall; this reduces I/O.
-                shortfall = refill(freeSpaceAt(limit), refillableState.bytesRequested);
+                shortfall = refill(refillableState.bytesRequested);
             } else {
                 // The request cannot be satisfied, but not because data was unavailable. Return normally; it is the
                 // caller's responsibility to recover.
@@ -650,23 +650,23 @@ private void shiftIndicesLeft(int shiftAmount) {
     /**
      * Attempts to fill the buffer with up to the requested number of additional bytes. It is the caller's
      * responsibility to ensure that there is space in the buffer.
-     * @param numberOfBytesToFill the number of additional bytes to attempt to add to the buffer.
      * @param minimumNumberOfBytesRequired the minimum number of bytes requested to fill the current value.
      * @return the shortfall between the number of bytes that were filled and the minimum number requested. If less than
      *  1, then at least `minimumNumberOfBytesRequired` were filled.
      */
-    private long refill(long numberOfBytesToFill, long minimumNumberOfBytesRequired) {
+    private long refill(long minimumNumberOfBytesRequired) {
         int numberOfBytesFilled = -1;
         long shortfall;
         // Sometimes an InputStream implementation will return fewer than the number of bytes requested even
         // if the stream is not at EOF. If this happens and there is still a shortfall, keep requesting bytes
         // until either the shortfall is filled or EOF is reached.
         do {
             try {
-                numberOfBytesFilled = refillableState.inputStream.read(buffer, (int) limit, (int) numberOfBytesToFill);
+                numberOfBytesFilled = refillableState.inputStream.read(buffer, (int) limit, (int) freeSpaceAt(limit));
             } catch (EOFException e) {
                 // Certain InputStream implementations (e.g. GZIPInputStream) throw EOFException if more bytes are requested
                 // to read than are currently available (e.g. if a header or trailer is incomplete).
+                numberOfBytesFilled = -1;
             } catch (IOException e) {
                 throwAsIonException(e);
             }

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);

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
      */

test/com/amazon/ion/impl/IonReaderContinuableTopLevelBinaryTest.java

@@ -34,10 +34,12 @@
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.CsvSource;
 import org.junit.jupiter.params.provider.ValueSource;
 
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
+import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.math.BigDecimal;
@@ -3202,10 +3204,17 @@ public void skipWithoutEnoughDataNonIncrementalFails() throws Exception {
     private static class ThrottlingInputStream extends InputStream {
 
         private final byte[] data;
+        private final boolean throwFromReadOnEof;
         private int offset = 0;
 
-        protected ThrottlingInputStream(byte[] data) {
+        /**
+         * @param data the data for the InputStream to provide.
+         * @param throwFromReadOnEof true if the stream should throw {@link java.io.EOFException} when read() is called
+         *                           at EOF. If false, simply returns -1.
+         */
+        protected ThrottlingInputStream(byte[] data, boolean throwFromReadOnEof) {
             this.data = data;
+            this.throwFromReadOnEof = throwFromReadOnEof;
         }
 
         @Override
@@ -3229,9 +3238,15 @@ private int calculateNumberOfBytesToReturn(int numberOfBytesRequested) {
         }
 
         @Override
-        public int read(byte[] b, int off, int len) {
+        public int read(byte[] b, int off, int len) throws IOException {
+            if (off + len > b.length) {
+                throw new IndexOutOfBoundsException();
+            }
             int numberOfBytesToReturn = calculateNumberOfBytesToReturn(len);
             if (numberOfBytesToReturn < 0) {
+                if (throwFromReadOnEof) {
+                    throw new EOFException();
+                }
                 return -1;
             }
             System.arraycopy(data, offset, b, off, numberOfBytesToReturn);
@@ -3247,12 +3262,17 @@ public long skip(long len) {
         }
     }
 
-    @ParameterizedTest(name = "incrementalReadingEnabled={0}")
-    @ValueSource(booleans = {true, false})
-    public void shouldNotFailWhenAnInputStreamProvidesFewerBytesThanRequestedWithoutReachingEof(boolean incrementalReadingEnabled) throws Exception {
+    @ParameterizedTest(name = "incrementalReadingEnabled={0},throwOnEof={1}")
+    @CsvSource({
+        "true, true",
+        "true, false",
+        "false, true",
+        "false, false"
+    })
+    public void shouldNotFailWhenAnInputStreamProvidesFewerBytesThanRequestedWithoutReachingEof(boolean incrementalReadingEnabled, boolean throwOnEof) throws Exception {
         readerBuilder = readerBuilder.withIncrementalReadingEnabled(incrementalReadingEnabled)
             .withBufferConfiguration(IonBufferConfiguration.Builder.standard().withInitialBufferSize(8).build());
-        reader = readerFor(new ThrottlingInputStream(bytes(0xE0, 0x01, 0x00, 0xEA, 0x89, 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i')));
+        reader = readerFor(new ThrottlingInputStream(bytes(0xE0, 0x01, 0x00, 0xEA, 0x89, 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i'), throwOnEof));
         assertSequence(
             next(IonType.STRING), stringValue("abcdefghi"),
             next(null)
@@ -3262,7 +3282,7 @@ public void shouldNotFailWhenAnInputStreamProvidesFewerBytesThanRequestedWithout
 
     @Test
     public void shouldNotFailWhenAnInputStreamProvidesFewerBytesThanRequestedWithoutReachingEofAndTheReaderSkipsTheValue() throws Exception {
-        reader = boundedReaderFor(new ThrottlingInputStream(bytes(0xE0, 0x01, 0x00, 0xEA, 0x89, 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 0x20)), 8, 8, byteAndOversizedValueCountingHandler);
+        reader = boundedReaderFor(new ThrottlingInputStream(bytes(0xE0, 0x01, 0x00, 0xEA, 0x89, 'a', 'b', 'c', 'd', 'e', 'f', 'g', 'h', 'i', 0x20), false), 8, 8, byteAndOversizedValueCountingHandler);
         assertSequence(
             next(IonType.INT), intValue(0),
             next(null)