Fix #1195: add actual BufferRecycler reuse (#1196)

Author: cowtowncoder

src/main/java/com/fasterxml/jackson/core/JsonFactory.java

@@ -2181,12 +2181,29 @@ public RecyclerPool<BufferRecycler> _getRecyclerPool() {
      * @return I/O context created
      */
     protected IOContext _createContext(ContentReference contentRef, boolean resourceManaged) {
-        // 21-Mar-2021, tatu: Bit of defensive coding for backwards compatibility
+        BufferRecycler br = null;
+        boolean recyclerExternal = false;
+
         if (contentRef == null) {
             contentRef = ContentReference.unknown();
+        } else {
+            Object content = contentRef.getRawContent();
+            // 18-Jan-2024, tatu: [core#1195] Let's see if we can reuse already allocated recycler
+            //   (is the case when SegmentedStringWriter / ByteArrayBuilder passed)
+            if (content instanceof BufferRecycler.Gettable) {
+                br = ((BufferRecycler.Gettable) content).bufferRecycler();
+                recyclerExternal = (br != null);
+            }
         }
-        return new IOContext(_streamReadConstraints, _streamWriteConstraints, _errorReportConfiguration,
-                _getBufferRecycler(), contentRef, resourceManaged);
+        if (br == null) {
+            br = _getBufferRecycler();
+        }
+        IOContext ctxt = new IOContext(_streamReadConstraints, _streamWriteConstraints, _errorReportConfiguration,
+                br, contentRef, resourceManaged);
+        if (recyclerExternal) {
+            ctxt.markBufferRecyclerReleased();
+        }
+        return ctxt;
     }
 
     /**

src/main/java/com/fasterxml/jackson/core/io/IOContext.java

@@ -64,6 +64,16 @@ public class IOContext implements AutoCloseable
      */
     protected final BufferRecycler _bufferRecycler;
 
+    /**
+     * Flag that indicates whether this context instance should release
+     * configured {@code _bufferRecycler} or not: if it does, it needs to call
+     * (via {@link BufferRecycler#releaseToPool()} when closed; if not,
+     * should do nothing (recycler life-cycle is externally managed)
+     *
+     * @since 2.17
+     */
+    protected boolean _releaseRecycler = true;
+
     /**
      * @since 2.15
      */
@@ -194,6 +204,18 @@ public IOContext(BufferRecycler br, Object rawContent, boolean managedResource)
         this(br, ContentReference.rawReference(rawContent), managedResource);
     }
 
+    /**
+     * Method to call to prevent {@link #_bufferRecycler} release upon
+     * {@link #close()}: called when {@link #_bufferRecycler} life-cycle is
+     * externally managed.
+     *
+     * @since 2.17
+     */
+    public IOContext markBufferRecyclerReleased() {
+        _releaseRecycler = false;
+        return this;
+    }
+
     /*
     /**********************************************************************
     /* Public API, accessors
@@ -469,8 +491,11 @@ private IllegalArgumentException wrongBuf() {
     @Override
     public void close() {
         if (!_closed) {
-            _bufferRecycler.releaseToPool();
             _closed = true;
+            if (_releaseRecycler) {
+                _releaseRecycler = false;
+                _bufferRecycler.releaseToPool();
+            }
         }
     }
 }

src/main/java/com/fasterxml/jackson/core/util/BufferRecycler.java

@@ -105,7 +105,7 @@ public interface Gettable {
 
     /*
     /**********************************************************
-    /* Construction
+    /* Life-cycle
     /**********************************************************
      */
 
@@ -131,6 +131,16 @@ protected BufferRecycler(int bbCount, int cbCount) {
         _charBuffers = new AtomicReferenceArray<char[]>(cbCount);
     }
 
+    /**
+     * @return True if this recycler is linked to pool and may be released
+     *   with {@link #releaseToPool()}; false if no linkage exists.
+     *
+     * @since 2.17
+     */
+    public boolean isLinkedWithPool() {
+        return _pool != null;
+    }
+
     /*
     /**********************************************************
     /* Public API, byte buffers

src/main/java/com/fasterxml/jackson/core/util/ByteArrayBuilder.java

@@ -95,7 +95,7 @@ public int size() {
     /**
      * Clean up method to call to release all buffers this object may be
      * using. After calling the method, no other accessors can be used (and
-     * attempt to do so may result in an exception)
+     * attempt to do so may result in an exception).
      */
     public void release() {
         reset();
@@ -182,6 +182,26 @@ public byte[] toByteArray()
         return result;
     }
 
