ensure all buffers get released again in JsonStreamParserImpl

Author: jungm

johnzon-core/src/main/java/org/apache/johnzon/core/JsonStreamParserImpl.java

@@ -74,7 +74,7 @@ public class JsonStreamParserImpl extends JohnzonJsonParserImpl implements JsonC
     //this buffer is used to store current String or Number value in case that
     //within the value a buffer boundary is crossed or the string contains escaped characters
     private char[] fallBackCopyBuffer;
-    private boolean releaseFallBackCopyBufferLength = true;
+    private boolean releaseFallBackCopyBuffer = true;
     private int fallBackCopyBufferLength;
     // when boundaries of fallBackCopyBuffer have been reached
     private List<Buffer> previousFallBackCopyBuffers;
@@ -936,6 +936,11 @@ private void combinePreviousFallbackBuffersToCurrent() {
         index += fallBackCopyBufferLength;
 
         releasePreviousFallBackCopyBuffers();
+        if (releaseFallBackCopyBuffer) {
+            valueProvider.release(fallBackCopyBuffer);
+            releaseFallBackCopyBuffer = false;
+        }
+
         fallBackCopyBuffer = newBuffer;
         fallBackCopyBufferLength = index;
     }
@@ -1038,7 +1043,7 @@ public void close() {
         }
 
         bufferProvider.release(buffer);
-        if (releaseFallBackCopyBufferLength) {
+        if (releaseFallBackCopyBuffer) {
             valueProvider.release(fallBackCopyBuffer);
         }
         releasePreviousFallBackCopyBuffers();

johnzon-core/src/test/java/org/apache/johnzon/core/HugeStringTest.java

@@ -18,18 +18,26 @@
  */
 package org.apache.johnzon.core;
 
+import org.junit.Ignore;
 import org.junit.Test;
 
+import javax.json.Json;
+import javax.json.JsonReader;
+import javax.json.JsonReaderFactory;
+import javax.json.spi.JsonProvider;
 import javax.json.stream.JsonParser;
 import java.io.ByteArrayInputStream;
 import java.io.IOException;
+import java.io.StringReader;
 import java.nio.charset.StandardCharsets;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.List;
 
 import static java.util.Arrays.asList;
 import static java.util.Collections.emptyMap;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
 
 public class JsonStreamParserImplTest {
     @Test
@@ -63,4 +71,80 @@ public void ensureNoArrayBoundErrorWhenOverflow() throws IOException {
                 asList("START_OBJECT", "KEY_NAME", "VALUE_STRING", "{\"foo\":\"barbar\\barbarbar\"}", "END_OBJECT"),
                 events);
     }
+
+    @Test
+    @Ignore("No real test, just run directly from your IDE")
+    public void largeStringPerformance() {
+        StringBuilder jsonBuilder = new StringBuilder("{\"data\":\"");
+        for (int i = 0; i < 50 * 1024 * 1024 + 1; i++) {
+            jsonBuilder.append("a");
+        }
+        jsonBuilder.append("\"}");
+        String json = jsonBuilder.toString();
+
+        // Warmup
+        for (int i = 0; i < 10; i++) {
+            try (JsonReader reader = Json.createReader(new StringReader(json))) {
+                reader.readObject();
+            }
+        }
+
+        long start = System.currentTimeMillis();
+        try (JsonReader reader = Json.createReader(new StringReader(json))) {
+            reader.readObject();
+        }
+        System.err.println("Took " + (System.currentTimeMillis() - start) + "ms");
+    }
+
+    @Test
+    public void allBuffersReleased() {
+        StringBuilder jsonBuilder = new StringBuilder("{\"data\":\"");
+        for (int i = 0; i < JsonParserFactoryImpl.DEFAULT_MAX_STRING_LENGTH * 2; i++) {
+            jsonBuilder.append("a");
+        }
+        jsonBuilder.append("\"}");
+        String json = jsonBuilder.toString();
+
+        JsonReaderFactory readerFactory = JsonProvider.provider().createReaderFactory(Collections.singletonMap(
+                JsonParserFactoryImpl.BUFFER_STRATEGY, TrackingBufferStrategy.class.getName()));
+
+        try (JsonReader reader = readerFactory.createReader(new StringReader(json))) {
+            reader.readObject();
+        }
+
+        assertTrue(TrackingBufferStrategy.TrackingBufferProvider.borrowed.isEmpty());
+    }
+
+    public static class TrackingBufferStrategy implements BufferStrategy {
+        private final BufferStrategy delegate = BufferStrategyFactory.valueOf("BY_INSTANCE");
+
+        @Override
+        public BufferProvider<char[]> newCharProvider(int size) {
+            return new TrackingBufferProvider(delegate.newCharProvider(size));
+        }
+
+        public static class TrackingBufferProvider implements BufferStrategy.BufferProvider<char[]> {
+            protected static List<char[]> borrowed = new ArrayList<>();
+
+            private final BufferStrategy.BufferProvider<char[]> delegate;
+
+            public TrackingBufferProvider(BufferStrategy.BufferProvider<char[]> delegate) {
+                this.delegate = delegate;
+            }
+
+            @Override
+            public char[] newBuffer() {
+                char[] result = delegate.newBuffer();
+                borrowed.add(result);
+
+                return result;
+            }
+
+            @Override
+            public void release(char[] value) {
+                borrowed.remove(value);
+                delegate.release(value);
+            }
+        }
+    }
 }