diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java index 45c41eda4977..001cb9abcc10 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/GZIPContentDecoder.java @@ -109,7 +109,7 @@ public RetainableByteBuffer decode(ByteBuffer compressed) RetainableByteBuffer result = acquire(length); for (RetainableByteBuffer buffer : _inflateds) { - BufferUtil.append(result.getByteBuffer(), buffer.getByteBuffer()); + result.append(buffer); buffer.release(); } _inflateds.clear(); diff --git a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java index da534a781aaa..9a86f5623eae 100644 --- a/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java +++ b/jetty-core/jetty-http/src/main/java/org/eclipse/jetty/http/MultiPart.java @@ -1114,7 +1114,7 @@ else if (type != HttpTokens.Type.SPACE && type != HttpTokens.Type.HTAB) if (state == State.EPILOGUE) notifyComplete(); else - throw new EOFException("unexpected EOF"); + throw new EOFException("unexpected EOF in " + state); } } catch (Throwable x) diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java index d3425e341dcd..ac383bec0bb8 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Connection.java @@ -478,6 +478,12 @@ public boolean canRetain() return retainable.canRetain(); } + @Override + public boolean isRetained() + { + return retainable.isRetained(); + } + @Override public void retain() { @@ -541,7 +547,7 @@ public boolean release() private void put(ByteBuffer source) { - BufferUtil.append(delegate.getByteBuffer(), source); + delegate.append(source); } } } diff --git a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java index dc3ecf7d5275..ccc5892f49a2 100644 --- a/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java +++ b/jetty-core/jetty-http2/jetty-http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java @@ -438,7 +438,7 @@ public default void onClosed(Stream stream) /** *

A {@link Retainable} wrapper of a {@link DataFrame}.

*/ - public abstract static class Data implements Retainable + abstract class Data implements Retainable { public static Data eof(int streamId) { diff --git a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/RawHTTP2ProxyTest.java b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/RawHTTP2ProxyTest.java index 711e705ba55d..c58570c1459b 100644 --- a/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/RawHTTP2ProxyTest.java +++ b/jetty-core/jetty-http2/jetty-http2-tests/src/test/java/org/eclipse/jetty/http2/tests/RawHTTP2ProxyTest.java @@ -44,7 +44,6 @@ import org.eclipse.jetty.http2.frames.ResetFrame; import org.eclipse.jetty.http2.server.RawHTTP2ServerConnectionFactory; import org.eclipse.jetty.io.ArrayByteBufferPool; -import org.eclipse.jetty.io.ByteBufferAggregator; import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.Server; @@ -63,8 +62,6 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertEquals; -import static org.junit.jupiter.api.Assertions.assertFalse; -import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; public class RawHTTP2ProxyTest @@ -245,7 +242,7 @@ public void onDataAvailable(Stream stream) CountDownLatch latch1 = new CountDownLatch(1); Stream stream1 = clientSession.newStream(new HeadersFrame(request1, null, false), new Stream.Listener() { - private final ByteBufferAggregator aggregator = new ByteBufferAggregator(client.getByteBufferPool(), true, data1.length, data1.length * 2); + private final RetainableByteBuffer.Aggregator aggregator = new RetainableByteBuffer.Aggregator(client.getByteBufferPool(), true, data1.length * 2); @Override public void onHeaders(Stream stream, HeadersFrame frame) @@ -262,17 +259,15 @@ public void onDataAvailable(Stream stream) DataFrame frame = data.frame(); if (LOGGER.isDebugEnabled()) LOGGER.debug("CLIENT1 received {}", frame); - assertFalse(aggregator.aggregate(frame.getByteBuffer())); + assertTrue(aggregator.append(frame.getByteBuffer())); data.release(); if (!data.frame().isEndStream()) { stream.demand(); return; } - RetainableByteBuffer buffer = aggregator.takeRetainableByteBuffer(); - assertNotNull(buffer); - assertEquals(buffer1.slice(), buffer.getByteBuffer()); - buffer.release(); + assertEquals(buffer1.slice(), aggregator.getByteBuffer()); + aggregator.release(); latch1.countDown(); } }).get(5, TimeUnit.SECONDS); diff --git a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/HTTP3StreamConnection.java b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/HTTP3StreamConnection.java index 29542da3d1eb..d311d2b6f038 100644 --- a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/HTTP3StreamConnection.java +++ b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/HTTP3StreamConnection.java @@ -445,6 +445,12 @@ public boolean canRetain() return retainable.canRetain(); } + @Override + public boolean isRetained() + { + return retainable.isRetained(); + } + @Override public void retain() { diff --git a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/api/Stream.java b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/api/Stream.java index d2e83791404a..9953e2ea04e8 100644 --- a/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/api/Stream.java +++ b/jetty-core/jetty-http3/jetty-http3-common/src/main/java/org/eclipse/jetty/http3/api/Stream.java @@ -384,7 +384,7 @@ public default void onFailure(Stream.Server stream, long error, Throwable failur * * @see Stream#readData() */ - public abstract static class Data implements Retainable + abstract class Data implements Retainable { public static final Data EOF = new EOFData(); diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java index 0fab64e92ae3..71aeda393a56 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ArrayByteBufferPool.java @@ -666,9 +666,19 @@ public Tracking(int minCapacity, int maxCapacity, int maxBucketSize) this(minCapacity, maxCapacity, maxBucketSize, -1L, -1L); } + public Tracking(int minCapacity, int factor, int maxCapacity, int maxBucketSize) + { + this(minCapacity, factor, maxCapacity, maxBucketSize, 0L, 0L); + } + public Tracking(int minCapacity, int maxCapacity, int maxBucketSize, long maxHeapMemory, long maxDirectMemory) { - super(minCapacity, -1, maxCapacity, maxBucketSize, maxHeapMemory, maxDirectMemory); + this(minCapacity, -1, maxCapacity, maxBucketSize, maxHeapMemory, maxDirectMemory); + } + + public Tracking(int minCapacity, int factor, int maxCapacity, int maxBucketSize, long maxHeapMemory, long maxDirectMemory) + { + super(minCapacity, factor, maxCapacity, maxBucketSize, maxHeapMemory, maxDirectMemory); } @Override diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAccumulator.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAccumulator.java index 2096be6e91bb..8bfdfe5a15ba 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAccumulator.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAccumulator.java @@ -28,9 +28,9 @@ * * The method {@link #ensureBuffer(int, int)} is used to write directly to the last buffer stored in the buffer list, * if there is less than a certain amount of space available in that buffer then a new one will be allocated and returned instead. - * @see #ensureBuffer(int, int) + * @deprecated use {@link RetainableByteBuffer.Accumulator} */ -// TODO: rename to *Aggregator to avoid confusion with RBBP.Accumulator? +@Deprecated(forRemoval = true) public class ByteBufferAccumulator implements AutoCloseable { private final List _buffers = new ArrayList<>(); diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAggregator.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAggregator.java index 1dd074703e25..7914c6792128 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAggregator.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferAggregator.java @@ -26,7 +26,9 @@ * Once the buffer is full, the aggregator will not aggregate any more bytes until its buffer is taken out, * after which a new aggregate/take buffer cycle can start.

*

The buffers are taken from the supplied {@link ByteBufferPool} or freshly allocated if one is not supplied.

+ * @deprecated Use {@link ByteBufferAccumulator} */ +@Deprecated public class ByteBufferAggregator { private static final Logger LOG = LoggerFactory.getLogger(ByteBufferAggregator.class); diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferCallbackAccumulator.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferCallbackAccumulator.java index 04df7e921ebe..4d24091dea2c 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferCallbackAccumulator.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferCallbackAccumulator.java @@ -25,7 +25,9 @@ * these into a single {@link ByteBuffer} or byte array and succeed the callbacks.

* *

This class is not thread safe and callers must do mutual exclusion.

+ * @deprecated use {@link ByteBufferAccumulator} */ +@Deprecated public class ByteBufferCallbackAccumulator { private final List _entries = new ArrayList<>(); diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java index f9d6965c2bc6..a46ea1f23dc0 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ByteBufferPool.java @@ -127,12 +127,15 @@ private Buffer(ByteBuffer byteBuffer) /** *

Accumulates a sequence of {@link RetainableByteBuffer} that - * are typically created during the generation of protocol bytes.

+ * are typically created during the generation of protocol bytes. + * The accumulated buffers are then used individually rather than + * as a single buffer (like {@link RetainableByteBuffer.Accumulator}.

*

{@code RetainableByteBuffer}s can be either * {@link #append(RetainableByteBuffer) appended} to the sequence, * or {@link #insert(int, RetainableByteBuffer) inserted} at a * specific position in the sequence, and then * {@link #release() released} when they are consumed.

+ * @see RetainableByteBuffer.Accumulator */ class Accumulator { diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java index 0358f8583969..7adad1f6bc1e 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ChunkAccumulator.java @@ -27,7 +27,9 @@ /** * An accumulator of {@link Content.Chunk}s used to facilitate minimal copy * aggregation of multiple chunks. + * @deprecated use {@link Content.Source#asRetainableByteBuffer(Content.Source, ByteBufferPool, boolean, int)} instead. */ +@Deprecated public class ChunkAccumulator { private static final ByteBufferPool NON_POOLING = new ByteBufferPool.NonPooling(); @@ -109,7 +111,7 @@ public RetainableByteBuffer take(ByteBufferPool pool, boolean direct) for (Chunk chunk : _chunks) { offset += chunk.remaining(); - BufferUtil.append(buffer.getByteBuffer(), chunk.getByteBuffer()); + buffer.append(chunk); chunk.release(); } assert offset == _length; diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java index 0a6fe5921c98..7e4dc5507d1f 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Content.java @@ -13,6 +13,7 @@ package org.eclipse.jetty.io; +import java.io.EOFException; import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; @@ -33,6 +34,7 @@ import org.eclipse.jetty.io.internal.ContentCopier; import org.eclipse.jetty.io.internal.ContentSourceByteBuffer; import org.eclipse.jetty.io.internal.ContentSourceConsumer; +import org.eclipse.jetty.io.internal.ContentSourceRetainableByteBuffer; import org.eclipse.jetty.io.internal.ContentSourceString; import org.eclipse.jetty.util.Blocker; import org.eclipse.jetty.util.BufferUtil; @@ -192,7 +194,7 @@ static ByteBuffer asByteBuffer(Source source) throws IOException */ static CompletableFuture asByteArrayAsync(Source source, int maxSize) { - return new ChunkAccumulator().readAll(source, maxSize); + return asRetainableByteBuffer(source, null, false, maxSize).thenApply(rbb -> rbb.getByteBuffer().array()); } /** @@ -230,7 +232,31 @@ static CompletableFuture asByteBufferAsync(Source source, int maxSiz */ static CompletableFuture asRetainableByteBuffer(Source source, ByteBufferPool pool, boolean direct, int maxSize) { - return new ChunkAccumulator().readAll(source, pool, direct, maxSize); + Promise.Completable promise = new Promise.Completable<>() + { + @Override + public void succeeded(RetainableByteBuffer result) + { + result.retain(); + super.succeeded(result); + } + }; + asRetainableByteBuffer(source, pool, direct, maxSize, promise); + return promise; + } + + /** + *

Reads, non-blocking, the whole content source into a {@link RetainableByteBuffer}.

+ * + * @param source the source to read + * @param pool The {@link ByteBufferPool} to acquire the buffer from, or null for a non {@link Retainable} buffer + * @param direct True if the buffer should be direct. + * @param maxSize The maximum size to read, or -1 for no limit + * @param promise the promise to notify when the whole content has been read into a RetainableByteBuffer. + */ + static void asRetainableByteBuffer(Source source, ByteBufferPool pool, boolean direct, int maxSize, Promise promise) + { + new ContentSourceRetainableByteBuffer(source, pool, direct, maxSize, promise).run(); } /** @@ -470,6 +496,43 @@ default boolean rewind() */ public interface Sink { + /** + *

Wraps the given {@link OutputStream} as a {@link Sink}. + * @param out The stream to wrap + * @return A sink wrapping the stream + */ + static Sink from(OutputStream out) + { + return new Sink() + { + boolean closed; + + @Override + public void write(boolean last, ByteBuffer byteBuffer, Callback callback) + { + if (closed) + { + callback.failed(new EOFException()); + return; + } + try + { + BufferUtil.writeTo(byteBuffer, out); + if (last) + { + closed = true; + out.close(); + } + callback.succeeded(); + } + catch (Throwable t) + { + callback.failed(t); + } + } + }; + } + /** *

Wraps the given content sink with a buffering sink.

* @@ -561,7 +624,7 @@ static void write(Sink sink, boolean last, String utf8Content, Callback callback * to release the {@code ByteBuffer} back into a pool), or the * {@link #release()} method overridden.

*/ - public interface Chunk extends Retainable + public interface Chunk extends RetainableByteBuffer { /** *

An empty, non-last, chunk.

@@ -804,11 +867,6 @@ static boolean isFailure(Chunk chunk, boolean last) return chunk != null && chunk.getFailure() != null && chunk.isLast() == last; } - /** - * @return the ByteBuffer of this Chunk - */ - ByteBuffer getByteBuffer(); - /** * Get a failure (which may be from a {@link Source#fail(Throwable) failure} or * a {@link Source#fail(Throwable, boolean) warning}), if any, associated with the chunk. @@ -831,68 +889,6 @@ default Throwable getFailure() */ boolean isLast(); - /** - * @return the number of bytes remaining in this Chunk - */ - default int remaining() - { - return getByteBuffer().remaining(); - } - - /** - * @return whether this Chunk has remaining bytes - */ - default boolean hasRemaining() - { - return getByteBuffer().hasRemaining(); - } - - /** - *

Copies the bytes from this Chunk to the given byte array.

- * - * @param bytes the byte array to copy the bytes into - * @param offset the offset within the byte array - * @param length the maximum number of bytes to copy - * @return the number of bytes actually copied - */ - default int get(byte[] bytes, int offset, int length) - { - ByteBuffer b = getByteBuffer(); - if (b == null || !b.hasRemaining()) - return 0; - length = Math.min(length, b.remaining()); - b.get(bytes, offset, length); - return length; - } - - /** - *

Skips, advancing the ByteBuffer position, the given number of bytes.

- * - * @param length the maximum number of bytes to skip - * @return the number of bytes actually skipped - */ - default int skip(int length) - { - if (length == 0) - return 0; - ByteBuffer byteBuffer = getByteBuffer(); - length = Math.min(byteBuffer.remaining(), length); - byteBuffer.position(byteBuffer.position() + length); - return length; - } - - /** - * @return an immutable version of this Chunk - */ - default Chunk asReadOnly() - { - if (getByteBuffer().isReadOnly()) - return this; - if (canRetain()) - return asChunk(getByteBuffer().asReadOnlyBuffer(), isLast(), this); - return from(getByteBuffer().asReadOnlyBuffer(), isLast()); - } - /** *

Implementations of this interface may process {@link Chunk}s being copied by the * {@link Content#copy(Source, Sink, Processor, Callback)} method, so that diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Retainable.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Retainable.java index 0e32781f6279..f9b1ab6bf1e4 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Retainable.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/Retainable.java @@ -62,6 +62,15 @@ default boolean canRetain() return false; } + /** + * @return whether this instance is retained + * @see ReferenceCounter#isRetained() + */ + default boolean isRetained() + { + return false; + } + /** *

Retains this resource, potentially incrementing a reference count if there are resources that will be released.

*/ @@ -103,6 +112,12 @@ public boolean canRetain() return getWrapped().canRetain(); } + @Override + public boolean isRetained() + { + return getWrapped().isRetained(); + } + @Override public void retain() { @@ -195,11 +210,7 @@ public boolean release() return ref == 0; } - /** - *

Returns whether {@link #retain()} has been called at least one more time than {@link #release()}.

- * - * @return whether this buffer is retained - */ + @Override public boolean isRetained() { return references.get() > 1; diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/RetainableByteBuffer.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/RetainableByteBuffer.java index 78e954833502..6046e529163c 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/RetainableByteBuffer.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/RetainableByteBuffer.java @@ -14,9 +14,14 @@ package org.eclipse.jetty.io; import java.nio.ByteBuffer; +import java.nio.ReadOnlyBufferException; +import java.util.ArrayList; +import java.util.List; import org.eclipse.jetty.io.internal.NonRetainableByteBuffer; import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.IteratingNestedCallback; /** *

A pooled {@link ByteBuffer} which maintains a reference count that is @@ -84,7 +89,7 @@ public ByteBuffer getByteBuffer() @Override public boolean isRetained() { - throw new UnsupportedOperationException(); + return retainable.isRetained(); } @Override @@ -108,10 +113,31 @@ public boolean release() } /** - * @return whether this instance is retained - * @see ReferenceCounter#isRetained() + *

Returns a {@code RetainableByteBuffer} that wraps + * the given {@code ByteBuffer} and {@link Runnable} releaser.

+ * + * @param byteBuffer the {@code ByteBuffer} to wrap + * @param releaser a {@link Runnable} to call when the buffer is released. + * @return a {@code RetainableByteBuffer} */ - boolean isRetained(); + static RetainableByteBuffer wrap(ByteBuffer byteBuffer, Runnable releaser) + { + return new AbstractRetainableByteBuffer(byteBuffer) + { + { + acquire(); + } + + @Override + public boolean release() + { + boolean released = super.release(); + if (released) + releaser.run(); + return released; + } + }; + } /** * Get the wrapped, not {@code null}, {@code ByteBuffer}. @@ -119,6 +145,22 @@ public boolean release() */ ByteBuffer getByteBuffer(); + /** + * Creates a copy of this RetainableByteBuffer that is entirely independent, but + * backed by the same memory space, i.e.: modifying the ByteBuffer of the original + * also modifies the ByteBuffer of the copy and vice-versa. + * @return A copy of this RetainableByteBuffer + */ + default RetainableByteBuffer copy() + { + return new AbstractRetainableByteBuffer(BufferUtil.copy(getByteBuffer())) + { + { + acquire(); + } + }; + } + /** * @return whether the {@code ByteBuffer} is direct */ @@ -152,6 +194,23 @@ default int capacity() } /** + * @return the number of bytes left for appending in the {@code ByteBuffer} + */ + default int space() + { + return capacity() - remaining(); + } + + /** + * @return whether the {@code ByteBuffer} has remaining bytes left for appending + */ + default boolean isFull() + { + return space() == 0; + } + + /** + * Clears the contained byte buffer to be empty in flush mode. * @see BufferUtil#clear(ByteBuffer) */ default void clear() @@ -159,6 +218,477 @@ default void clear() BufferUtil.clear(getByteBuffer()); } + /** + * Copies the contents of the given byte buffer at the end of this buffer. + * @param bytes the byte buffer to copy from. + * @return true if all bytes of the given buffer were copied, false otherwise. + * @throws ReadOnlyBufferException if the buffer is read only + * @see BufferUtil#append(ByteBuffer, ByteBuffer) + */ + default boolean append(ByteBuffer bytes) throws ReadOnlyBufferException + { + BufferUtil.append(getByteBuffer(), bytes); + return !bytes.hasRemaining(); + } + + /** + * Copies the contents of the given retainable byte buffer at the end of this buffer. + * @param bytes the retainable byte buffer to copy from. + * @return true if all bytes of the given buffer were copied, false otherwise. + * @throws ReadOnlyBufferException if the buffer is read only + * @see BufferUtil#append(ByteBuffer, ByteBuffer) + */ + default boolean append(RetainableByteBuffer bytes) throws ReadOnlyBufferException + { + return bytes.remaining() == 0 || append(bytes.getByteBuffer()); + } + + /** + *

Copies the bytes from this Chunk to the given byte array.

+ * + * @param bytes the byte array to copy the bytes into + * @param offset the offset within the byte array + * @param length the maximum number of bytes to copy + * @return the number of bytes actually copied + */ + default int get(byte[] bytes, int offset, int length) + { + ByteBuffer b = getByteBuffer(); + if (b == null || !b.hasRemaining()) + return 0; + length = Math.min(length, b.remaining()); + b.get(bytes, offset, length); + return length; + } + + /** + *

Skips, advancing the ByteBuffer position, the given number of bytes.

+ * + * @param length the maximum number of bytes to skip + * @return the number of bytes actually skipped + */ + default int skip(int length) + { + if (length == 0) + return 0; + ByteBuffer byteBuffer = getByteBuffer(); + length = Math.min(byteBuffer.remaining(), length); + byteBuffer.position(byteBuffer.position() + length); + return length; + } + + /** + * Copies the contents of this retainable byte buffer at the end of the given byte buffer. + * @param toInfillMode the destination buffer. + * @see ByteBuffer#put(ByteBuffer) + */ + default void putTo(ByteBuffer toInfillMode) + { + toInfillMode.put(getByteBuffer()); + } + + /** + * Asynchronously copies the contents of this retainable byte buffer into given sink. + * @param sink the destination sink. + * @param last true if this is the last write. + * @param callback the callback to call upon the write completion. + * @see org.eclipse.jetty.io.Content.Sink#write(boolean, ByteBuffer, Callback) + */ + default void writeTo(Content.Sink sink, boolean last, Callback callback) + { + sink.write(last, getByteBuffer(), callback); + } + + /** + * An aggregating {@link RetainableByteBuffer} that may grow when content is appended to it. + */ + class Aggregator implements RetainableByteBuffer + { + private final ByteBufferPool _pool; + private final boolean _direct; + private final int _growBy; + private final int _maxCapacity; + private RetainableByteBuffer _buffer; + + /** + * Construct an aggregating {@link RetainableByteBuffer} that may grow when content is appended to it. + * {@link RetainableByteBuffer}s with zero-copy if the {@link #append(RetainableByteBuffer)} API is used + * @param pool The pool from which to allocate buffers + * @param direct true if direct buffers should be used + * @param maxCapacity The maximum requested length of the accumulated buffers or -1 for 2GB limit. + * Note that the pool may provide a buffer that exceeds this capacity. + */ + public Aggregator(ByteBufferPool pool, boolean direct, int maxCapacity) + { + this(pool, direct, -1, maxCapacity); + } + + /** + * Construct an aggregating {@link RetainableByteBuffer} that may grow when content is appended to it. + * {@link RetainableByteBuffer}s with zero-copy if the {@link #append(RetainableByteBuffer)} API is used + * @param pool The pool from which to allocate buffers + * @param direct true if direct buffers should be used + * @param growBy the size to grow the buffer by or <= 0 for a heuristic + * @param maxCapacity The maximum requested length of the accumulated buffers or -1 for 2GB limit. + * Note that the pool may provide a buffer that exceeds this capacity. + */ + public Aggregator(ByteBufferPool pool, boolean direct, int growBy, int maxCapacity) + { + _pool = pool == null ? new ByteBufferPool.NonPooling() : pool; + _direct = direct; + _maxCapacity = maxCapacity <= 0 ? Integer.MAX_VALUE : maxCapacity; + + if (growBy <= 0) + { + _buffer = _pool.acquire(Math.min(1024, _maxCapacity), _direct); + _growBy = Math.min(_maxCapacity, _buffer.capacity()); + } + else + { + if (growBy > _maxCapacity) + throw new IllegalArgumentException("growBy(%d) must be <= maxCapacity(%d)".formatted(growBy, _maxCapacity)); + + _growBy = growBy; + _buffer = _pool.acquire(_growBy, _direct); + } + } + + @Override + public void clear() + { + if (isRetained()) + { + _buffer.release(); + _buffer = _pool.acquire(_growBy, _direct); + } + else + { + BufferUtil.clear(_buffer.getByteBuffer()); + } + } + + @Override + public boolean canRetain() + { + return _buffer.canRetain(); + } + + @Override + public boolean isRetained() + { + return _buffer.isRetained(); + } + + @Override + public void retain() + { + _buffer.retain(); + } + + @Override + public boolean release() + { + return _buffer.release(); + } + + @Override + public ByteBuffer getByteBuffer() + { + return _buffer.getByteBuffer(); + } + + @Override + public RetainableByteBuffer copy() + { + RetainableByteBuffer buffer = _buffer; + buffer.retain(); + return new AbstractRetainableByteBuffer(buffer.getByteBuffer().slice()) + { + { + acquire(); + } + + @Override + public boolean release() + { + if (super.release()) + { + buffer.release(); + return true; + } + return false; + } + }; + } + + @Override + public int capacity() + { + return Math.max(_buffer.capacity(), _maxCapacity); + } + + @Override + public boolean append(ByteBuffer bytes) + { + ensureSpace(bytes.remaining()); + return RetainableByteBuffer.super.append(bytes); + } + + @Override + public boolean append(RetainableByteBuffer bytes) + { + ensureSpace(bytes.remaining()); + return RetainableByteBuffer.super.append(bytes); + } + + private void ensureSpace(int spaceNeeded) + { + int capacity = _buffer.capacity(); + int space = capacity - _buffer.remaining(); + if (spaceNeeded <= space || capacity >= _maxCapacity) + return; + + int newCapacity = Math.multiplyExact(1 + Math.addExact(capacity, spaceNeeded) / _growBy, _growBy); + if (newCapacity > _maxCapacity) + { + newCapacity = Math.addExact(capacity, spaceNeeded - space); + if (newCapacity > _maxCapacity) + newCapacity = _maxCapacity; + } + + RetainableByteBuffer ensured = _pool.acquire(newCapacity, _direct); + ensured.append(_buffer); + _buffer.release(); + _buffer = ensured; + } + } + + /** + * An accumulating {@link RetainableByteBuffer} that may internally accumulate multiple other + * {@link RetainableByteBuffer}s with zero-copy if the {@link #append(RetainableByteBuffer)} API is used + */ + class Accumulator implements RetainableByteBuffer + { + private final ReferenceCounter _retainable = new ReferenceCounter(1); + private final ByteBufferPool _pool; + private final boolean _direct; + private final long _maxLength; + private final List _buffers = new ArrayList<>(); + + /** + * Construct an accumulating {@link RetainableByteBuffer} that may internally accumulate multiple other + * {@link RetainableByteBuffer}s with zero-copy if the {@link #append(RetainableByteBuffer)} API is used + * @param pool The pool from which to allocate buffers + * @param direct true if direct buffers should be used + * @param maxLength The maximum length of the accumulated buffers or -1 for 2GB limit + */ + public Accumulator(ByteBufferPool pool, boolean direct, long maxLength) + { + _pool = pool == null ? new ByteBufferPool.NonPooling() : pool; + _direct = direct; + _maxLength = maxLength < 0 ? Long.MAX_VALUE : maxLength; + } + + @Override + public ByteBuffer getByteBuffer() + { + return switch (_buffers.size()) + { + case 0 -> RetainableByteBuffer.EMPTY.getByteBuffer(); + case 1 -> _buffers.get(0).getByteBuffer(); + default -> + { + RetainableByteBuffer combined = copy(true); + _buffers.add(combined); + yield combined.getByteBuffer(); + } + }; + } + + @Override + public RetainableByteBuffer copy() + { + return copy(false); + } + + private RetainableByteBuffer copy(boolean take) + { + int length = remaining(); + RetainableByteBuffer combinedBuffer = _pool.acquire(length, _direct); + ByteBuffer byteBuffer = combinedBuffer.getByteBuffer(); + BufferUtil.flipToFill(byteBuffer); + for (RetainableByteBuffer buffer : _buffers) + { + byteBuffer.put(buffer.getByteBuffer().slice()); + if (take) + buffer.release(); + } + BufferUtil.flipToFlush(byteBuffer, 0); + if (take) + _buffers.clear(); + return combinedBuffer; + } + + /** + * {@inheritDoc} + * @return {@link Integer#MAX_VALUE} if the length of this {@code Accumulator} is greater than {@link Integer#MAX_VALUE} + */ + @Override + public int remaining() + { + long remainingLong = remainingLong(); + return remainingLong > Integer.MAX_VALUE ? Integer.MAX_VALUE : Math.toIntExact(remainingLong); + } + + public long remainingLong() + { + long length = 0; + for (RetainableByteBuffer buffer : _buffers) + length += buffer.remaining(); + return length; + } + + /** + * {@inheritDoc} + * @return {@link Integer#MAX_VALUE} if the maxLength of this {@code Accumulator} is greater than {@link Integer#MAX_VALUE}. + */ + @Override + public int capacity() + { + long capacityLong = capacityLong(); + return capacityLong > Integer.MAX_VALUE ? Integer.MAX_VALUE : Math.toIntExact(capacityLong); + } + + public long capacityLong() + { + return _maxLength; + } + + @Override + public boolean canRetain() + { + return _retainable.canRetain(); + } + + @Override + public boolean isRetained() + { + return _retainable.isRetained(); + } + + @Override + public void retain() + { + _retainable.retain(); + } + + @Override + public boolean release() + { + if (_retainable.release()) + { + clear(); + return true; + } + return false; + } + + @Override + public void clear() + { + for (RetainableByteBuffer buffer : _buffers) + buffer.release(); + _buffers.clear(); + } + + @Override + public boolean append(ByteBuffer bytes) + { + int remaining = bytes.remaining(); + if (remaining == 0) + return true; + + long currentlyRemaining = _maxLength - remainingLong(); + if (currentlyRemaining >= remaining) + { + RetainableByteBuffer rbb = RetainableByteBuffer.wrap(bytes.slice()); + bytes.position(bytes.limit()); + _buffers.add(rbb); + return true; + } + else + { + ByteBuffer slice = bytes.slice(); + slice.limit((int)(slice.position() + currentlyRemaining)); + RetainableByteBuffer rbb = RetainableByteBuffer.wrap(slice); + bytes.position((int)(bytes.position() + currentlyRemaining)); + _buffers.add(rbb); + return false; + } + } + + @Override + public boolean append(RetainableByteBuffer retainableBytes) + { + ByteBuffer bytes = retainableBytes.getByteBuffer(); + int remaining = bytes.remaining(); + if (remaining == 0) + return true; + + long currentlyRemaining = _maxLength - remainingLong(); + if (currentlyRemaining >= remaining) + { + retainableBytes.retain(); + RetainableByteBuffer rbb = RetainableByteBuffer.wrap(bytes.slice(), retainableBytes); + bytes.position(bytes.limit()); + _buffers.add(rbb); + return true; + } + else + { + retainableBytes.retain(); + ByteBuffer slice = bytes.slice(); + slice.limit((int)(slice.position() + currentlyRemaining)); + RetainableByteBuffer rbb = RetainableByteBuffer.wrap(slice, retainableBytes); + bytes.position((int)(bytes.position() + currentlyRemaining)); + _buffers.add(rbb); + return false; + } + } + + @Override + public void putTo(ByteBuffer toInfillMode) + { + for (RetainableByteBuffer buffer : _buffers) + buffer.putTo(toInfillMode); + } + + @Override + public void writeTo(Content.Sink sink, boolean last, Callback callback) + { + switch (_buffers.size()) + { + case 0 -> callback.succeeded(); + case 1 -> _buffers.get(0).writeTo(sink, last, callback); + default -> new IteratingNestedCallback(callback) + { + private int i = 0; + + @Override + protected Action process() + { + if (i < _buffers.size()) + { + _buffers.get(i).writeTo(sink, last && ++i == _buffers.size(), this); + return Action.SCHEDULED; + } + return Action.SUCCEEDED; + } + }.iterate(); + } + } + } + /** * A wrapper for {@link RetainableByteBuffer} instances */ diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java index 6bd5eeebc32a..d4db343868a2 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/AsyncContent.java @@ -301,6 +301,12 @@ public boolean canRetain() return referenceCounter != null; } + @Override + public boolean isRetained() + { + return canRetain() && referenceCounter.isRetained(); + } + @Override public void retain() { @@ -330,5 +336,17 @@ public void failed(Throwable x) { callback.failed(x); } + + @Override + public String toString() + { + return "%s@%x[rc=%s,l=%b,b=%s]".formatted( + getClass().getSimpleName(), + hashCode(), + referenceCounter == null ? "-" : referenceCounter.get(), + isLast(), + BufferUtil.toDetailString(getByteBuffer()) + ); + } } } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java index acb85e2ddb60..d709df155946 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/content/BufferedContentSink.java @@ -17,7 +17,6 @@ import java.nio.ByteBuffer; import java.nio.channels.WritePendingException; -import org.eclipse.jetty.io.ByteBufferAggregator; import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.Content; import org.eclipse.jetty.io.RetainableByteBuffer; @@ -43,15 +42,13 @@ public class BufferedContentSink implements Content.Sink private static final Logger LOG = LoggerFactory.getLogger(BufferedContentSink.class); - private static final int START_BUFFER_SIZE = 1024; - + private final Flusher _flusher = new Flusher(); private final Content.Sink _delegate; private final ByteBufferPool _bufferPool; private final boolean _direct; private final int _maxBufferSize; private final int _maxAggregationSize; - private final Flusher _flusher; - private ByteBufferAggregator _aggregator; + private RetainableByteBuffer _aggregator; private boolean _firstWrite = true; private boolean _lastWritten; @@ -68,7 +65,13 @@ public BufferedContentSink(Content.Sink delegate, ByteBufferPool bufferPool, boo _direct = direct; _maxBufferSize = maxBufferSize; _maxAggregationSize = maxAggregationSize; - _flusher = new Flusher(delegate); + } + + private void releaseAggregator() + { + if (_aggregator != null) + _aggregator.release(); + _aggregator = null; } @Override @@ -99,14 +102,13 @@ public void write(boolean last, ByteBuffer byteBuffer, Callback callback) { // current buffer can be aggregated if (_aggregator == null) - _aggregator = new ByteBufferAggregator(_bufferPool, _direct, Math.min(START_BUFFER_SIZE, _maxBufferSize), _maxBufferSize); - aggregateAndFlush(last, current, callback); - } - else - { - // current buffer is greater than the max aggregation size - flush(last, current, callback); + _aggregator = new RetainableByteBuffer.Aggregator(_bufferPool, _direct, _maxBufferSize); + aggregate(last, current, callback); + return; } + + // current buffer is greater than the max aggregation size + _flusher.flush(last, current, callback); } /** @@ -115,154 +117,57 @@ public void write(boolean last, ByteBuffer byteBuffer, Callback callback) */ public void flush(Callback callback) { - flush(false, FLUSH_BUFFER, callback); - } - - /** - * Flushes the aggregated buffer if something was aggregated, then flushes the - * given buffer, bypassing the aggregator. - */ - private void flush(boolean last, ByteBuffer currentBuffer, Callback callback) - { - if (LOG.isDebugEnabled()) - LOG.debug("given buffer is greater than _maxBufferSize"); - - RetainableByteBuffer aggregatedBuffer = _aggregator == null ? null : _aggregator.takeRetainableByteBuffer(); - if (aggregatedBuffer == null) - { - if (LOG.isDebugEnabled()) - LOG.debug("nothing aggregated, flushing current buffer {}", currentBuffer); - _flusher.offer(last, currentBuffer, callback); - } - else if (BufferUtil.hasContent(currentBuffer)) - { - if (LOG.isDebugEnabled()) - LOG.debug("flushing aggregated buffer {}", aggregatedBuffer); - _flusher.offer(false, aggregatedBuffer.getByteBuffer(), new Callback.Nested(Callback.from(aggregatedBuffer::release)) - { - @Override - public void succeeded() - { - super.succeeded(); - if (LOG.isDebugEnabled()) - LOG.debug("succeeded writing aggregated buffer, flushing current buffer {}", currentBuffer); - _flusher.offer(last, currentBuffer, callback); - } - - @Override - public void failed(Throwable x) - { - if (LOG.isDebugEnabled()) - LOG.debug("failure writing aggregated buffer", x); - super.failed(x); - callback.failed(x); - } - }); - } - else - { - _flusher.offer(false, aggregatedBuffer.getByteBuffer(), Callback.from(aggregatedBuffer::release, callback)); - } + _flusher.flush(false, FLUSH_BUFFER, callback); } /** * Aggregates the given buffer, flushing the aggregated buffer if necessary. */ - private void aggregateAndFlush(boolean last, ByteBuffer currentBuffer, Callback callback) + private void aggregate(boolean last, ByteBuffer byteBuffer, Callback callback) { - boolean full = _aggregator.aggregate(currentBuffer); - boolean empty = !currentBuffer.hasRemaining(); - boolean flush = full || currentBuffer == FLUSH_BUFFER; + boolean full = !_aggregator.append(byteBuffer) || _aggregator.isFull(); + boolean empty = !byteBuffer.hasRemaining(); + boolean flush = full || byteBuffer == FLUSH_BUFFER; boolean complete = last && empty; if (LOG.isDebugEnabled()) - LOG.debug("aggregated current buffer, full={}, complete={}, bytes left={}, aggregator={}", full, complete, currentBuffer.remaining(), _aggregator); - if (complete) - { - RetainableByteBuffer aggregatedBuffer = _aggregator.takeRetainableByteBuffer(); - if (aggregatedBuffer != null) - { - if (LOG.isDebugEnabled()) - LOG.debug("complete; writing aggregated buffer as the last one: {} bytes", aggregatedBuffer.remaining()); - _flusher.offer(true, aggregatedBuffer.getByteBuffer(), Callback.from(callback, aggregatedBuffer::release)); - } - else - { - if (LOG.isDebugEnabled()) - LOG.debug("complete; no aggregated buffer, writing last empty buffer"); - _flusher.offer(true, BufferUtil.EMPTY_BUFFER, callback); - } - } - else if (flush) - { - RetainableByteBuffer aggregatedBuffer = _aggregator.takeRetainableByteBuffer(); - if (LOG.isDebugEnabled()) - LOG.debug("writing aggregated buffer: {} bytes, then {}", aggregatedBuffer.remaining(), currentBuffer.remaining()); - - if (BufferUtil.hasContent(currentBuffer)) - { - _flusher.offer(false, aggregatedBuffer.getByteBuffer(), new Callback.Nested(Callback.from(aggregatedBuffer::release)) - { - @Override - public void succeeded() - { - super.succeeded(); - if (LOG.isDebugEnabled()) - LOG.debug("written aggregated buffer, writing remaining of current: {} bytes{}", currentBuffer.remaining(), (last ? " (last write)" : "")); - if (last) - _flusher.offer(true, currentBuffer, callback); - else - aggregateAndFlush(false, currentBuffer, callback); - } - - @Override - public void failed(Throwable x) - { - if (LOG.isDebugEnabled()) - LOG.debug("failure writing aggregated buffer", x); - super.failed(x); - callback.failed(x); - } - }); - } - else - { - _flusher.offer(false, aggregatedBuffer.getByteBuffer(), Callback.from(aggregatedBuffer::release, callback)); - } - } + LOG.debug("aggregated current buffer, full={}, complete={}, bytes left={}, aggregator={}", full, complete, byteBuffer.remaining(), _aggregator); + if (complete || flush) + _flusher.flush(last, byteBuffer, callback); else - { - if (LOG.isDebugEnabled()) - LOG.debug("buffer fully aggregated, delaying writing - aggregator: {}", _aggregator); - _flusher.offer(callback); - } + _flusher.serialize(callback); } - private static class Flusher extends IteratingCallback + private class Flusher extends IteratingCallback { - private static final ByteBuffer COMPLETE_CALLBACK = BufferUtil.allocate(0); + private enum Scheduled + { + FLUSHING_AGGREGATION, + FLUSHING_BUFFER, + } - private final Content.Sink _sink; + private Scheduled _scheduled; + private boolean _flush; private boolean _last; - private ByteBuffer _buffer; + private ByteBuffer _byteBuffer; private Callback _callback; private boolean _lastWritten; - Flusher(Content.Sink sink) + private void flush(boolean last, ByteBuffer byteBuffer, Callback callback) { - _sink = sink; - } - - void offer(Callback callback) - { - offer(false, COMPLETE_CALLBACK, callback); + if (_callback != null) + throw new WritePendingException(); + _flush = true; + _last = last; + _byteBuffer = byteBuffer; + _callback = callback; + iterate(); } - void offer(boolean last, ByteBuffer byteBuffer, Callback callback) + private void serialize(Callback callback) { if (_callback != null) throw new WritePendingException(); - _last = last; - _buffer = byteBuffer; + _flush = false; _callback = callback; iterate(); } @@ -270,37 +175,79 @@ void offer(boolean last, ByteBuffer byteBuffer, Callback callback) @Override protected Action process() { - if (_lastWritten) - return Action.SUCCEEDED; - if (_callback == null) - return Action.IDLE; - if (_buffer != COMPLETE_CALLBACK) + if (_scheduled != null) { - _lastWritten = _last; - _sink.write(_last, _buffer, this); + switch (_scheduled) + { + case FLUSHING_AGGREGATION -> + { + _aggregator.clear(); + if (_byteBuffer != null && _byteBuffer.remaining() <= _maxAggregationSize && !_last) + { + _aggregator.append(_byteBuffer); + _byteBuffer = null; + _flush = false; + } + } + + case FLUSHING_BUFFER -> + _byteBuffer = null; + } + _scheduled = null; } - else + + if (_flush && _aggregator != null && _aggregator.hasRemaining()) { - succeeded(); + boolean last = _last && BufferUtil.isEmpty(_byteBuffer); + _lastWritten |= last; + _aggregator.writeTo(_delegate, last, this); + _scheduled = Scheduled.FLUSHING_AGGREGATION; + return Action.SCHEDULED; } - return Action.SCHEDULED; + + if (_flush && (BufferUtil.hasContent(_byteBuffer) || _byteBuffer == FLUSH_BUFFER)) + { + ByteBuffer buffer = _byteBuffer; + _byteBuffer = null; + _lastWritten |= _last; + _delegate.write(_last, buffer, this); + _scheduled = Scheduled.FLUSHING_BUFFER; + return Action.SCHEDULED; + } + + if (_last && !_lastWritten) + { + _lastWritten = true; + _delegate.write(_last, BufferUtil.EMPTY_BUFFER, this); + return Action.SCHEDULED; + } + + Callback callback = _callback; + if (callback != null) + { + _callback = null; + callback.succeeded(); + this.succeeded(); + return Action.SCHEDULED; + } + + return _last ? Action.SUCCEEDED : Action.IDLE; } @Override - public void succeeded() + protected void onCompleteFailure(Throwable cause) { - _buffer = null; + releaseAggregator(); Callback callback = _callback; _callback = null; - callback.succeeded(); - super.succeeded(); + if (callback != null) + callback.failed(cause); } @Override - protected void onCompleteFailure(Throwable cause) + protected void onCompleteSuccess() { - _buffer = null; - _callback.failed(cause); + releaseAggregator(); } } } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteBufferChunk.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteBufferChunk.java index 821782fd32fe..29f40d25c4f2 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteBufferChunk.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ByteBufferChunk.java @@ -65,6 +65,12 @@ public WithReferenceCount(ByteBuffer byteBuffer, boolean last) super(byteBuffer, last); } + @Override + public boolean isRetained() + { + return references.isRetained(); + } + @Override public boolean canRetain() { @@ -148,6 +154,12 @@ public WithRetainable(ByteBuffer byteBuffer, boolean last, Retainable retainable this.retainable = retainable; } + @Override + public boolean isRetained() + { + return retainable.isRetained(); + } + @Override public boolean canRetain() { diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceByteBuffer.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceByteBuffer.java index 9db5923b287f..79936a6c3e0f 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceByteBuffer.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceByteBuffer.java @@ -15,13 +15,13 @@ import java.nio.ByteBuffer; -import org.eclipse.jetty.io.ByteBufferAccumulator; import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.util.Promise; public class ContentSourceByteBuffer implements Runnable { - private final ByteBufferAccumulator accumulator = new ByteBufferAccumulator(); + private final RetainableByteBuffer.Accumulator accumulator = new RetainableByteBuffer.Accumulator(null, false, -1); private final Content.Source source; private final Promise promise; @@ -52,12 +52,18 @@ public void run() return; } - accumulator.copyBuffer(chunk.getByteBuffer()); + accumulator.append(chunk); chunk.release(); if (chunk.isLast()) { - promise.succeeded(accumulator.takeByteBuffer()); + RetainableByteBuffer copy = accumulator.copy(); + accumulator.release(); + + // We know the accumulator is not using a pool, so whilst we release after succeeded, it is safe + // for the promise to retain the ByteBuffer after the call. + promise.succeeded(copy.getByteBuffer()); + copy.release(); return; } } diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRetainableByteBuffer.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRetainableByteBuffer.java new file mode 100644 index 000000000000..65015cd2d64b --- /dev/null +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/internal/ContentSourceRetainableByteBuffer.java @@ -0,0 +1,75 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.io.internal; + +import org.eclipse.jetty.io.ByteBufferPool; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.RetainableByteBuffer; +import org.eclipse.jetty.util.Promise; + +public class ContentSourceRetainableByteBuffer implements Runnable +{ + private final RetainableByteBuffer.Accumulator accumulator; + private final Content.Source source; + private final Promise promise; + + public ContentSourceRetainableByteBuffer(Content.Source source, ByteBufferPool pool, boolean direct, int maxSize, Promise promise) + { + this.source = source; + this.accumulator = new RetainableByteBuffer.Accumulator(pool, direct, maxSize); + this.promise = promise; + } + + @Override + public void run() + { + while (true) + { + Content.Chunk chunk = source.read(); + + if (chunk == null) + { + source.demand(this); + return; + } + + if (Content.Chunk.isFailure(chunk)) + { + promise.failed(chunk.getFailure()); + if (!chunk.isLast()) + source.fail(chunk.getFailure()); + return; + } + + boolean appended = accumulator.append(chunk); + chunk.release(); + + if (!appended) + { + IllegalStateException ise = new IllegalStateException("Max size (" + accumulator.capacity() + ") exceeded"); + promise.failed(ise); + accumulator.release(); + source.fail(ise); + return; + } + + if (chunk.isLast()) + { + promise.succeeded(accumulator); + accumulator.release(); + return; + } + } + } +} diff --git a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java index 39f74d7dde11..7c9c8acb59a4 100644 --- a/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java +++ b/jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/ssl/SslConnection.java @@ -342,7 +342,7 @@ private void acquireEncryptedOutput() public void onUpgradeTo(ByteBuffer buffer) { acquireEncryptedInput(); - BufferUtil.append(_encryptedInput.getByteBuffer(), buffer); + _encryptedInput.append(buffer); } @Override diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/BufferedContentSinkTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/BufferedContentSinkTest.java index 0731eff5d32b..0f4108f6343d 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/BufferedContentSinkTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/BufferedContentSinkTest.java @@ -13,9 +13,13 @@ package org.eclipse.jetty.io; +import java.io.ByteArrayOutputStream; +import java.io.EOFException; import java.io.IOException; import java.nio.ByteBuffer; +import java.util.ArrayList; import java.util.Arrays; +import java.util.List; import java.util.Objects; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; @@ -34,9 +38,11 @@ import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import static java.nio.charset.StandardCharsets.US_ASCII; import static java.nio.charset.StandardCharsets.UTF_8; import static org.awaitility.Awaitility.await; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; @@ -270,6 +276,12 @@ public void testFlush(BiConsumer flusher) throws assertThat(BufferUtil.toString(chunk.getByteBuffer()), is("Hello World!")); chunk.release(); callback.get(5, TimeUnit.SECONDS); + + buffered.write(true, BufferUtil.EMPTY_BUFFER, Callback.NOOP); + chunk = async.read(); + assertThat(chunk.isLast(), is(true)); + assertThat(chunk.remaining(), is(0)); + chunk.release(); } } @@ -509,10 +521,11 @@ public void succeeded() @Test public void testByteByByteAsync() throws Exception { + // This test relies on selecting a size that will not be over allocated by the buffer pool try (AsyncContent async = new AsyncContent()) { - BufferedContentSink buffered = new BufferedContentSink(async, _bufferPool, true, 1024, 1024); - AtomicInteger count = new AtomicInteger(2048); + BufferedContentSink buffered = new BufferedContentSink(async, _bufferPool, true, 8 * 1024, 8 * 1024); + AtomicInteger count = new AtomicInteger(16 * 1024); CountDownLatch complete = new CountDownLatch(1); Callback callback = new Callback() { @@ -540,12 +553,12 @@ public void succeeded() Content.Chunk read = await().atMost(5, TimeUnit.SECONDS).until(async::read, Objects::nonNull); assertThat(read.isLast(), is(false)); - assertThat(read.remaining(), is(1024)); + assertThat(read.remaining(), is(8 * 1024)); assertThat(read.release(), is(true)); read = await().atMost(5, TimeUnit.SECONDS).until(async::read, Objects::nonNull); assertThat(read.isLast(), is(true)); - assertThat(read.remaining(), is(1024)); + assertThat(read.remaining(), is(8 * 1024)); assertThat(read.release(), is(true)); assertTrue(complete.await(5, TimeUnit.SECONDS)); @@ -594,4 +607,45 @@ public void succeeded() assertThat(count.get(), is(-1)); } } + + @Test + public void testFromOutputStream() + { + ByteArrayOutputStream baos = new ByteArrayOutputStream(); + Content.Sink sink = Content.Sink.from(baos); + + AccountingCallback accountingCallback = new AccountingCallback(); + + sink.write(false, ByteBuffer.wrap("hello ".getBytes(US_ASCII)), accountingCallback); + assertThat(accountingCallback.reports, equalTo(List.of("succeeded"))); + accountingCallback.reports.clear(); + + sink.write(true, ByteBuffer.wrap("world".getBytes(US_ASCII)), accountingCallback); + assertThat(accountingCallback.reports, equalTo(List.of("succeeded"))); + accountingCallback.reports.clear(); + + sink.write(true, ByteBuffer.wrap(" again".getBytes(US_ASCII)), accountingCallback); + assertThat(accountingCallback.reports.size(), is(1)); + assertThat(accountingCallback.reports.get(0), instanceOf(EOFException.class)); + accountingCallback.reports.clear(); + + assertThat(baos.toString(US_ASCII), is("hello world")); + } + + private static class AccountingCallback implements Callback + { + private final List reports = new ArrayList<>(); + + @Override + public void succeeded() + { + reports.add("succeeded"); + } + + @Override + public void failed(Throwable x) + { + reports.add(x); + } + } } diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ByteBufferAccumulatorTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ByteBufferAccumulatorTest.java index 4de6dfc3bc3f..ec2332863178 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ByteBufferAccumulatorTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ByteBufferAccumulatorTest.java @@ -28,6 +28,7 @@ import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertThrows; +@Deprecated(forRemoval = true) public class ByteBufferAccumulatorTest { private CountingBufferPool bufferPool; diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ByteBufferAggregatorTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ByteBufferAggregatorTest.java deleted file mode 100644 index 80509a364522..000000000000 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ByteBufferAggregatorTest.java +++ /dev/null @@ -1,88 +0,0 @@ -// -// ======================================================================== -// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. -// -// This program and the accompanying materials are made available under the -// terms of the Eclipse Public License v. 2.0 which is available at -// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 -// which is available at https://www.apache.org/licenses/LICENSE-2.0. -// -// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 -// ======================================================================== -// - -package org.eclipse.jetty.io; - -import java.nio.ByteBuffer; - -import org.junit.jupiter.api.AfterEach; -import org.junit.jupiter.api.BeforeEach; -import org.junit.jupiter.api.Test; - -import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.is; -import static org.junit.jupiter.api.Assertions.assertThrows; - -public class ByteBufferAggregatorTest -{ - private ArrayByteBufferPool.Tracking bufferPool; - - @BeforeEach - public void before() - { - bufferPool = new ArrayByteBufferPool.Tracking(); - } - - @AfterEach - public void tearDown() - { - assertThat("Leaks: " + bufferPool.dumpLeaks(), bufferPool.getLeaks().size(), is(0)); - } - - @Test - public void testConstructor() - { - assertThrows(IllegalArgumentException.class, () -> new ByteBufferAggregator(bufferPool, true, 0, 0)); - assertThrows(IllegalArgumentException.class, () -> new ByteBufferAggregator(bufferPool, true, 0, 1)); - assertThrows(IllegalArgumentException.class, () -> new ByteBufferAggregator(bufferPool, true, 1, 0)); - assertThrows(IllegalArgumentException.class, () -> new ByteBufferAggregator(bufferPool, true, 0, -1)); - assertThrows(IllegalArgumentException.class, () -> new ByteBufferAggregator(bufferPool, true, -1, 0)); - assertThrows(IllegalArgumentException.class, () -> new ByteBufferAggregator(bufferPool, true, 2, 1)); - } - - @Test - public void testFullInSingleShot() - { - ByteBufferAggregator aggregator = new ByteBufferAggregator(bufferPool, true, 1, 16); - - ByteBuffer byteBuffer1 = ByteBuffer.wrap(new byte[16]); - assertThat(aggregator.aggregate(byteBuffer1), is(true)); - assertThat(byteBuffer1.remaining(), is(0)); - - ByteBuffer byteBuffer2 = ByteBuffer.wrap(new byte[16]); - assertThat(aggregator.aggregate(byteBuffer2), is(true)); - assertThat(byteBuffer2.remaining(), is(16)); - - RetainableByteBuffer retainableByteBuffer = aggregator.takeRetainableByteBuffer(); - assertThat(retainableByteBuffer.getByteBuffer().remaining(), is(16)); - assertThat(retainableByteBuffer.release(), is(true)); - } - - @Test - public void testFullInMultipleShots() - { - ByteBufferAggregator aggregator = new ByteBufferAggregator(bufferPool, true, 1, 16); - - ByteBuffer byteBuffer1 = ByteBuffer.wrap(new byte[15]); - assertThat(aggregator.aggregate(byteBuffer1), is(false)); - assertThat(byteBuffer1.remaining(), is(0)); - - ByteBuffer byteBuffer2 = ByteBuffer.wrap(new byte[16]); - assertThat(aggregator.aggregate(byteBuffer2), is(true)); - assertThat(byteBuffer2.remaining(), is(15)); - - RetainableByteBuffer retainableByteBuffer = aggregator.takeRetainableByteBuffer(); - assertThat(retainableByteBuffer.getByteBuffer().remaining(), is(16)); - assertThat(retainableByteBuffer.release(), is(true)); - } -} diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTest.java index 45799c4f1fcc..76822febd09f 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentSourceTest.java @@ -25,6 +25,7 @@ import java.util.Deque; import java.util.List; import java.util.concurrent.CancellationException; +import java.util.concurrent.CompletableFuture; import java.util.concurrent.ConcurrentLinkedDeque; import java.util.concurrent.CountDownLatch; import java.util.concurrent.ExecutionException; @@ -57,6 +58,7 @@ import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -654,6 +656,8 @@ public void demand(Runnable demandCallback) @Override public void fail(Throwable failure) { + _chunks.clear(); + _chunks.add(Content.Chunk.from(failure, true)); } } @@ -715,4 +719,142 @@ public void testAsyncContentWithWarningsAsInputStream() throws Exception len = in.read(buffer); assertThat(len, is(-1)); } + + @Test + public void testAsRetainableByteBufferWithPromise() throws Exception + { + TestContentSource source = new TestContentSource(); + + FuturePromise promise = new FuturePromise<>() + { + @Override + public void succeeded(RetainableByteBuffer result) + { + result.retain(); + super.succeeded(result); + } + }; + Content.Source.asRetainableByteBuffer(source, null, false, -1, promise); + + Retainable.ReferenceCounter counter = new Retainable.ReferenceCounter(3); + + Runnable todo = source.takeDemand(); + assertNotNull(todo); + source.add(Content.Chunk.asChunk(BufferUtil.toBuffer("hello"), false, counter)); + todo.run(); + assertFalse(promise.isDone()); + + todo = source.takeDemand(); + assertNotNull(todo); + source.add(Content.Chunk.asChunk(BufferUtil.toBuffer(" cruel"), false, counter)); + source.add(Content.Chunk.asChunk(BufferUtil.toBuffer(" world"), true, counter)); + todo.run(); + + todo = source.takeDemand(); + assertNull(todo); + assertTrue(promise.isDone()); + + RetainableByteBuffer buffer = promise.get(); + assertNotNull(buffer); + + assertThat(BufferUtil.toString(buffer.getByteBuffer()), equalTo("hello cruel world")); + } + + @Test + public void testAsRetainableByteBufferWithPromiseExceedsMaxSize() throws Exception + { + TestContentSource source = new TestContentSource(); + + FuturePromise promise = new FuturePromise<>() + { + @Override + public void succeeded(RetainableByteBuffer result) + { + result.retain(); + super.succeeded(result); + } + }; + Content.Source.asRetainableByteBuffer(source, null, false, 3, promise); + + Runnable todo = source.takeDemand(); + assertNotNull(todo); + source.add(Content.Chunk.asChunk(BufferUtil.toBuffer("hello"), false, new Retainable.ReferenceCounter(1))); + todo.run(); + assertTrue(promise.isDone()); + + try + { + promise.get(); + fail("expected ExecutionException"); + } + catch (ExecutionException e) + { + assertInstanceOf(IllegalStateException.class, e.getCause()); + } + + assertInstanceOf(IllegalStateException.class, source.read().getFailure()); + } + + @Test + public void testAsRetainableByteBufferWithCompletableFuture() throws Exception + { + TestContentSource source = new TestContentSource(); + + CompletableFuture completableFuture = Content.Source.asRetainableByteBuffer(source, null, false, -1); + + Retainable.ReferenceCounter counter = new Retainable.ReferenceCounter(3); + + Runnable todo = source.takeDemand(); + assertNotNull(todo); + source.add(Content.Chunk.asChunk(BufferUtil.toBuffer("hello"), false, counter)); + todo.run(); + assertFalse(completableFuture.isDone()); + + todo = source.takeDemand(); + assertNotNull(todo); + source.add(Content.Chunk.asChunk(BufferUtil.toBuffer(" cruel"), false, counter)); + source.add(Content.Chunk.asChunk(BufferUtil.toBuffer(" world"), true, counter)); + todo.run(); + + todo = source.takeDemand(); + assertNull(todo); + assertTrue(completableFuture.isDone()); + + RetainableByteBuffer buffer = completableFuture.get(); + assertNotNull(buffer); + + assertThat(BufferUtil.toString(buffer.getByteBuffer()), equalTo("hello cruel world")); + } + + @Test + public void testAsByteArrayAsync() throws Exception + { + TestContentSource source = new TestContentSource(); + + CompletableFuture completableFuture = Content.Source.asByteArrayAsync(source, -1); + + Retainable.ReferenceCounter counter = new Retainable.ReferenceCounter(3); + + Runnable todo = source.takeDemand(); + assertNotNull(todo); + source.add(Content.Chunk.asChunk(BufferUtil.toBuffer("hello"), false, counter)); + todo.run(); + assertFalse(completableFuture.isDone()); + + todo = source.takeDemand(); + assertNotNull(todo); + source.add(Content.Chunk.asChunk(BufferUtil.toBuffer(" cruel"), false, counter)); + source.add(Content.Chunk.asChunk(BufferUtil.toBuffer(" world"), true, counter)); + todo.run(); + + todo = source.takeDemand(); + assertNull(todo); + assertTrue(completableFuture.isDone()); + + byte[] buffer = completableFuture.get(); + assertNotNull(buffer); + + assertThat(new String(buffer, UTF_8), equalTo("hello cruel world")); + + } } diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentTest.java index 211914a4b802..add174fb4dd5 100644 --- a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentTest.java +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/ContentTest.java @@ -14,43 +14,18 @@ package org.eclipse.jetty.io; import java.nio.ByteBuffer; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; -import org.eclipse.jetty.util.BufferUtil; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; -import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.sameInstance; public class ContentTest { - @Test - public void testAsReadOnly() - { - assertThat(Content.Chunk.EOF.asReadOnly(), sameInstance(Content.Chunk.EOF)); - assertThat(Content.Chunk.EMPTY.asReadOnly(), sameInstance(Content.Chunk.EMPTY)); - - assertThat(Content.Chunk.from(BufferUtil.EMPTY_BUFFER, true).asReadOnly(), sameInstance(Content.Chunk.EOF)); - assertThat(Content.Chunk.from(BufferUtil.EMPTY_BUFFER, false).asReadOnly(), sameInstance(Content.Chunk.EMPTY)); - - Content.Chunk failureChunk = Content.Chunk.from(new NumberFormatException()); - assertThat(failureChunk.asReadOnly(), sameInstance(failureChunk)); - - Content.Chunk chunk = Content.Chunk.from(ByteBuffer.wrap(new byte[1]).asReadOnlyBuffer(), false); - assertThat(chunk.asReadOnly(), sameInstance(chunk)); - - Content.Chunk rwChunk = Content.Chunk.from(ByteBuffer.wrap("abc".getBytes(StandardCharsets.US_ASCII)), false); - Content.Chunk roChunk = rwChunk.asReadOnly(); - assertThat(rwChunk, not(sameInstance(roChunk))); - assertThat(BufferUtil.toString(rwChunk.getByteBuffer(), StandardCharsets.US_ASCII), equalTo(BufferUtil.toString(roChunk.getByteBuffer(), StandardCharsets.US_ASCII))); - } - @Test public void testFromEmptyByteBufferWithoutReleaser() { diff --git a/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/RetainableByteBufferTest.java b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/RetainableByteBufferTest.java new file mode 100644 index 000000000000..cdbef592e615 --- /dev/null +++ b/jetty-core/jetty-io/src/test/java/org/eclipse/jetty/io/RetainableByteBufferTest.java @@ -0,0 +1,260 @@ +// +// ======================================================================== +// Copyright (c) 1995 Mort Bay Consulting Pty Ltd and others. +// +// This program and the accompanying materials are made available under the +// terms of the Eclipse Public License v. 2.0 which is available at +// https://www.eclipse.org/legal/epl-2.0, or the Apache License, Version 2.0 +// which is available at https://www.apache.org/licenses/LICENSE-2.0. +// +// SPDX-License-Identifier: EPL-2.0 OR Apache-2.0 +// ======================================================================== +// + +package org.eclipse.jetty.io; + +import java.io.ByteArrayOutputStream; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.util.Arrays; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.stream.Stream; + +import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.FutureCallback; +import org.junit.jupiter.api.AfterAll; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; + +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.is; +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class RetainableByteBufferTest +{ + public static final int MIN_CAPACITY = 32; + public static final int MAX_CAPACITY = 64; + private static ArrayByteBufferPool.Tracking _pool; + + @BeforeAll + public static void beforeAll() + { + _pool = new ArrayByteBufferPool.Tracking(MIN_CAPACITY, MIN_CAPACITY, MAX_CAPACITY, Integer.MAX_VALUE); + } + + @AfterAll + public static void afterAll() + { + assertThat("Leaks: " + _pool.dumpLeaks(), _pool.getLeaks().size(), is(0)); + } + + public static Stream buffers() + { + return Stream.of( + Arguments.of(_pool.acquire(MIN_CAPACITY, true)), + Arguments.of(_pool.acquire(MIN_CAPACITY, false)), + Arguments.of(new RetainableByteBuffer.Aggregator(_pool, true, MIN_CAPACITY, MIN_CAPACITY)), + Arguments.of(new RetainableByteBuffer.Aggregator(_pool, false, MIN_CAPACITY, MIN_CAPACITY)), + Arguments.of(new RetainableByteBuffer.Accumulator(_pool, true, MIN_CAPACITY)), + Arguments.of(new RetainableByteBuffer.Accumulator(_pool, false, MIN_CAPACITY)) + ); + } + + @ParameterizedTest + @MethodSource("buffers") + public void testEmptyBuffer(RetainableByteBuffer buffer) + { + assertThat(buffer.remaining(), is(0)); + assertFalse(buffer.hasRemaining()); + assertThat(buffer.capacity(), greaterThanOrEqualTo(MIN_CAPACITY)); + assertFalse(buffer.isFull()); + + assertThat(buffer.getByteBuffer().remaining(), is(0)); + assertFalse(buffer.getByteBuffer().hasRemaining()); + buffer.release(); + } + + @ParameterizedTest + @MethodSource("buffers") + public void testAppendOneByte(RetainableByteBuffer buffer) + { + byte[] bytes = new byte[] {'-', 'X', '-'}; + while (!buffer.isFull()) + assertThat(buffer.append(ByteBuffer.wrap(bytes, 1, 1)), is(true)); + + assertThat(BufferUtil.toString(buffer.getByteBuffer()), is("X".repeat(buffer.capacity()))); + buffer.release(); + } + + @ParameterizedTest + @MethodSource("buffers") + public void testAppendOneByteRetainable(RetainableByteBuffer buffer) + { + RetainableByteBuffer toAppend = _pool.acquire(1, true); + BufferUtil.append(toAppend.getByteBuffer(), (byte)'X'); + assertThat(buffer.append(toAppend), is(true)); + assertFalse(toAppend.hasRemaining()); + toAppend.release(); + assertThat(BufferUtil.toString(buffer.getByteBuffer()), is("X")); + buffer.release(); + } + + @ParameterizedTest + @MethodSource("buffers") + public void testAppendMoreBytesThanCapacity(RetainableByteBuffer buffer) + { + byte[] bytes = new byte[MAX_CAPACITY * 2]; + Arrays.fill(bytes, (byte)'X'); + ByteBuffer b = ByteBuffer.wrap(bytes); + + if (buffer.append(b)) + { + assertTrue(BufferUtil.isEmpty(b)); + assertThat(buffer.capacity(), greaterThanOrEqualTo(MAX_CAPACITY * 2)); + } + else + { + assertFalse(BufferUtil.isEmpty(b)); + assertThat(b.remaining(), is(MAX_CAPACITY * 2 - buffer.capacity())); + } + + assertThat(BufferUtil.toString(buffer.getByteBuffer()), is("X".repeat(buffer.capacity()))); + assertTrue(buffer.isFull()); + buffer.release(); + } + + @ParameterizedTest + @MethodSource("buffers") + public void testAppendMoreBytesThanCapacityRetainable(RetainableByteBuffer buffer) + { + RetainableByteBuffer toAppend = _pool.acquire(MAX_CAPACITY * 2, true); + int pos = BufferUtil.flipToFill(toAppend.getByteBuffer()); + byte[] bytes = new byte[MAX_CAPACITY * 2]; + Arrays.fill(bytes, (byte)'X'); + toAppend.getByteBuffer().put(bytes); + BufferUtil.flipToFlush(toAppend.getByteBuffer(), pos); + + if (buffer.append(toAppend)) + { + assertTrue(BufferUtil.isEmpty(toAppend.getByteBuffer())); + assertThat(buffer.capacity(), greaterThanOrEqualTo(MAX_CAPACITY * 2)); + } + else + { + assertFalse(BufferUtil.isEmpty(toAppend.getByteBuffer())); + assertThat(toAppend.remaining(), is(MAX_CAPACITY * 2 - buffer.capacity())); + } + toAppend.release(); + + assertThat(BufferUtil.toString(buffer.getByteBuffer()), is("X".repeat(buffer.capacity()))); + assertTrue(buffer.isFull()); + buffer.release(); + } + + @ParameterizedTest + @MethodSource("buffers") + public void testAppendSmallByteBuffer(RetainableByteBuffer buffer) + { + byte[] bytes = new byte[] {'-', 'X', '-'}; + ByteBuffer from = ByteBuffer.wrap(bytes, 1, 1); + while (!buffer.isFull()) + { + ByteBuffer slice = from.slice(); + buffer.append(slice); + assertFalse(slice.hasRemaining()); + } + + assertThat(BufferUtil.toString(buffer.getByteBuffer()), is("X".repeat(buffer.capacity()))); + buffer.release(); + } + + @ParameterizedTest + @MethodSource("buffers") + public void testAppendBigByteBuffer(RetainableByteBuffer buffer) + { + ByteBuffer from = BufferUtil.toBuffer("X".repeat(buffer.capacity() * 2)); + buffer.append(from); + assertTrue(from.hasRemaining()); + assertThat(BufferUtil.toString(buffer.getByteBuffer()), is("X".repeat(buffer.capacity()))); + assertTrue(buffer.isFull()); + buffer.release(); + } + + @ParameterizedTest + @MethodSource("buffers") + public void testNonRetainableWriteTo(RetainableByteBuffer buffer) throws Exception + { + buffer.append(RetainableByteBuffer.wrap(BufferUtil.toBuffer("Hello"))); + buffer.append(RetainableByteBuffer.wrap(BufferUtil.toBuffer(" "))); + buffer.append(RetainableByteBuffer.wrap(BufferUtil.toBuffer("World!"))); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + FutureCallback callback = new FutureCallback(); + buffer.writeTo(Content.Sink.from(out), true, callback); + callback.get(5, TimeUnit.SECONDS); + assertThat(out.toString(StandardCharsets.ISO_8859_1), is("Hello World!")); + buffer.release(); + } + + @ParameterizedTest + @MethodSource("buffers") + public void testRetainableWriteTo(RetainableByteBuffer buffer) throws Exception + { + CountDownLatch released = new CountDownLatch(3); + RetainableByteBuffer[] buffers = new RetainableByteBuffer[3]; + buffer.append(buffers[0] = RetainableByteBuffer.wrap(BufferUtil.toBuffer("Hello"), released::countDown)); + buffer.append(buffers[1] = RetainableByteBuffer.wrap(BufferUtil.toBuffer(" "), released::countDown)); + buffer.append(buffers[2] = RetainableByteBuffer.wrap(BufferUtil.toBuffer("World!"), released::countDown)); + Arrays.asList(buffers).forEach(RetainableByteBuffer::release); + + ByteArrayOutputStream out = new ByteArrayOutputStream(); + FutureCallback callback = new FutureCallback(); + buffer.writeTo(Content.Sink.from(out), true, callback); + callback.get(5, TimeUnit.SECONDS); + assertThat(out.toString(StandardCharsets.ISO_8859_1), is("Hello World!")); + + buffer.release(); + assertTrue(released.await(5, TimeUnit.SECONDS)); + } + + @ParameterizedTest + @MethodSource("buffers") + public void testCopy(RetainableByteBuffer original) + { + original.append(ByteBuffer.wrap("hello".getBytes(StandardCharsets.UTF_8))); + RetainableByteBuffer copy = original.copy(); + + assertEquals(0, copy.space()); + assertEquals(5, copy.remaining()); + assertEquals(5, original.remaining()); + assertEquals("hello", StandardCharsets.UTF_8.decode(original.getByteBuffer()).toString()); + assertEquals("hello", StandardCharsets.UTF_8.decode(copy.getByteBuffer()).toString()); + + copy.release(); + original.release(); + } + + @ParameterizedTest + @MethodSource("buffers") + public void testCopyThenModifyOriginal(RetainableByteBuffer original) + { + original.append(ByteBuffer.wrap("hello".getBytes(StandardCharsets.UTF_8))); + RetainableByteBuffer copy = original.copy(); + original.append(ByteBuffer.wrap(" world".getBytes(StandardCharsets.UTF_8))); + + assertEquals(0, copy.space()); + assertEquals(5, copy.remaining()); + assertEquals(11, original.remaining()); + assertEquals("hello world", StandardCharsets.UTF_8.decode(original.getByteBuffer()).toString()); + assertEquals("hello", StandardCharsets.UTF_8.decode(copy.getByteBuffer()).toString()); + + copy.release(); + original.release(); + } +} diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/DetectorConnectionFactory.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/DetectorConnectionFactory.java index c68a69e6324c..d9a5e816f160 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/DetectorConnectionFactory.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/DetectorConnectionFactory.java @@ -155,7 +155,7 @@ public void onUpgradeTo(ByteBuffer byteBuffer) if (LOG.isDebugEnabled()) LOG.debug("Detector {} adopting {}", getProtocol(), BufferUtil.toDetailString(byteBuffer)); // Throws if the ByteBuffer to adopt is too big, but it is handled. - BufferUtil.append(_buffer.getByteBuffer(), byteBuffer); + _buffer.append(byteBuffer); } @Override diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ProxyConnectionFactory.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ProxyConnectionFactory.java index f8b747090417..d639139de71f 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ProxyConnectionFactory.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/ProxyConnectionFactory.java @@ -253,7 +253,7 @@ public void onUpgradeTo(ByteBuffer buffer) { if (LOG.isDebugEnabled()) LOG.debug("Proxy v1 copying unconsumed buffer {}", BufferUtil.toDetailString(buffer)); - BufferUtil.append(_buffer.getByteBuffer(), buffer); + _buffer.append(buffer); } /** @@ -476,7 +476,7 @@ public void onUpgradeTo(ByteBuffer buffer) { if (LOG.isDebugEnabled()) LOG.debug("Proxy v2 copying unconsumed buffer {}", BufferUtil.toDetailString(buffer)); - BufferUtil.append(_buffer.getByteBuffer(), buffer); + _buffer.append(buffer); } @Override diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/EventsHandler.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/EventsHandler.java index b338f5cb3bb6..4f31b702e3bc 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/EventsHandler.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/handler/EventsHandler.java @@ -106,7 +106,7 @@ private void notifyOnRequestRead(Request wrapped, Content.Chunk chunk) { try { - onRequestRead(wrapped, chunk == null ? null : chunk.asReadOnly()); + onRequestRead(wrapped, chunk); } catch (Throwable x) { diff --git a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java index fd6137db3eb4..d8159ddb7afc 100644 --- a/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java +++ b/jetty-core/jetty-server/src/main/java/org/eclipse/jetty/server/internal/HttpConnection.java @@ -333,7 +333,7 @@ public ByteBuffer onUpgradeFrom() @Override public void onUpgradeTo(ByteBuffer buffer) { - BufferUtil.append(getRequestBuffer(), buffer); + getRetainableRequestBuffer().append(buffer); } @Override @@ -356,11 +356,16 @@ void releaseRequestBuffer() } } - private ByteBuffer getRequestBuffer() + private RetainableByteBuffer getRetainableRequestBuffer() { if (_retainableByteBuffer == null) _retainableByteBuffer = _bufferPool.acquire(getInputBufferSize(), isUseInputDirectByteBuffers()); - return _retainableByteBuffer.getByteBuffer(); + return _retainableByteBuffer; + } + + private ByteBuffer getRequestBuffer() + { + return getRetainableRequestBuffer().getByteBuffer(); } public boolean isRequestBufferEmpty() diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java index 3d771910596d..6d7fce2c2b60 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/BufferUtil.java @@ -21,6 +21,7 @@ import java.nio.BufferOverflowException; import java.nio.ByteBuffer; import java.nio.ByteOrder; +import java.nio.ReadOnlyBufferException; import java.nio.channels.Channels; import java.nio.channels.FileChannel; import java.nio.channels.FileChannel.MapMode; @@ -429,41 +430,38 @@ public static boolean compact(ByteBuffer buffer) /** * Put data from one buffer into another, avoiding over/under flows * - * @param from Buffer to take bytes from in flush mode + * @param from Buffer to take bytes from in flush mode, whose position is modified with the bytes taken. * @param to Buffer to put bytes to in fill mode. * @return number of bytes moved + * @throws ReadOnlyBufferException if the buffer is read only */ - public static int put(ByteBuffer from, ByteBuffer to) + public static int put(ByteBuffer from, ByteBuffer to) throws ReadOnlyBufferException { - int put; - int remaining = from.remaining(); - if (remaining > 0) + int length = from.remaining(); + if (length == 0) + return 0; + + int space = to.remaining(); + if (space >= length) { - if (remaining <= to.remaining()) - { - to.put(from); - put = remaining; - from.position(from.limit()); - } - else if (from.hasArray()) - { - put = to.remaining(); - to.put(from.array(), from.arrayOffset() + from.position(), put); - from.position(from.position() + put); - } - else - { - put = to.remaining(); - ByteBuffer slice = from.slice(); - slice.limit(put); - to.put(slice); - from.position(from.position() + put); - } + to.put(from); + return length; + } + + if (from.hasArray()) + { + to.put(from.array(), from.arrayOffset() + from.position(), space); + from.position(from.position() + space); } else - put = 0; + { + ByteBuffer slice = from.slice(); + slice.limit(slice.position() + space); + to.put(slice); + from.position(from.position() + space); + } - return put; + return space; } /** @@ -474,8 +472,9 @@ else if (from.hasArray()) * @param off offset into byte * @param len length to append * @throws BufferOverflowException if unable to append buffer due to space limits + * @throws ReadOnlyBufferException if the buffer is read only */ - public static void append(ByteBuffer to, byte[] b, int off, int len) throws BufferOverflowException + public static void append(ByteBuffer to, byte[] b, int off, int len) throws BufferOverflowException, ReadOnlyBufferException { int pos = flipToFill(to); try @@ -494,8 +493,9 @@ public static void append(ByteBuffer to, byte[] b, int off, int len) throws Buff * @param to Buffer is flush mode * @param b bytes to append * @throws BufferOverflowException if unable to append buffer due to space limits + * @throws ReadOnlyBufferException if the buffer is read only */ - public static void append(ByteBuffer to, byte[] b) throws BufferOverflowException + public static void append(ByteBuffer to, byte[] b) throws BufferOverflowException, ReadOnlyBufferException { append(to, b, 0, b.length); } @@ -506,8 +506,9 @@ public static void append(ByteBuffer to, byte[] b) throws BufferOverflowExceptio * @param to Buffer is flush mode * @param s String to append as UTF8 * @throws BufferOverflowException if unable to append buffer due to space limits + * @throws ReadOnlyBufferException if the buffer is read only */ - public static void append(ByteBuffer to, String s) throws BufferOverflowException + public static void append(ByteBuffer to, String s) throws BufferOverflowException, ReadOnlyBufferException { byte[] b = s.getBytes(StandardCharsets.UTF_8); append(to, b, 0, b.length); @@ -519,8 +520,9 @@ public static void append(ByteBuffer to, String s) throws BufferOverflowExceptio * @param to Buffer is flush mode * @param b byte to append * @throws BufferOverflowException if unable to append buffer due to space limits + * @throws ReadOnlyBufferException if the buffer is read only */ - public static void append(ByteBuffer to, byte b) + public static void append(ByteBuffer to, byte b) throws BufferOverflowException, ReadOnlyBufferException { int pos = flipToFill(to); try @@ -536,11 +538,12 @@ public static void append(ByteBuffer to, byte b) /** * Appends a buffer to a buffer * - * @param to Buffer is flush mode - * @param b buffer to append - * @return The position of the valid data before the flipped position. + * @param to Buffer in flush mode, whose position will be incremented by the number of bytes appended + * @param b buffer to append to in flush mode, whose limit will be incremented by the number of bytes appended. + * @return The number of bytes appended. + * @throws ReadOnlyBufferException if the buffer is read only */ - public static int append(ByteBuffer to, ByteBuffer b) + public static int append(ByteBuffer to, ByteBuffer b) throws ReadOnlyBufferException { int pos = flipToFill(to); try @@ -686,6 +689,12 @@ else if (read < 0) } } + /** + * Write a {@link ByteBuffer} to an {@link OutputStream}, updating the position for the bytes written. + * @param buffer The buffer to write + * @param out The output stream + * @throws IOException if there was a problem writing. + */ public static void writeTo(ByteBuffer buffer, OutputStream out) throws IOException { if (buffer.hasArray()) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Jetty.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Jetty.java index d98fdb82ea74..b01d1c67a76b 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Jetty.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Jetty.java @@ -81,9 +81,8 @@ private static String formatTimestamp(String timestamp) { try { - if (StringUtil.isBlank(timestamp)) - return "unknown"; - long epochMillis = Long.parseLong(timestamp); + long epochMillis = (StringUtil.isBlank(timestamp) || timestamp.startsWith("$")) + ? System.currentTimeMillis() : Long.parseLong(timestamp); return Instant.ofEpochMilli(epochMillis).toString(); } catch (NumberFormatException e) diff --git a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Utf8StringBuilder.java b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Utf8StringBuilder.java index 77c17b673ffe..fb87917135d6 100644 --- a/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Utf8StringBuilder.java +++ b/jetty-core/jetty-util/src/main/java/org/eclipse/jetty/util/Utf8StringBuilder.java @@ -234,7 +234,11 @@ protected void bufferAppend(char c) protected void bufferReset() { + // If the buffer is much larger than necessary, trim it to empty + boolean trim = _buffer.capacity() > (_buffer.length() * 8); _buffer.setLength(0); + if (trim) + _buffer.trimToSize(); } public void appendByte(byte b) throws IOException diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/Parser.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/Parser.java index 7626f40a5960..dd0e4dc709f9 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/Parser.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/internal/Parser.java @@ -344,7 +344,7 @@ private Frame.Parsed parsePayload(ByteBuffer buffer) // No space in the buffer, so we have to copy the partial payload. // The size of this allocation is limited by the maxFrameSize. aggregate = bufferPool.acquire(payloadLength, false); - BufferUtil.append(aggregate.getByteBuffer(), buffer); + aggregate.append(buffer); return null; } @@ -376,7 +376,7 @@ private Frame.Parsed parsePayload(ByteBuffer buffer) if (available < expecting) { // not enough data to complete this frame, just copy it - BufferUtil.append(aggregate.getByteBuffer(), buffer); + aggregate.append(buffer); return null; } diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ByteArrayMessageSink.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ByteArrayMessageSink.java index 2d562c78fbf7..2b57b9b36911 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ByteArrayMessageSink.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ByteArrayMessageSink.java @@ -17,8 +17,10 @@ import java.lang.invoke.MethodType; import java.nio.ByteBuffer; -import org.eclipse.jetty.io.ByteBufferCallbackAccumulator; +import org.eclipse.jetty.io.Content; +import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.ByteArrayOutputStream2; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.websocket.core.CoreSession; import org.eclipse.jetty.websocket.core.Frame; @@ -32,7 +34,7 @@ */ public class ByteArrayMessageSink extends AbstractMessageSink { - private ByteBufferCallbackAccumulator accumulator; + private RetainableByteBuffer.Accumulator accumulator; /** * Creates a new {@link ByteArrayMessageSink}. @@ -56,7 +58,7 @@ public void accept(Frame frame, Callback callback) { try { - long size = (accumulator == null ? 0 : accumulator.getLength()) + frame.getPayloadLength(); + long size = (accumulator == null ? 0 : accumulator.remaining()) + frame.getPayloadLength(); long maxSize = getCoreSession().getMaxBinaryMessageSize(); if (maxSize > 0 && size > maxSize) { @@ -67,7 +69,7 @@ public void accept(Frame frame, Callback callback) // If the frame is fin and no accumulator has been // created or used, then we don't need to aggregate. ByteBuffer payload = frame.getPayload(); - if (frame.isFin() && (accumulator == null || accumulator.getLength() == 0)) + if (frame.isFin() && (accumulator == null || accumulator.remaining() == 0)) { byte[] buf = BufferUtil.toArray(payload); getMethodHandle().invoke(buf, 0, buf.length); @@ -84,14 +86,20 @@ public void accept(Frame frame, Callback callback) } if (accumulator == null) - accumulator = new ByteBufferCallbackAccumulator(); - accumulator.addEntry(payload, callback); + accumulator = new RetainableByteBuffer.Accumulator(getCoreSession().getByteBufferPool(), false, maxSize); + RetainableByteBuffer wrappedPayload = RetainableByteBuffer.wrap(payload, callback::succeeded); + accumulator.append(wrappedPayload); + wrappedPayload.release(); if (frame.isFin()) { // Do not complete twice the callback if the invocation fails. callback = Callback.NOOP; - byte[] buf = accumulator.takeByteArray(); + + ByteArrayOutputStream2 bout = new ByteArrayOutputStream2(accumulator.remaining()); + accumulator.writeTo(Content.Sink.from(bout), true, Callback.NOOP); + byte[] buf = bout.getBuf(); + accumulator.release(); getMethodHandle().invoke(buf, 0, buf.length); autoDemand(); } @@ -111,6 +119,6 @@ public void accept(Frame frame, Callback callback) public void fail(Throwable failure) { if (accumulator != null) - accumulator.fail(failure); + accumulator.clear(); } } diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ByteBufferMessageSink.java b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ByteBufferMessageSink.java index 2783387b3381..30fd1bc26eb0 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ByteBufferMessageSink.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-common/src/main/java/org/eclipse/jetty/websocket/core/messages/ByteBufferMessageSink.java @@ -17,8 +17,6 @@ import java.lang.invoke.MethodType; import java.nio.ByteBuffer; -import org.eclipse.jetty.io.ByteBufferCallbackAccumulator; -import org.eclipse.jetty.io.ByteBufferPool; import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.util.Callback; import org.eclipse.jetty.websocket.core.CoreSession; @@ -33,7 +31,7 @@ */ public class ByteBufferMessageSink extends AbstractMessageSink { - private ByteBufferCallbackAccumulator accumulator; + private RetainableByteBuffer.Accumulator accumulator; /** * Creates a new {@link ByteBufferMessageSink}. @@ -64,7 +62,7 @@ public void accept(Frame frame, Callback callback) { try { - long size = (accumulator == null ? 0 : accumulator.getLength()) + frame.getPayloadLength(); + int size = Math.addExact(accumulator == null ? 0 : Math.toIntExact(accumulator.remaining()), frame.getPayloadLength()); long maxSize = getCoreSession().getMaxBinaryMessageSize(); if (maxSize > 0 && size > maxSize) { @@ -74,7 +72,7 @@ public void accept(Frame frame, Callback callback) // If the frame is fin and no accumulator has been // created or used, then we don't need to aggregate. - if (frame.isFin() && (accumulator == null || accumulator.getLength() == 0)) + if (frame.isFin() && (accumulator == null || accumulator.remaining() == 0)) { invoke(getMethodHandle(), frame.getPayload(), callback); autoDemand(); @@ -89,17 +87,15 @@ public void accept(Frame frame, Callback callback) } if (accumulator == null) - accumulator = new ByteBufferCallbackAccumulator(); - accumulator.addEntry(frame.getPayload(), callback); + accumulator = new RetainableByteBuffer.Accumulator(getCoreSession().getByteBufferPool(), false, maxSize); + RetainableByteBuffer wrappedPayload = RetainableByteBuffer.wrap(frame.getPayload(), callback::succeeded); + accumulator.append(wrappedPayload); + wrappedPayload.release(); if (frame.isFin()) { - ByteBufferPool bufferPool = getCoreSession().getByteBufferPool(); - RetainableByteBuffer buffer = bufferPool.acquire(accumulator.getLength(), false); - ByteBuffer byteBuffer = buffer.getByteBuffer(); - accumulator.writeTo(byteBuffer); - callback = Callback.from(buffer::release); - invoke(getMethodHandle(), byteBuffer, callback); + callback = Callback.from(accumulator::release); + invoke(getMethodHandle(), accumulator.getByteBuffer(), callback); autoDemand(); } else @@ -119,7 +115,7 @@ public void accept(Frame frame, Callback callback) public void fail(Throwable failure) { if (accumulator != null) - accumulator.fail(failure); + accumulator.clear(); } protected void invoke(MethodHandle methodHandle, ByteBuffer byteBuffer, Callback callback) throws Throwable diff --git a/jetty-core/jetty-websocket/jetty-websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/extensions/PermessageDeflateDemandTest.java b/jetty-core/jetty-websocket/jetty-websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/extensions/PermessageDeflateDemandTest.java index ca91ccfbd521..602666250d1f 100644 --- a/jetty-core/jetty-websocket/jetty-websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/extensions/PermessageDeflateDemandTest.java +++ b/jetty-core/jetty-websocket/jetty-websocket-core-tests/src/test/java/org/eclipse/jetty/websocket/core/extensions/PermessageDeflateDemandTest.java @@ -19,12 +19,14 @@ import java.util.concurrent.BlockingQueue; import java.util.concurrent.TimeUnit; -import org.eclipse.jetty.io.ByteBufferCallbackAccumulator; +import org.eclipse.jetty.io.ArrayByteBufferPool; +import org.eclipse.jetty.io.RetainableByteBuffer; import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.util.BlockingArrayQueue; import org.eclipse.jetty.util.BufferUtil; import org.eclipse.jetty.util.Callback; +import org.eclipse.jetty.util.component.LifeCycle; import org.eclipse.jetty.websocket.core.CloseStatus; import org.eclipse.jetty.websocket.core.CoreSession; import org.eclipse.jetty.websocket.core.Frame; @@ -40,12 +42,14 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; public class PermessageDeflateDemandTest { private Server _server; + private ArrayByteBufferPool.Tracking _bufferPool; private WebSocketCoreClient _client; private ServerConnector _connector; private WebSocketUpgradeHandler _upgradeHandler; @@ -53,7 +57,8 @@ public class PermessageDeflateDemandTest @BeforeEach public void before() throws Exception { - _server = new Server(); + _bufferPool = new ArrayByteBufferPool.Tracking(); + _server = new Server(null, null, _bufferPool); _connector = new ServerConnector(_server); _server.addConnector(_connector); @@ -68,8 +73,15 @@ public void before() throws Exception @AfterEach public void after() throws Exception { - _client.stop(); - _server.stop(); + try + { + assertThat("Detected leaks: " + _bufferPool.dumpLeaks(), _bufferPool.getLeaks().size(), is(0)); + } + finally + { + LifeCycle.stop(_client); + LifeCycle.stop(_server); + } } @Test @@ -113,12 +125,13 @@ public static class ServerHandler implements FrameHandler public BlockingQueue textMessages = new BlockingArrayQueue<>(); public BlockingQueue binaryMessages = new BlockingArrayQueue<>(); private StringBuilder _stringBuilder = new StringBuilder(); - private ByteBufferCallbackAccumulator _byteBuilder = new ByteBufferCallbackAccumulator(); + private RetainableByteBuffer.Accumulator _byteBuilder; @Override public void onOpen(CoreSession coreSession, Callback callback) { _coreSession = coreSession; + _byteBuilder = new RetainableByteBuffer.Accumulator(_coreSession.getByteBufferPool(), false, -1); callback.succeeded(); coreSession.demand(); } @@ -154,11 +167,14 @@ public void onFrame(Frame frame, Callback callback) } break; case OpCode.BINARY: - _byteBuilder.addEntry(frame.getPayload(), callback); + RetainableByteBuffer wrappedPayload = RetainableByteBuffer.wrap(frame.getPayload(), callback::succeeded); + _byteBuilder.append(wrappedPayload); + wrappedPayload.release(); if (frame.isFin()) { - binaryMessages.add(BufferUtil.toBuffer(_byteBuilder.takeByteArray())); - _byteBuilder = new ByteBufferCallbackAccumulator(); + // TODO this looks wrong + binaryMessages.add(ByteBuffer.wrap(BufferUtil.toArray(_byteBuilder.getByteBuffer()))); + _byteBuilder.clear(); } break; default: @@ -177,12 +193,16 @@ public void onFrame(Frame frame, Callback callback) public void onError(Throwable cause, Callback callback) { cause.printStackTrace(); + if (_byteBuilder != null) + _byteBuilder.clear(); callback.succeeded(); } @Override public void onClosed(CloseStatus closeStatus, Callback callback) { + if (_byteBuilder != null) + _byteBuilder.clear(); callback.succeeded(); } } diff --git a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ResponseHeadersTest.java b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ResponseHeadersTest.java index 2c4f0e6e6542..fd5848e70d0d 100644 --- a/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ResponseHeadersTest.java +++ b/jetty-ee10/jetty-ee10-servlet/src/test/java/org/eclipse/jetty/ee10/servlet/ResponseHeadersTest.java @@ -14,6 +14,7 @@ package org.eclipse.jetty.ee10.servlet; import java.io.IOException; +import java.io.OutputStream; import java.io.PrintWriter; import java.net.URLDecoder; import java.nio.ByteBuffer; @@ -153,6 +154,44 @@ protected void doGet(HttpServletRequest req, HttpServletResponse response) throw assertThat("Response Header X-example", response.get("X-Example"), is(expected)); } + @Test + public void testCharsetAfterGetOutputStream() throws Exception + { + ServletContextHandler contextHandler = new ServletContextHandler(); + contextHandler.setContextPath("/"); + + HttpServlet charsetResetToJsonMimeTypeServlet = new HttpServlet() + { + @Override + protected void doGet(HttpServletRequest request, HttpServletResponse response) throws IOException + { + OutputStream out = response.getOutputStream(); + response.setCharacterEncoding("utf-8"); + response.setContentType("text/html"); + out.write("hello".getBytes(StandardCharsets.UTF_8)); + } + }; + + contextHandler.addServlet(charsetResetToJsonMimeTypeServlet, "/charset/hello/*"); + startServer(contextHandler); + + HttpTester.Request request = new HttpTester.Request(); + request.setMethod("GET"); + request.setURI("/charset/hello/"); + request.setVersion(HttpVersion.HTTP_1_1); + request.setHeader("Connection", "close"); + request.setHeader("Host", "test"); + + ByteBuffer responseBuffer = connector.getResponse(request.generate()); + // System.err.println(BufferUtil.toUTF8String(responseBuffer)); + HttpTester.Response response = HttpTester.parseResponse(responseBuffer); + + // Now test for properly formatted HTTP Response Headers. + assertThat("Response Code", response.getStatus(), is(200)); + // The Content-Type should not have a charset= portion + assertThat("Response Header Content-Type", response.get("Content-Type"), is("text/html;charset=UTF-8")); + } + @Test public void testCharsetResetToJsonMimeType() throws Exception { diff --git a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/BufferedResponseHandler.java b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/BufferedResponseHandler.java index 61fa4cd6738f..9f4b649045bb 100644 --- a/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/BufferedResponseHandler.java +++ b/jetty-ee9/jetty-ee9-nested/src/main/java/org/eclipse/jetty/ee9/nested/BufferedResponseHandler.java @@ -205,7 +205,7 @@ class ArrayBufferedInterceptor implements BufferedInterceptor private final HttpChannel _channel; private final Queue _buffers = new ArrayDeque<>(); private Boolean _aggregating; - private ByteBuffer _aggregate; + private ByteBuffer _aggregate; // TODO use RetainableByteBuffer.Aggregator public ArrayBufferedInterceptor(HttpChannel httpChannel, Interceptor interceptor) { diff --git a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee9/websocket/tests/ClientDisconnectTest.java b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee9/websocket/tests/ClientDisconnectTest.java index 97909fb5c8a4..9ad1b890e400 100644 --- a/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee9/websocket/tests/ClientDisconnectTest.java +++ b/jetty-ee9/jetty-ee9-websocket/jetty-ee9-websocket-jetty-tests/src/test/java/org/eclipse/jetty/ee9/websocket/tests/ClientDisconnectTest.java @@ -29,12 +29,13 @@ import org.eclipse.jetty.server.Server; import org.eclipse.jetty.server.ServerConnector; import org.eclipse.jetty.util.BufferUtil; +import org.eclipse.jetty.util.component.LifeCycle; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import static org.hamcrest.MatcherAssert.assertThat; -import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; @@ -44,6 +45,7 @@ public class ClientDisconnectTest private final Duration _serverIdleTimeout = Duration.ofSeconds(5); private final int _messageSize = 5 * 1024 * 1024; private Server _server; + private ArrayByteBufferPool.Tracking _bufferPool; private ServerConnector _connector; private WebSocketClient _client; @@ -62,7 +64,8 @@ public void onOpen(Session session) public void before() throws Exception { _client = new WebSocketClient(); - _server = new Server(); + _bufferPool = new ArrayByteBufferPool.Tracking(); + _server = new Server(null, null, _bufferPool); _connector = new ServerConnector(_server); _server.addConnector(_connector); @@ -82,8 +85,15 @@ public void before() throws Exception @AfterEach public void after() throws Exception { - _client.stop(); - _server.stop(); + try + { + assertThat("Detected leaks: " + _bufferPool.dumpLeaks(), _bufferPool.getLeaks().size(), is(0)); + } + finally + { + LifeCycle.stop(_client); + LifeCycle.stop(_server); + } } @Test @@ -105,10 +115,5 @@ public void testBuffersAfterIncompleteMessage() throws Exception // Wait for the server to close its session. assertTrue(serverSocket.closeLatch.await(_serverIdleTimeout.toSeconds() + 1, TimeUnit.SECONDS)); - - // We should have no buffers still used in the pool. - ArrayByteBufferPool bufferPool = _server.getBean(ArrayByteBufferPool.class); - assertThat(bufferPool.getDirectByteBufferCount() - bufferPool.getAvailableDirectByteBufferCount(), equalTo(0L)); - assertThat(bufferPool.getHeapByteBufferCount() - bufferPool.getAvailableHeapByteBufferCount(), equalTo(0L)); } }