+    /**
+     * Method functionally same as calling:
+     *<pre>
+     *  byte[] bytes = toByteArray();
+     *  release();
+     *  return bytes;
+     *</pre>
+     * that is; aggregates output contained in the builder (if any),
+     * clear state; returns buffer(s) to {@link BufferRecycler} configured,
+     * if any, and returns output to caller.
+     *
+     * @since 2.17
+     */
+    public byte[] getClearAndRelease()
+    {
+        byte[] result = toByteArray();
+        release();
+        return result;
+    }
+
     /*
     /**********************************************************
     /* BufferRecycler.Gettable implementation
@@ -273,7 +293,12 @@ public void write(int b) {
         append(b);
     }
 
-    @Override public void close() { /* NOP */ }
+    @Override
+    public void close() {
+        // 18-Jan-2024, tatu: Ideally would call `release()` but currently
+        //   not possible due to existing usage
+    }
+
     @Override public void flush() { /* NOP */ }
 
     /*

src/test/java/com/fasterxml/jackson/core/io/BufferRecyclerPoolTest.java

@@ -100,8 +100,11 @@ public BufferRecycler acquirePooled() {
         }
 
         @Override
-        public void releasePooled(BufferRecycler recycler) {
-            this.bufferRecycler = recycler;
+        public void releasePooled(BufferRecycler r) {
+            if (bufferRecycler == r) { // just sanity check for this test
+                throw new IllegalStateException("BufferRecyler released more than once");
+            }
+            bufferRecycler = r;
         }
     }
 }

src/test/java/com/fasterxml/jackson/core/io/SegmentedStringWriterTest.java

@@ -1,6 +1,9 @@
 package com.fasterxml.jackson.core.io;
 
+import com.fasterxml.jackson.core.*;
+import com.fasterxml.jackson.core.base.GeneratorBase;
 import com.fasterxml.jackson.core.util.BufferRecycler;
+import com.fasterxml.jackson.core.util.JsonRecyclerPools;
 
 public class SegmentedStringWriterTest
     extends com.fasterxml.jackson.core.BaseTest
@@ -44,4 +47,36 @@ public void testSimple() throws Exception
         String act = w.getAndClear();
         assertEquals(exp.toString(), act);
     }
+
+    // [core#1195]: Try to verify that BufferRecycler instance is indeed reused
+    public void testBufferRecyclerReuse() throws Exception
+    {
+        JsonFactory f = new JsonFactory();
+        BufferRecycler br = new BufferRecycler()
+                // need to link with some pool
+                .withPool(JsonRecyclerPools.newBoundedPool(3));
+
+        SegmentedStringWriter ssw = new SegmentedStringWriter(br);
+        assertSame(br, ssw.bufferRecycler());
+
+        JsonGenerator g = f.createGenerator(ssw);
+        IOContext ioCtxt = ((GeneratorBase) g).ioContext();
+        assertSame(br, ioCtxt.bufferRecycler());
+        assertTrue(ioCtxt.bufferRecycler().isLinkedWithPool());
+
+        g.writeStartArray();
+        g.writeEndArray();
+        g.close();
+
+        // Generator.close() should NOT release buffer recycler
+        assertTrue(br.isLinkedWithPool());
+
+        // Nor accessing contents
+        assertEquals("[]", ssw.getAndClear());
+        assertTrue(br.isLinkedWithPool());
+
+        // only explicit release does
+        br.releaseToPool();
+        assertFalse(br.isLinkedWithPool());
+    }
 }

src/test/java/com/fasterxml/jackson/core/util/ByteArrayBuilderTest.java

@@ -1,7 +1,14 @@
 package com.fasterxml.jackson.core.util;
 
+import java.nio.charset.StandardCharsets;
+
 import org.junit.Assert;
 
+import com.fasterxml.jackson.core.JsonFactory;
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.core.base.GeneratorBase;
+import com.fasterxml.jackson.core.io.IOContext;
+
 public class ByteArrayBuilderTest extends com.fasterxml.jackson.core.BaseTest
 {
     public void testSimple() throws Exception
@@ -27,4 +34,37 @@ public void testSimple() throws Exception
         b.release();
         b.close();
     }
+
+    // [core#1195]: Try to verify that BufferRecycler instance is indeed reused
+    public void testBufferRecyclerReuse() throws Exception
+    {
+        JsonFactory f = new JsonFactory();
+        BufferRecycler br = new BufferRecycler()
+                // need to link with some pool
+                .withPool(JsonRecyclerPools.newBoundedPool(3));
+
+        ByteArrayBuilder bab = new ByteArrayBuilder(br, 20);
+        assertSame(br, bab.bufferRecycler());
+
+        JsonGenerator g = f.createGenerator(bab);
+        IOContext ioCtxt = ((GeneratorBase) g).ioContext();
+        assertSame(br, ioCtxt.bufferRecycler());
+        assertTrue(ioCtxt.bufferRecycler().isLinkedWithPool());
+
+        g.writeStartArray();
+        g.writeEndArray();
+        g.close();
+
+        // Generator.close() should NOT release buffer recycler
+        assertTrue(br.isLinkedWithPool());
+
+        byte[] result = bab.getClearAndRelease();
+        assertEquals("[]", new String(result, StandardCharsets.UTF_8));
+        // Nor accessing contents
+        assertTrue(br.isLinkedWithPool());
+
+        // only explicit release does
+        br.releaseToPool();
+        assertFalse(br.isLinkedWithPool());
+    }
 }