From f0ee2e146750291f2acd908f9b1ad6748688144b Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Wed, 26 May 2021 17:13:29 +0200 Subject: [PATCH 01/12] Remove `acquire` from the public API This is a step toward effectively eliminating reference counting. Reference counting is only needed when the memory in buffers can be shared. If we remove all forms of sharing, then the buffers would be in an owned state at all times. Then we would no longer need to worry about the state of the buffers before calling, e.g. `ensureWritable` and methods like that. Just removing `acquire` is not enough; we also need to remove the `slice` method. In this commit we are, however, starting with `acquire` because doing so requires rearranging the type hierarchy and the generics we have in play. This was not an easy exercise, but for the purpose of record keeping, it's useful to have that work separate from the work of removing `slice`. --- RATIONALE.adoc | 84 +++------ .../main/java/io/netty/buffer/api/Buffer.java | 61 ++----- .../io/netty/buffer/api/BufferAllocator.java | 4 +- .../io/netty/buffer/api/BufferHolder.java | 69 ++------ .../java/io/netty/buffer/api/BufferRef.java | 20 +-- .../io/netty/buffer/api/CompositeBuffer.java | 131 +++++--------- .../main/java/io/netty/buffer/api/Drop.java | 10 +- .../main/java/io/netty/buffer/api/Owned.java | 16 +- .../src/main/java/io/netty/buffer/api/Rc.java | 80 --------- .../api/ReadableComponentProcessor.java | 6 +- .../java/io/netty/buffer/api/Resource.java | 41 +++++ .../main/java/io/netty/buffer/api/Scope.java | 12 +- .../main/java/io/netty/buffer/api/Send.java | 43 ++--- .../buffer/api/adaptor/ByteBufAdaptor.java | 16 +- .../buffer/api/bytebuffer/NioBuffer.java | 4 +- .../api/{ => internal}/LifecycleTracer.java | 38 ++-- .../ResourceSupport.java} | 49 ++++-- .../io/netty/buffer/api/internal/Statics.java | 8 + .../api/{ => internal}/TransferSend.java | 11 +- .../netty/buffer/api/unsafe/UnsafeBuffer.java | 4 +- buffer-api/src/main/java/module-info.java | 8 +- .../netty/buffer/api/memseg/MemSegBuffer.java | 6 +- .../api/tests/BufferBulkAccessTest.java | 16 +- .../buffer/api/tests/BufferCompactTest.java | 3 +- .../tests/BufferComponentIterationTest.java | 12 +- .../api/tests/BufferCompositionTest.java | 166 +++++++++--------- .../api/tests/BufferEnsureWritableTest.java | 2 +- .../buffer/api/tests/BufferReadOnlyTest.java | 11 +- .../netty/buffer/api/tests/BufferRefTest.java | 36 ++-- .../tests/BufferReferenceCountingTest.java | 52 +++--- .../buffer/api/tests/BufferSendTest.java | 7 +- .../buffer/api/tests/BufferTestSupport.java | 17 +- .../io/netty/buffer/api/tests/ScopeTest.java | 20 +-- .../benchmarks/ByteIterationBenchmark.java | 4 +- .../examples/ComposingAndSlicingExample.java | 12 +- .../api/tests/examples/SendExample.java | 45 +---- .../AlternativeMessageDecoder.java | 16 +- .../ByteToMessageDecoder.java | 10 +- .../ByteToMessageDecoderTest.java | 9 +- 39 files changed, 467 insertions(+), 692 deletions(-) delete mode 100644 buffer-api/src/main/java/io/netty/buffer/api/Rc.java create mode 100644 buffer-api/src/main/java/io/netty/buffer/api/Resource.java rename buffer-api/src/main/java/io/netty/buffer/api/{ => internal}/LifecycleTracer.java (82%) rename buffer-api/src/main/java/io/netty/buffer/api/{RcSupport.java => internal/ResourceSupport.java} (74%) rename buffer-api/src/main/java/io/netty/buffer/api/{ => internal}/TransferSend.java (84%) diff --git a/RATIONALE.adoc b/RATIONALE.adoc index 58af8df..d212391 100644 --- a/RATIONALE.adoc +++ b/RATIONALE.adoc @@ -34,7 +34,7 @@ We have been collaborating with the panama-foreign project, providing feedback t Our new buffer API is being designed with a future in mind, where access to Unsafe and JNI, is no longer possible. This is, however, not the implementation we are going to provide at first. The APIs from panama-foreign are still not finished, and likely won’t be in time for the release of JDK 17. -With this in mind, Netty 5 will likely baseline on Java 11 anyway. +With this in mind, Netty 5 will baseline on Java 11. == Where we are going @@ -71,32 +71,15 @@ Hopefully you’ll be able to see these principles reflected in the new API. In this section we’ll outline the major changes, and most prominent points of interest in the new API. -=== An explicit concept of ownership - -In the new API we make it explicit that a buffer can have an “owner”. -This is the case when the buffer isn’t borrowed out anywhere else; the reference count is one. -In this state the buffer is said to be “owned”. -This is important because certain operations can only be called when the buffer is in an owned state, as they could otherwise violate our safety requirements. -All of these operations will call this out in their javadocs. -These operations either change the ownership of the buffer, or they change the internal memory allocation of the buffer, in some way. -For instance, `ensureWritable()` requires ownership because it may replace the internal memory allocation with a new larger one. -The `split()` method requires ownership because it changes how the buffer ownership itself is set up. -The `send()` method is used for transferring ownership between threads, which obviously requires ownership to begin with. - -[source,java] ----- -if (buf.isOwned()) { - buf.compact(); // compact() requires ownership because it moves data. -} ----- - === Reference counting Buffers are now `AutoCloseable` and can be used in try-with-resources clauses. -Every allocation and acquire call on a buffer (any `Rc` object, really) must be paired with a `close()`, and every `receive()` call on a `Send` must also be paired with a `close()`. -Every method that increments the reference count will document this fact. -When the reference count is not incremented, then there is obviously no need to decrement it either. -Buffers can thus be passed through a pipeline without having to do much reference counting work. +Every allocation and acquire call on a buffer (any `Resource` object, really) must be paired with a `close()`, and every `receive()` call on a `Send` must also be paired with a `close()`. + +While referene counting is a useful thing for tracking resource life-cycles internally, it is not itself exposed in the public API. +Instead, the public API effectively has a boolean open/closed state. +This simplifies the API a great deal; buffers are created, and in the end they are closed. +The code in between needs to be arranged such that it just avoids ever holding on to buffers that might be closed. [source,java] ---- @@ -105,15 +88,13 @@ try (Buffer buf = allocator.allocate(8)) { } // buf is deallocated here. ---- -The updates to the reference counts are not thread-safe, because the buffers themselves - their contents and their offsets - are not thread-safe. +The change of the open/closed state is not thread-safe, because the buffers themselves - their contents and their offsets - are not thread-safe. This is a deviation from how ByteBuf works, where the updates are atomic, and the reference count checks on memory accesses are “optimistic” in that they permit data races to occur. This codifies that buffers cannot be modified by more than one thread at a time, and that buffers should be shared via safe publication. Using the `send()` mechanism helps with the thread-safe transfer of buffer ownership. A buffers contents can still be access from multiple threads via the `get*` methods. However, the buffer should be effectively read-only while it is exposed like that, as accesses would otherwise be racy. -See https://github.com/netty/netty-incubator-buffer-api/blob/main/src/main/java/io/netty/buffer/api/Rc.java - If these simple rules and patterns are followed strictly, then memory leaks should not occur. === Cleaner attached by default @@ -126,20 +107,13 @@ Note, however, that the buffers are still reference counted, because this has mo Off-heap (or direct) buffers can give the GC an inaccurate picture of memory usage, which in turn can lead to abrupt bouts of poor performance when the system is under load. The cleaner is a fall back that will likely also be used as part of leak detection. -=== Slices are always retained +=== Slices are gone -The existing ByteBuf API has both slice() and retainedSlice() methods, where the latter increments the reference count of the parent buffer, and the former does not. -In the new API the slice() method always increments the reference count of the buffer being sliced. -The slice itself has its own independent positions, and its own reference count. - -[source,java] ----- -try (Buffer slice = buf.slice()) { - // process slice of readable data. -} ----- - -There is currently no duplicate() methods on the API, because it is not clear if they are really needed, but if we were to add them, they would work in the same way as slice() does. +The existing ByteBuf API has a number of methods that allow multiple buffers to share access to the same memory. +It turns out that this capability is at the heart of why reference counting is a necessary part of the ByteBuf API. +By removing the various `slice()` and `duplicate()` methods, along with the `retain()`/`release()` family of methods, we also remove the ability for buffers to share memory. +This allows us to simplify the reference counting concept to a simple boolean open/closed state. +Buffers are created, and at the end of their life, they are closed, which releases their memory back to where it came from. === Buffer interface @@ -229,24 +203,14 @@ That’s why the method to compose buffers takes a `BufferAllocator` as a first ---- try (Buffer x = allocator.allocate(128); Buffer y = allocator.allocate(128)) { - return CompositeBuffer.compose(allocator, x, y); + return CompositeBuffer.compose(allocator, x.send(), y.send()); } ---- The static `compose()` method will create a composite buffer, even when only given a single buffer, or no buffers. -The composite buffer acquires a reference on each of its constituent component buffers. -This means that, for instance, newly allocated buffers will not be owned by the composite buffer unless the reference outside of the composite buffer is closed. -In the above example, the reference counts for the buffers x and y are initially 1, then gets incremented to 2 by creating the composite buffer, and it drops back down to 1 at the end of the try-with-resources clause. -When the method returns, the composite buffer will be the only thing holding on to the two buffers, and it will thus have ownership over them. - -A composite buffer can only be owned if all of its constituent buffers are owned. -Conversely, by acquiring a reference to each constituent component buffer, the composite buffer prevents them from being owned elsewhere. -This is important because buffers cannot change their size, or transfer their ownership elsewhere, unless they are already owned. -If the constituent component buffers of a composite buffer could change their size, they would be able to break the offset computations inside of the composite buffer, and break the illusion that the composite buffer is just like one large buffer. - -A composite buffer can also be composed out of `Send` instances. -This ensures the composite buffer gets an exclusive reference to the sent components. +The composite buffer takes ownership of each of its constituent component buffers, via the `Send` arguments. +This guarantees that the composite cannot be brought into a state that is invalid, through direct manipulation of its components. Although there is in principle is no need for integrating code to know whether a buffer is composite, it is still possible to query, in case it is helpful for some optimisations. This is done with the `countComponents()`, `countReadableComponents()`, and `countWritableComponents()` family of methods. @@ -260,10 +224,6 @@ That is, you can pass composite buffers to the `CompositeBuffer.compose()` metho However, the new composite buffer will end up with the flattened concatenation of all constituent components. This means the number of indirections will not increase in the new buffer. -Because the new composite buffer increases the reference counts on all of its components, and because a composite buffer can only be owned when all of its constituent components are owned, the ownership model ends up working just fine with this flattening. -This also means that a composite buffer that is composed of other composite buffers, do not increase the reference counts of those other composite buffers – only their components have their reference counts increased. -This won’t make any difference in how the buffers behave, but it may cause some surprises to the few who are inspecting the `countBorrows()`. - === Iterating components The `forEachReadable()` and `forEachWritable()` methods iterate a buffers readable and writable areas, respectively. @@ -354,15 +314,15 @@ The MemorySegment APIs that are being developed in the OpenJDK project will use The `get`/`set`/`read`/`writeBoolean` accessor methods are being removed with no replacement planned. They have ambiguous meaning when working with buffers that are fundamentally byte-granular. -=== Splitting buffer ownership with split() +=== Splitting buffers with split() -The more explicit concept of ownership, and how ownership is now a requirement for calling some Buffer methods, may get in the way in some cases. -For instance, in Netty, the `ByteToMessageDecoder` collects data into a collecting buffer, from which data frames are sliced off and then sent off to be processed in parallel in other threads. +With the removal of the `slice()` family of methods, we are in need of an alternative way to process a buffer in parts. +For instance, in Netty, the `ByteToMessageDecoder` collects data into a collecting buffer, from which data frames are produced and then sent off to be processed further down a pipeline, potentially in parallel in other threads. -Since slices are now always retaining, they would effectively lock out all methods that require ownership. +Since slices would cause memory to be shared, they would effectively lock out all methods that require ownership. This would be a problem for such a collecting buffer, since it needs to grow dynamically to accommodate the largest message or frame size. -To address this, the new API introduces a `Buffer.split()` (https://github.com/netty/netty-incubator-buffer-api/blob/main/src/main/java/io/netty/buffer/api/Buffer.java#L481) method. +To address this, the new API introduces a `Buffer.split()` (https://github.com/netty/netty-incubator-buffer-api/blob/main/src/main/java/io/netty/buffer/api/Buffer.java#L529) method. This method splits the ownership of a buffer in two. All the read and readable bytes are returned in a new, independent buffer, and the existing buffer gets truncated at the head by a corresponding amount. The capacities and offsets of both buffers are adjusted such that they cannot access each others memory. diff --git a/buffer-api/src/main/java/io/netty/buffer/api/Buffer.java b/buffer-api/src/main/java/io/netty/buffer/api/Buffer.java index 8105b33..4b1b6bf 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/Buffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/Buffer.java @@ -32,43 +32,17 @@ import java.nio.ByteOrder; *

Life cycle and reference counting

* * The buffer has a life cycle, where it is allocated, used, and deallocated. - * The reference count controls this life cycle. - *

* When the buffer is initially allocated, a pairing {@link #close()} call will deallocate it. - * In this state, the buffer {@linkplain #isOwned() is "owned"}. - *

- * The buffer can also be {@linkplain #acquire() acquired} when it's about to be involved in a complicated lifetime. - * The {@link #acquire()} call increments the reference count of the buffer, - * and a pairing {@link #close()} call will decrement the reference count. - * Each acquire lends out the buffer, and the buffer is said to be in a "borrowed" state. - *

- * Certain operations, such as {@link #send()}, are only available on owned buffers. + * If a buffer is {@linkplain #send() sent} elsewhere, the {@linkplain #close() close} method on the given instance + * will become a no-op. + * The buffer can be thought of as a view onto memory, and calling {@link #send()} on the buffer will effectively close + * that view, and recreate it upon reception at its destination. * *

Thread-safety

* * Buffers are not thread-safe. - * The reference counting implied by the {@link Rc} interface is itself not thread-safe, - * and buffers additionally contain other mutable data that is not thread-safe. - * Depending on the buffer implementation, the buffer may impose confinement restrictions as well, - * so that the buffer cannot even be read using absolute offsets, - * such as with the {@link #getByte(int)} method, - * from multiple threads. - *

- * If a buffer needs to be accessed by a different thread, - * then the ownership of that buffer must be sent to that thread. - * This can be done with the {@link #send()} method. - * The send method consumes the buffer, if it is in an owned state, and produces a {@link Send} object. - * The {@link Send} object can then be shared in a thread-safe way (so-called "safe publication"), - * with the intended recipient thread. - *

- * To send a buffer to another thread, the buffer must not have any outstanding borrows. - * That is to say, all {@linkplain #acquire() acquires} must have been paired with a {@link #close()}; - * all {@linkplain #slice() slices} must have been closed. - * And if this buffer is a constituent of a - * {@linkplain CompositeBuffer#compose(BufferAllocator, Buffer...) composite buffer}, then that composite buffer must - * be closed. - * And if this buffer is itself a composite buffer, then it must own all of its constituent buffers. - * The {@link #isOwned()} method can be used on any buffer to check if it can be sent or not. + * The {@linkplain #isAccessible() accessibility state} implied by the {@link Resource} interface is itself not + * thread-safe, and buffers additionally contain other mutable data that is not thread-safe. * *

Accessing data

* @@ -137,7 +111,7 @@ import java.nio.ByteOrder; * The {@link BufferAllocator} has a {@link BufferAllocator#constBufferSupplier(byte[])} method that solves this, and * prevents these bugs from occurring. */ -public interface Buffer extends Rc, BufferAccessors { +public interface Buffer extends Resource, BufferAccessors { /** * Change the default byte order of this buffer, and return this buffer. * @@ -412,7 +386,6 @@ public interface Buffer extends Rc, BufferAccessors { /** * Ensure that this buffer has {@linkplain #writableBytes() available space for writing} the given number of * bytes. - * The buffer must be in {@linkplain #isOwned() an owned state}, or an exception will be thrown. * If this buffer already has the necessary space, then this method returns immediately. * If this buffer does not already have the necessary space, then it will be expanded using the * {@link BufferAllocator} the buffer was created with. @@ -420,8 +393,7 @@ public interface Buffer extends Rc, BufferAccessors { * {@code false}. * * @param size The requested number of bytes of space that should be available for writing. - * @throws IllegalStateException if this buffer is not in an {@linkplain #isOwned() owned} state, - * or is {@linkplain #readOnly() read-only}. + * @throws IllegalStateException if this buffer is not in a bad state, or is {@linkplain #readOnly() read-only}. */ default void ensureWritable(int size) { ensureWritable(size, 1, true); @@ -430,7 +402,6 @@ public interface Buffer extends Rc, BufferAccessors { /** * Ensure that this buffer has {@linkplain #writableBytes() available space for writing} the given number of * bytes. - * The buffer must be in {@linkplain #isOwned() an owned state}, or an exception will be thrown. * If this buffer already has the necessary space, then this method returns immediately. * If this buffer does not already have the necessary space, then space will be made available in one or all of * the following available ways: @@ -463,8 +434,7 @@ public interface Buffer extends Rc, BufferAccessors { * @param allowCompaction {@code true} if the method is allowed to modify the * {@linkplain #readerOffset() reader offset} and * {@linkplain #writerOffset() writer offset}, otherwise {@code false}. - * @throws IllegalStateException if this buffer is not in an {@linkplain #isOwned() owned} state, - * or is {@linkplain #readOnly() read-only}. + * @throws IllegalStateException if this buffer is not in a bad state, or is {@linkplain #readOnly() read-only}. */ void ensureWritable(int size, int minimumGrowth, boolean allowCompaction); @@ -514,8 +484,6 @@ public interface Buffer extends Rc, BufferAccessors { /** * Split the buffer into two, at the {@linkplain #writerOffset() write offset} position. *

- * The buffer must be in {@linkplain #isOwned() an owned state}, or an exception will be thrown. - *

* The region of this buffer that contain the read and readable bytes, will be captured and returned in a new * buffer, that will hold its own ownership of that region. This allows the returned buffer to be independently * {@linkplain #send() sent} to other threads. @@ -565,8 +533,6 @@ public interface Buffer extends Rc, BufferAccessors { /** * Split the buffer into two, at the given {@code splitOffset}. *

- * The buffer must be in {@linkplain #isOwned() an owned state}, or an exception will be thrown. - *

* The region of this buffer that precede the {@code splitOffset}, will be captured and returned in a new * buffer, that will hold its own ownership of that region. This allows the returned buffer to be independently * {@linkplain #send() sent} to other threads. @@ -615,8 +581,7 @@ public interface Buffer extends Rc, BufferAccessors { /** * Discards the read bytes, and moves the buffer contents to the beginning of the buffer. * - * @throws IllegalStateException if this buffer is not in an {@linkplain #isOwned() owned} state, - * or is {@linkplain #readOnly() read-only}. + * @throws IllegalStateException if this buffer is not in a bad state, or is {@linkplain #readOnly() read-only}. */ void compact(); @@ -675,7 +640,8 @@ public interface Buffer extends Rc, BufferAccessors { *

* The {@link ByteBuffer} instances obtained from the component, share lifetime with that internal component. * This means they can be accessed as long as the internal memory store remain unchanged. Methods that may cause - * such changes, are any method that requires the buffer to be {@linkplain #isOwned() owned}. + * such changes are {@link #split(int)}, {@link #split()}, {@link #compact()}, {@link #ensureWritable(int)}, + * {@link #ensureWritable(int, int, boolean)}, and {@link #send()}. *

* The best way to ensure this doesn't cause any trouble, is to use the buffers directly as part of the iteration, * or immediately after the iteration while we are still in the scope of the method that triggered the iteration. @@ -717,7 +683,8 @@ public interface Buffer extends Rc, BufferAccessors { *

* The {@link ByteBuffer} instances obtained from the component, share lifetime with that internal component. * This means they can be accessed as long as the internal memory store remain unchanged. Methods that may cause - * such changes, are any method that requires the buffer to be {@linkplain #isOwned() owned}. + * such changes are {@link #split(int)}, {@link #split()}, {@link #compact()}, {@link #ensureWritable(int)}, + * {@link #ensureWritable(int, int, boolean)}, and {@link #send()}. *

* The best way to ensure this doesn't cause any trouble, is to use the buffers directly as part of the iteration, * or immediately after the iteration while we are still in the scope of the method that triggered the iteration. diff --git a/buffer-api/src/main/java/io/netty/buffer/api/BufferAllocator.java b/buffer-api/src/main/java/io/netty/buffer/api/BufferAllocator.java index 184c154..ad275e3 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/BufferAllocator.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/BufferAllocator.java @@ -74,8 +74,8 @@ public interface BufferAllocator extends AutoCloseable { * byte contents. The buffer has the same capacity as the byte array length, and its write offset is placed at the * end, and its read offset is at the beginning, such that the entire buffer contents are readable. *

- * The buffers produced by the supplier will have {@linkplain Buffer#isOwned() ownership}, and closing them will - * make them {@linkplain Buffer#isAccessible() inaccessible}, just like a normally allocated buffer. + * The buffers produced by the supplier will each have their own independent life-cycle, and closing them will + * make them {@linkplain Buffer#isAccessible() inaccessible}, just like normally allocated buffers. *

* The buffers produced are "constants", in the sense that they are {@linkplain Buffer#readOnly() read-only}. *

diff --git a/buffer-api/src/main/java/io/netty/buffer/api/BufferHolder.java b/buffer-api/src/main/java/io/netty/buffer/api/BufferHolder.java index 0c0d66b..eadd4da 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/BufferHolder.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/BufferHolder.java @@ -15,6 +15,7 @@ */ package io.netty.buffer.api; +import io.netty.buffer.api.internal.ResourceSupport; import io.netty.buffer.api.internal.Statics; import java.lang.invoke.VarHandle; @@ -31,12 +32,12 @@ import static java.lang.invoke.MethodHandles.lookup; * as inspiration. *

* If you just want an object that is a reference to a buffer, then the {@link BufferRef} can be used for that purpose. - * If you have an advanced use case where you wish to implement {@link Rc}, and tightly control lifetimes, then - * {@link RcSupport} can be of help. + * If you have an advanced use case where you wish to implement {@link Resource}, and tightly control lifetimes, then + * {@link ResourceSupport} can be of help. * * @param The concrete {@link BufferHolder} type. */ -public abstract class BufferHolder> implements Rc { +public abstract class BufferHolder> implements Resource { private static final VarHandle BUF = Statics.findVarHandle(lookup(), BufferHolder.class, "buf", Buffer.class); private Buffer buf; @@ -48,7 +49,7 @@ public abstract class BufferHolder> implements Rc { * @param buf The {@linkplain Buffer buffer} to be held by this holder. */ protected BufferHolder(Buffer buf) { - this.buf = Objects.requireNonNull(buf, "The buffer cannot be null.").acquire(); + this.buf = Objects.requireNonNull(buf, "The buffer cannot be null."); } /** @@ -62,23 +63,11 @@ public abstract class BufferHolder> implements Rc { buf = Objects.requireNonNull(send, "The send cannot be null.").receive(); } - @SuppressWarnings("unchecked") - @Override - public T acquire() { - buf.acquire(); - return (T) this; - } - @Override public void close() { buf.close(); } - @Override - public boolean isOwned() { - return buf.isOwned(); - } - @SuppressWarnings("unchecked") @Override public Send send() { @@ -95,24 +84,6 @@ public abstract class BufferHolder> implements Rc { */ protected abstract T receive(Buffer buf); - /** - * Replace the underlying referenced buffer with the given buffer. - *

- * This method is protected to permit advanced use cases of {@link BufferHolder} sub-class implementations. - *

- * Note: this method decreases the reference count of the current buffer, - * and increases the reference count of the new buffer. - *

- * The buffer assignment is performed using a plain store. - * - * @param newBuf The new {@link Buffer} instance that is replacing the currently held buffer. - */ - protected final void replaceBuf(Buffer newBuf) { - try (var ignore = buf) { - buf = newBuf.acquire(); - } - } - /** * Replace the underlying referenced buffer with the given buffer. *

@@ -125,27 +96,9 @@ public abstract class BufferHolder> implements Rc { * * @param send The new {@link Buffer} instance that is replacing the currently held buffer. */ - protected final void replaceBuf(Send send) { - try (var ignore = buf) { - buf = send.receive(); - } - } - - /** - * Replace the underlying referenced buffer with the given buffer. - *

- * This method is protected to permit advanced use cases of {@link BufferHolder} sub-class implementations. - *

- * Note: this method decreases the reference count of the current buffer, - * and increases the reference count of the new buffer. - *

- * The buffer assignment is performed using a volatile store. - * - * @param newBuf The new {@link Buffer} instance that is replacing the currently held buffer. - */ - protected final void replaceBufVolatile(Buffer newBuf) { - var prev = (Buffer) BUF.getAndSet(this, newBuf.acquire()); - prev.close(); + protected final void replaceBuffer(Send send) { + buf.close(); + buf = send.receive(); } /** @@ -160,7 +113,7 @@ public abstract class BufferHolder> implements Rc { * * @param send The {@link Send} with the new {@link Buffer} instance that is replacing the currently held buffer. */ - protected final void replaceBufVolatile(Send send) { + protected final void replaceBufferVolatile(Send send) { var prev = (Buffer) BUF.getAndSet(this, send.receive()); prev.close(); } @@ -172,7 +125,7 @@ public abstract class BufferHolder> implements Rc { * * @return The {@link Buffer} instance being held by this {@linkplain T buffer holder}. */ - protected final Buffer getBuf() { + protected final Buffer getBuffer() { return buf; } @@ -183,7 +136,7 @@ public abstract class BufferHolder> implements Rc { * * @return The {@link Buffer} instance being held by this {@linkplain T buffer holder}. */ - protected final Buffer getBufVolatile() { + protected final Buffer getBufferVolatile() { return (Buffer) BUF.getVolatile(this); } diff --git a/buffer-api/src/main/java/io/netty/buffer/api/BufferRef.java b/buffer-api/src/main/java/io/netty/buffer/api/BufferRef.java index 2c2ee97..3a87c6f 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/BufferRef.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/BufferRef.java @@ -27,7 +27,7 @@ public final class BufferRef extends BufferHolder { * * @param buf The buffer to reference. */ - public BufferRef(Buffer buf) { + private BufferRef(Buffer buf) { super(buf); // BufferRef is meant to be atomic, so we need to add a fence to get the semantics of a volatile store. VarHandle.fullFence(); @@ -49,20 +49,6 @@ public final class BufferRef extends BufferHolder { return new BufferRef(buf); } - /** - * Replace the underlying referenced buffer with the given buffer. - *

- * Note: this method decreases the reference count of the current buffer, - * and increases the reference count of the new buffer. - *

- * The buffer assignment is performed using a volatile store. - * - * @param newBuf The new {@link Buffer} instance that is replacing the currently held buffer. - */ - public void replace(Buffer newBuf) { - replaceBufVolatile(newBuf); - } - /** * Replace the underlying referenced buffer with the given buffer. *

@@ -74,7 +60,7 @@ public final class BufferRef extends BufferHolder { * @param send The {@link Send} with the new {@link Buffer} instance that is replacing the currently held buffer. */ public void replace(Send send) { - replaceBufVolatile(send); + replaceBufferVolatile(send); } /** @@ -83,6 +69,6 @@ public final class BufferRef extends BufferHolder { * @return The buffer held by the reference. */ public Buffer contents() { - return getBufVolatile(); + return getBufferVolatile(); } } diff --git a/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java b/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java index c76acf5..7ff1fbb 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java @@ -15,6 +15,8 @@ */ package io.netty.buffer.api; +import io.netty.buffer.api.internal.ResourceSupport; + import java.nio.ByteBuffer; import java.nio.ByteOrder; import java.util.Arrays; @@ -32,11 +34,6 @@ import java.util.stream.Stream; * A composite buffer is constructed using one of the {@code compose} methods: *

    *
  • - * {@link #compose(BufferAllocator, Buffer...)} creates a composite buffer from the given set of buffers. - * The reference count to the passed buffers will be increased, and the resulting composite buffer may or may - * not have ownership. - *
    • - *
  • * {@link #compose(BufferAllocator, Send[])} creates a composite buffer from the buffers that are sent to it via * the passed in send objects. Since {@link Send#receive()} transfers ownership, the resulting composite buffer * will have ownership, because it is guaranteed that there are no other references to its constituent buffers. @@ -48,7 +45,7 @@ import java.util.stream.Stream; *
    • *
* Composite buffers can later be extended with internally allocated components, with {@link #ensureWritable(int)}, - * or with externally allocated buffers, using {@link #extendWith(Buffer)}. + * or with externally allocated buffers, using {@link #extendWith(Send)}. * *

Constituent buffer requirements

* @@ -82,12 +79,12 @@ import java.util.stream.Stream; * *

Ownership and Send

* - * {@linkplain Buffer#send() Sending} a composite buffer implies sending all of its constituent buffers. - * For sending to be possible, both the composite buffer itself, and all of its constituent buffers, must be in an - * {@linkplain Rc#isOwned() owned state}. - * This means that the composite buffer must be the only reference to the constituent buffers. + * {@linkplain Resource#send() Sending} a composite buffer implies sending all of its constituent buffers. + * For sending to be possible, both the composite buffer itself, and all of its constituent buffers, must be in a + * state that permits them being sent. This should be the case by default, as it shouldn't be possible to create + * composite buffers that can't be sent. */ -public final class CompositeBuffer extends RcSupport implements Buffer { +public final class CompositeBuffer extends ResourceSupport implements Buffer { /** * The max array size is JVM implementation dependant, but most seem to settle on {@code Integer.MAX_VALUE - 8}. * We set the max composite buffer capacity to the same, since it would otherwise be impossible to create a @@ -122,37 +119,6 @@ public final class CompositeBuffer extends RcSupport im private boolean closed; private boolean readOnly; - /** - * Compose the given sequence of buffers and present them as a single buffer. - *

- * Note: The composite buffer increments the reference count on all the constituent buffers, - * and holds a reference to them until the composite buffer is deallocated. - * This means the constituent buffers must still have their outside-reference count decremented as normal. - * If the buffers are allocated for the purpose of participating in the composite buffer, - * then they should be closed as soon as the composite buffer has been created, like in this example: - * 

{@code
-     *     try (Buffer a = allocator.allocate(size);
-     *          Buffer b = allocator.allocate(size)) {
-     *         return allocator.compose(a, b); // Reference counts for 'a' and 'b' incremented here.
-     *     } // Reference count for 'a' and 'b' decremented here; composite buffer now holds the last references.
-     * }
- *

- * See the class documentation for more information on what is required of the given buffers for composition to be - * allowed. - * - * @param allocator The allocator for the composite buffer. This allocator will be used e.g. to service - * {@link #ensureWritable(int)} calls. - * @param bufs The buffers to compose into a single buffer view. - * @return A buffer composed of, and backed by, the given buffers. - * @throws IllegalArgumentException if the given buffers have an inconsistent - * {@linkplain Buffer#order() byte order}. - */ - public static CompositeBuffer compose(BufferAllocator allocator, Buffer... bufs) { - Stream bufferStream = Arrays.stream(bufs) - .map(buf -> buf.acquire()); // Increments reference counts. - return new CompositeBuffer(allocator, filterExternalBufs(bufferStream), COMPOSITE_DROP, false); - } - /** * Compose the given sequence of sends of buffers and present them as a single buffer. *

@@ -201,25 +167,25 @@ public final class CompositeBuffer extends RcSupport im if (ise != null) { throw ise; } - return new CompositeBuffer(allocator, filterExternalBufs(Arrays.stream(bufs)), COMPOSITE_DROP, false); + return new CompositeBuffer(allocator, filterExternalBufs(Arrays.stream(bufs)), COMPOSITE_DROP); } /** * Create an empty composite buffer, that has no components. The buffer can be extended with components using either - * {@link #ensureWritable(int)} or {@link #extendWith(Buffer)}. + * {@link #ensureWritable(int)} or {@link #extendWith(Send)}. * * @param allocator The allocator for the composite buffer. This allocator will be used e.g. to service * {@link #ensureWritable(int)} calls. * @return A composite buffer that has no components, and has a capacity of zero. */ public static CompositeBuffer compose(BufferAllocator allocator) { - return new CompositeBuffer(allocator, EMPTY_BUFFER_ARRAY, COMPOSITE_DROP, false); + return new CompositeBuffer(allocator, EMPTY_BUFFER_ARRAY, COMPOSITE_DROP); } /** - * Check if the given buffer is a {@linkplain #compose(BufferAllocator, Buffer...) composite} buffer or not. + * Check if the given buffer is a {@linkplain #compose(BufferAllocator, Send...) composite} buffer or not. * @param composite The buffer to check. - * @return {@code true} if the given buffer was created with {@link #compose(BufferAllocator, Buffer...)}, + * @return {@code true} if the given buffer was created with {@link #compose(BufferAllocator, Send...)}, * {@code false} otherwise. */ public static boolean isComposite(Buffer composite) { @@ -267,24 +233,14 @@ public final class CompositeBuffer extends RcSupport im // Extract components and move our reference count from the composite onto the components. var composite = (CompositeBuffer) buf; var bufs = composite.bufs; - for (Buffer b : bufs) { - b.acquire(); - } - buf.close(); // Important: acquire on components *before* closing composite. return Stream.of(bufs); } return Stream.of(buf); } - private CompositeBuffer(BufferAllocator allocator, Buffer[] bufs, Drop drop, - boolean acquireBufs) { + private CompositeBuffer(BufferAllocator allocator, Buffer[] bufs, Drop drop) { super(drop); this.allocator = allocator; - if (acquireBufs) { - for (Buffer buf : bufs) { - buf.acquire(); - } - } try { if (bufs.length > 0) { ByteOrder targetOrder = bufs[0].order(); @@ -490,7 +446,7 @@ public final class CompositeBuffer extends RcSupport im // This is important because 1) slice() already acquired the buffers, and 2) if this slice is empty // then we need to keep holding on to it to prevent this originating composite buffer from getting // ownership. If it did, its behaviour would be inconsistent with that of a non-composite buffer. - return new CompositeBuffer(allocator, slices, COMPOSITE_DROP, false); + return new CompositeBuffer(allocator, slices, COMPOSITE_DROP); } @Override @@ -838,33 +794,37 @@ public final class CompositeBuffer extends RcSupport im * the composite buffer was created. * The extension buffer is added to the end of this composite buffer, which is modified in-place. * - * @see #compose(BufferAllocator, Buffer...) * @see #compose(BufferAllocator, Send...) * @param extension The buffer to extend the composite buffer with. */ - public void extendWith(Buffer extension) { + public void extendWith(Send extension) { Objects.requireNonNull(extension, "Extension buffer cannot be null."); + Buffer buffer = extension.receive(); if (!isOwned()) { + buffer.close(); throw new IllegalStateException("This buffer cannot be extended because it is not in an owned state."); } - if (bufs.length > 0 && extension.order() != order()) { + if (bufs.length > 0 && buffer.order() != order()) { + buffer.close(); throw new IllegalArgumentException( "This buffer uses " + order() + " byte order, and cannot be extended with " + - "a buffer that uses " + extension.order() + " byte order."); + "a buffer that uses " + buffer.order() + " byte order."); } - if (bufs.length > 0 && extension.readOnly() != readOnly()) { + if (bufs.length > 0 && buffer.readOnly() != readOnly()) { + buffer.close(); throw new IllegalArgumentException( "This buffer is " + (readOnly? "read-only" : "writable") + ", " + "and cannot be extended with a buffer that is " + - (extension.readOnly()? "read-only." : "writable.")); + (buffer.readOnly()? "read-only." : "writable.")); } - long extensionCapacity = extension.capacity(); + long extensionCapacity = buffer.capacity(); if (extensionCapacity == 0) { // Extending by a zero-sized buffer makes no difference. Especially since it's not allowed to change the // capacity of buffers that are constiuents of composite buffers. // This also ensures that methods like countComponents, and forEachReadable, do not have to worry about // overflow in their component counters. + buffer.close(); return; } @@ -873,10 +833,10 @@ public final class CompositeBuffer extends RcSupport im Buffer[] restoreTemp = bufs; // We need this to restore our buffer array, in case offset computations fail. try { - if (extension instanceof CompositeBuffer) { + if (buffer instanceof CompositeBuffer) { // If the extension is itself a composite buffer, then extend this one by all the constituent // component buffers. - CompositeBuffer compositeExtension = (CompositeBuffer) extension; + CompositeBuffer compositeExtension = (CompositeBuffer) buffer; Buffer[] addedBuffers = compositeExtension.bufs; Set duplicatesCheck = Collections.newSetFromMap(new IdentityHashMap<>()); duplicatesCheck.addAll(Arrays.asList(bufs)); @@ -884,24 +844,21 @@ public final class CompositeBuffer extends RcSupport im if (duplicatesCheck.size() < bufs.length + addedBuffers.length) { throw extensionDuplicatesException(); } - for (Buffer addedBuffer : addedBuffers) { - addedBuffer.acquire(); - } int extendAtIndex = bufs.length; bufs = Arrays.copyOf(bufs, extendAtIndex + addedBuffers.length); System.arraycopy(addedBuffers, 0, bufs, extendAtIndex, addedBuffers.length); computeBufferOffsets(); } else { for (Buffer buf : restoreTemp) { - if (buf == extension) { + if (buf == buffer) { throw extensionDuplicatesException(); } } - unsafeExtendWith(extension.acquire()); + unsafeExtendWith(buffer); } if (restoreTemp.length == 0) { - order = extension.order(); - readOnly = extension.readOnly(); + order = buffer.order(); + readOnly = buffer.readOnly(); } } catch (Exception e) { bufs = restoreTemp; @@ -939,7 +896,7 @@ public final class CompositeBuffer extends RcSupport im checkSplit(splitOffset); if (bufs.length == 0) { // Splitting a zero-length buffer is trivial. - return new CompositeBuffer(allocator, bufs, unsafeGetDrop(), true).order(order); + return new CompositeBuffer(allocator, bufs, unsafeGetDrop()).order(order); } int i = searchOffsets(splitOffset); @@ -954,16 +911,9 @@ public final class CompositeBuffer extends RcSupport im } private CompositeBuffer buildSplitBuffer(Buffer[] splits) { - try { - var compositeBuf = new CompositeBuffer(allocator, splits, unsafeGetDrop(), true); - compositeBuf.order = order; // Preserve byte order even if splits array is empty. - return compositeBuf; - } finally { - // Drop our references to the buffers in the splits array. They belong to the new composite buffer now. - for (Buffer split : splits) { - split.close(); - } - } + var compositeBuf = new CompositeBuffer(allocator, splits, unsafeGetDrop()); + compositeBuf.order = order; // Preserve byte order even if splits array is empty. + return compositeBuf; } /** @@ -980,7 +930,7 @@ public final class CompositeBuffer extends RcSupport im checkSplit(splitOffset); if (bufs.length == 0) { // Splitting a zero-length buffer is trivial. - return new CompositeBuffer(allocator, bufs, unsafeGetDrop(), true).order(order); + return new CompositeBuffer(allocator, bufs, unsafeGetDrop()).order(order); } int i = searchOffsets(splitOffset); @@ -1008,7 +958,7 @@ public final class CompositeBuffer extends RcSupport im checkSplit(splitOffset); if (bufs.length == 0) { // Splitting a zero-length buffer is trivial. - return new CompositeBuffer(allocator, bufs, unsafeGetDrop(), true).order(order); + return new CompositeBuffer(allocator, bufs, unsafeGetDrop()).order(order); } int i = searchOffsets(splitOffset); @@ -1415,7 +1365,7 @@ public final class CompositeBuffer extends RcSupport im for (int i = 0; i < sends.length; i++) { received[i] = sends[i].receive(); } - var composite = new CompositeBuffer(allocator, received, drop, false); + var composite = new CompositeBuffer(allocator, received, drop); composite.readOnly = readOnly; drop.attach(composite); return composite; @@ -1439,7 +1389,8 @@ public final class CompositeBuffer extends RcSupport im private boolean allConstituentsAreOwned() { boolean result = true; for (Buffer buf : bufs) { - result &= buf.isOwned(); + //noinspection unchecked + result &= ((ResourceSupport) buf).isOwned(); } return result; } diff --git a/buffer-api/src/main/java/io/netty/buffer/api/Drop.java b/buffer-api/src/main/java/io/netty/buffer/api/Drop.java index 1658c7e..1461620 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/Drop.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/Drop.java @@ -16,24 +16,24 @@ package io.netty.buffer.api; /** - * The Drop interface is used by {@link Rc} instances to implement their resource disposal mechanics. The {@link - * #drop(Object)} method will be called by the Rc when their last reference is closed. + * The Drop interface is used by {@link Resource} instances to implement their resource disposal mechanics. + * The {@link #drop(Object)} method will be called by the resource when they are closed. * * @param */ @FunctionalInterface public interface Drop { /** - * Dispose of the resources in the given Rc. + * Dispose of the resources in the given {@link Resource} instance. * - * @param obj The Rc instance being dropped. + * @param obj The {@link Resource} instance being dropped. */ void drop(T obj); /** * Called when the resource changes owner. * - * @param obj The new Rc instance with the new owner. + * @param obj The new {@link Resource} instance with the new owner. */ default void attach(T obj) { } diff --git a/buffer-api/src/main/java/io/netty/buffer/api/Owned.java b/buffer-api/src/main/java/io/netty/buffer/api/Owned.java index 9fa7000..9f22ecc 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/Owned.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/Owned.java @@ -16,23 +16,23 @@ package io.netty.buffer.api; /** - * This interface encapsulates the ownership of an {@link Rc}, and exposes a method that may be used to transfer this - * ownership to the specified recipient thread. + * This interface encapsulates the ownership of a {@link Resource}, and exposes a method that may be used to transfer + * this ownership to the specified recipient thread. * - * @param The concrete type of {@link Rc} that is owned. + * @param The concrete type of {@link Resource} that is owned. */ @SuppressWarnings("InterfaceMayBeAnnotatedFunctional") public interface Owned { /** - * Transfer the ownership of the owned Rc, to the calling thread. The owned Rc is invalidated but without - * disposing of its internal state. Then a new Rc with the given owner is produced in its stead. + * Transfer the ownership of the resource, to the calling thread. The resource instance is invalidated but without + * disposing of its internal state. Then a new resource instance with the given owner is produced in its stead. *

* This method is called by {@link Send} implementations. These implementations will ensure that the transfer of * ownership (the calling of this method) happens-before the new owner begins accessing the new object. This ensures - * that the new Rc is safely published to the new owners. + * that the new resource instanec is safely published to the new owners. * - * @param drop The drop object that knows how to dispose of the state represented by this Rc. - * @return A new Rc instance that is exactly the same as this Rc, except it has the new owner. + * @param drop The drop object that knows how to dispose of the state represented by this {@link Resource}. + * @return A new resource instance that is exactly the same as this resource. */ T transferOwnership(Drop drop); } diff --git a/buffer-api/src/main/java/io/netty/buffer/api/Rc.java b/buffer-api/src/main/java/io/netty/buffer/api/Rc.java deleted file mode 100644 index 48d776a..0000000 --- a/buffer-api/src/main/java/io/netty/buffer/api/Rc.java +++ /dev/null @@ -1,80 +0,0 @@ -/* - * Copyright 2020 The Netty Project - * - * The Netty Project licenses this file to you under the Apache License, - * version 2.0 (the "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at: - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations - * under the License. - */ -package io.netty.buffer.api; - -/** - * An Rc is a reference counted, thread-confined, resource of sorts. Because these resources are thread-confined, the - * reference counting is NOT atomic. An Rc can only be accessed by one thread at a time - the owner thread that the - * resource is confined to. - *

- * When the last reference is closed (accounted for using {@link AutoCloseable} and try-with-resources statements, - * ideally), then the resource is desposed of, or released, or returned to the pool it came from. The precise action is - * implemented by the {@link Drop} instance given as an argument to the Rc constructor. - * - * @param The concrete subtype. - */ -public interface Rc> extends AutoCloseable { - /** - * Increment the reference count. - *

- * Note, this method is not thread-safe because reference counted objects are meant to thread-confined. - * - * @return This Rc instance. - */ - I acquire(); - - /** - * Decrement the reference count, and despose of the resource if the last reference is closed. - *

- * Note, this method is not thread-safe because reference counted objects are meant to be thread-confined. - * - * @throws IllegalStateException If this Rc has already been closed. - */ - @Override - void close(); - - /** - * Send this reference counted object instance to another Thread, transferring the ownership to the recipient. - *

- * Note that the object must be {@linkplain #isOwned() owned}, and cannot have any outstanding borrows, - * when it's being sent. - * That is, all previous acquires must have been closed, and {@link #isOwned()} must return {@code true}. - *

- * This instance immediately becomes inaccessible, and all attempts at accessing this reference counted object - * will throw. Calling {@link #close()} will have no effect, so this method is safe to call within a - * try-with-resources statement. - */ - Send send(); - - /** - * Check that this reference counted object is owned. - *

- * To be owned, the object must have no outstanding acquires, and no other implementation defined restrictions. - * - * @return {@code true} if this object can be {@linkplain #send() sent}, - * or {@code false} if calling {@link #send()} would throw an exception. - */ - boolean isOwned(); - - /** - * Check if this object is accessible. - * - * @return {@code true} if this object is still valid and can be accessed, - * otherwise {@code false} if, for instance, this object has been dropped/deallocated, - * or been {@linkplain #send() sent} elsewhere. - */ - boolean isAccessible(); -} diff --git a/buffer-api/src/main/java/io/netty/buffer/api/ReadableComponentProcessor.java b/buffer-api/src/main/java/io/netty/buffer/api/ReadableComponentProcessor.java index b6e21f4..74232d4 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/ReadableComponentProcessor.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/ReadableComponentProcessor.java @@ -27,10 +27,10 @@ public interface ReadableComponentProcessor { * {@link Buffer#forEachReadable(int, ReadableComponentProcessor) iteration}. *

* The component object itself is only valid during this call, but the {@link ByteBuffer byte buffers}, arrays, and - * native address pointers obtained from it, will be valid until any {@link Buffer#isOwned() ownership} requiring - * operation is performed on the buffer. + * native address pointers obtained from it, will be valid until any operation is performed on the buffer, which + * changes the internal memory. * - * @param index The current index of the given buffer component, based on the initial index passed to the + * @param index The current index of the given buffer component, based on the initial index passed to the * {@link Buffer#forEachReadable(int, ReadableComponentProcessor)} method. * @param component The current buffer component being processed. * @return {@code true} if the iteration should continue and more components should be processed, otherwise diff --git a/buffer-api/src/main/java/io/netty/buffer/api/Resource.java b/buffer-api/src/main/java/io/netty/buffer/api/Resource.java new file mode 100644 index 0000000..8fa18a9 --- /dev/null +++ b/buffer-api/src/main/java/io/netty/buffer/api/Resource.java @@ -0,0 +1,41 @@ +package io.netty.buffer.api; + +/** + * A resource that has a life-time, and can be {@linkplain #close() closed}. + * Resources are initially {@linkplain #isAccessible() accessible}, but closing them makes them inaccessible. + */ +public interface Resource> extends AutoCloseable { + /** + * Send this object instance to another Thread, transferring the ownership to the recipient. + *

+ * The object must be in a state where it can be sent, which includes at least being + * {@linkplain #isAccessible() accessible}. + *

+ * When sent, this instance will immediately become inaccessible, as if by {@linkplain #close() closing} it. + * All attempts at accessing an object that has been sent, even if that object has not yet been received, should + * cause an exception to be thrown. + *

+ * Calling {@link #close()} on an object that has been sent will have no effect, so this method is safe to call + * within a try-with-resources statement. + */ + Send send(); + + /** + * Close the resource, making it inaccessible. + *

+ * Note, this method is not thread-safe unless otherwise specific. + * + * @throws IllegalStateException If this {@code Resource} has already been closed. + */ + @Override + void close(); + + /** + * Check if this object is accessible. + * + * @return {@code true} if this object is still valid and can be accessed, + * otherwise {@code false} if, for instance, this object has been dropped/deallocated, + * or been {@linkplain #send() sent} elsewhere. + */ + boolean isAccessible(); +} diff --git a/buffer-api/src/main/java/io/netty/buffer/api/Scope.java b/buffer-api/src/main/java/io/netty/buffer/api/Scope.java index fa24987..ce746d3 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/Scope.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/Scope.java @@ -19,7 +19,7 @@ import java.util.ArrayDeque; /** * A scope is a convenient mechanism for capturing the life cycles of multiple reference counted objects. Once the scope - * is closed, all of the added objects will also be closed in reverse insert order. That is, the most recently added + * is closed, all the added objects will also be closed in reverse insert order. That is, the most recently added * object will be closed first. *

* Scopes can be reused. After a scope has been closed, new objects can be added to it, and they will be closed when the @@ -31,18 +31,18 @@ import java.util.ArrayDeque; * Note that scopes are not thread-safe. They are intended to be used from a single thread. */ public final class Scope implements AutoCloseable { - private final ArrayDeque> deque = new ArrayDeque<>(); + private final ArrayDeque> deque = new ArrayDeque<>(); /** - * Add the given reference counted object to this scope, so that it will be {@linkplain Rc#close() closed} when this - * scope is {@linkplain #close() closed}. + * Add the given reference counted object to this scope, so that it will be {@linkplain Resource#close() closed} + * when this scope is {@linkplain #close() closed}. * * @param obj The reference counted object to add to this scope. * @param The type of the reference counted object. * @return The same exact object that was added; further operations can be chained on the object after this method * call. */ - public > T add(T obj) { + public > T add(T obj) { deque.addLast(obj); return obj; } @@ -52,7 +52,7 @@ public final class Scope implements AutoCloseable { */ @Override public void close() { - Rc obj; + Resource obj; while ((obj = deque.pollLast()) != null) { obj.close(); } diff --git a/buffer-api/src/main/java/io/netty/buffer/api/Send.java b/buffer-api/src/main/java/io/netty/buffer/api/Send.java index a269d22..0b414b7 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/Send.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/Send.java @@ -20,33 +20,34 @@ import java.util.function.Function; import java.util.function.Supplier; /** - * A Send object is a temporary holder of an {@link Rc}, used for transferring the ownership of the Rc from one thread - * to another. + * A {@code Send} object is a temporary holder of a {@link Resource}, used for transferring the ownership of the + * resource from one thread to another. *

- * Prior to the Send being created, the originating Rc is invalidated, to prevent access while it is being sent. This - * means it cannot be accessed, closed, or disposed of, while it is in-flight. Once the Rc is {@linkplain #receive() - * received}, the new ownership is established. + * Prior to the {@code Send} being created, the originating resource is invalidated, to prevent access while it is being + * sent. This means it cannot be accessed, closed, or disposed of, while it is in-flight. Once the resource is + * {@linkplain #receive() received}, the new ownership is established. *

- * Care must be taken to ensure that the Rc is always received by some thread. Failure to do so can result in a resource - * leak. + * Care must be taken to ensure that the resource is always received by some thread. + * Failure to do so can result in a resource leak. * * @param */ -public interface Send> { +public interface Send> { /** * Construct a {@link Send} based on the given {@link Supplier}. * The supplier will be called only once, in the receiving thread. * * @param concreteObjectType The concrete type of the object being sent. Specifically, the object returned from the * {@link Supplier#get()} method must be an instance of this class. - * @param supplier The supplier of the object being sent, which will be called when the object is ready to be - * received. - * @param The type of object being sent. + * @param supplier The supplier of the object being sent, which will be called when the object is ready to be + * received. + * @param The type of object being sent. * @return A {@link Send} which will deliver an object of the given type, from the supplier. */ - static > Send sending(Class concreteObjectType, Supplier supplier) { + static > Send sending(Class concreteObjectType, Supplier supplier) { return new Send() { private final AtomicBoolean gate = new AtomicBoolean(); + @Override public T receive() { if (gate.getAndSet(true)) { @@ -73,7 +74,7 @@ public interface Send> { * Determine if the given candidate object is an instance of a {@link Send} from which an object of the given type * can be received. * - * @param type The type of object we wish to receive. + * @param type The type of object we wish to receive. * @param candidate The candidate object that might be a {@link Send} of an object of the given type. * @return {@code true} if the candidate object is a {@link Send} that would deliver an object of the given type, * otherwise {@code false}. @@ -83,12 +84,12 @@ public interface Send> { } /** - * Receive the {@link Rc} instance being sent, and bind its ownership to the calling thread. The invalidation of the - * sent Rc in the sending thread happens-before the return of this method. + * Receive the {@link Resource} instance being sent, and bind its ownership to the calling thread. + * The invalidation of the sent resource in the sending thread happens-before the return of this method. *

* This method can only be called once, and will throw otherwise. * - * @return The sent Rc instance. + * @return The sent resource instance. * @throws IllegalStateException If this method is called more than once. */ T receive(); @@ -96,24 +97,24 @@ public interface Send> { /** * Apply a mapping function to the object being sent. The mapping will occur when the object is received. * - * @param type The result type of the mapping function. + * @param type The result type of the mapping function. * @param mapper The mapping function to apply to the object being sent. - * @param The result type of the mapping function. + * @param The result type of the mapping function. * @return A new {@link Send} instance that will deliver an object that is the result of the mapping. */ - default > Send map(Class type, Function mapper) { + default > Send map(Class type, Function mapper) { return sending(type, () -> mapper.apply(receive())); } /** * Discard this {@link Send} and the object it contains. - * This has no effect if the send has already been received. + * This has no effect if the send object has already been received. */ default void discard() { try { receive().close(); } catch (IllegalStateException ignore) { - // Don't do anything if the send has already been consumed. + // Don't do anything if the "Send" has already been consumed. } } diff --git a/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java b/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java index 927ea26..c58148a 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java @@ -25,7 +25,7 @@ import io.netty.buffer.Unpooled; import io.netty.buffer.api.BufferAllocator; import io.netty.buffer.api.internal.Statics; import io.netty.buffer.api.Buffer; -import io.netty.buffer.api.RcSupport; +import io.netty.buffer.api.internal.ResourceSupport; import io.netty.util.ByteProcessor; import io.netty.util.IllegalReferenceCountException; @@ -40,6 +40,8 @@ import java.nio.channels.ScatteringByteChannel; import java.nio.charset.Charset; import java.util.concurrent.atomic.AtomicReference; +import static io.netty.buffer.api.internal.Statics.asRS; + public final class ByteBufAdaptor extends ByteBuf { private final ByteBufAllocatorAdaptor alloc; private final Buffer buffer; @@ -80,7 +82,7 @@ public final class ByteBufAdaptor extends ByteBuf { try { buffer.ensureWritable(diff); } catch (IllegalStateException e) { - if (!buffer.isOwned()) { + if (!asRS(buffer).isOwned()) { throw new UnsupportedOperationException(e); } throw e; @@ -215,7 +217,7 @@ public final class ByteBufAdaptor extends ByteBuf { checkAccess(); if (writableBytes() < minWritableBytes) { try { - if (buffer.isOwned()) { + if (asRS(buffer).isOwned()) { // Good place. buffer.ensureWritable(minWritableBytes); } else { @@ -1629,7 +1631,7 @@ public final class ByteBufAdaptor extends ByteBuf { @Override public ByteBuf retain(int increment) { for (int i = 0; i < increment; i++) { - buffer.acquire(); + asRS(buffer).acquire(); } return this; } @@ -1643,11 +1645,11 @@ public final class ByteBufAdaptor extends ByteBuf { if (!buffer.isAccessible()) { return -1; } - if (buffer instanceof RcSupport) { - var rc = (RcSupport) buffer; + if (buffer instanceof ResourceSupport) { + var rc = (ResourceSupport) buffer; return rc.countBorrows(); } - return buffer.isOwned()? 0 : 1; + return asRS(buffer).isOwned()? 0 : 1; } @Override diff --git a/buffer-api/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java b/buffer-api/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java index 4c99a2e..809da74 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java @@ -21,7 +21,7 @@ import io.netty.buffer.api.BufferAllocator; import io.netty.buffer.api.ByteCursor; import io.netty.buffer.api.Drop; import io.netty.buffer.api.Owned; -import io.netty.buffer.api.RcSupport; +import io.netty.buffer.api.internal.ResourceSupport; import io.netty.buffer.api.ReadableComponent; import io.netty.buffer.api.ReadableComponentProcessor; import io.netty.buffer.api.WritableComponent; @@ -39,7 +39,7 @@ import static io.netty.buffer.api.internal.Statics.bbslice; import static io.netty.buffer.api.internal.Statics.bufferIsClosed; import static io.netty.buffer.api.internal.Statics.bufferIsReadOnly; -class NioBuffer extends RcSupport implements Buffer, ReadableComponent, WritableComponent { +class NioBuffer extends ResourceSupport implements Buffer, ReadableComponent, WritableComponent { private static final ByteBuffer CLOSED_BUFFER = ByteBuffer.allocate(0); private final AllocatorControl control; diff --git a/buffer-api/src/main/java/io/netty/buffer/api/LifecycleTracer.java b/buffer-api/src/main/java/io/netty/buffer/api/internal/LifecycleTracer.java similarity index 82% rename from buffer-api/src/main/java/io/netty/buffer/api/LifecycleTracer.java rename to buffer-api/src/main/java/io/netty/buffer/api/internal/LifecycleTracer.java index 103d984..384904a 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/LifecycleTracer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/internal/LifecycleTracer.java @@ -13,14 +13,18 @@ * License for the specific language governing permissions and limitations * under the License. */ -package io.netty.buffer.api; +package io.netty.buffer.api.internal; + +import io.netty.buffer.api.Drop; +import io.netty.buffer.api.Owned; +import io.netty.buffer.api.Resource; import java.util.ArrayDeque; import java.util.Set; import java.util.function.Function; import java.util.stream.Stream; -abstract class LifecycleTracer { +public abstract class LifecycleTracer { public static LifecycleTracer get() { if (Trace.TRACE_LIFECYCLE_DEPTH == 0) { return new NoOpTracer(); @@ -30,36 +34,36 @@ abstract class LifecycleTracer { return stackTracer; } - abstract void acquire(int acquires); + public abstract void acquire(int acquires); - abstract void drop(int acquires); + public abstract void drop(int acquires); - abstract void close(int acquires); + public abstract void close(int acquires); - abstract , T extends RcSupport> Owned send(Owned send, int acquires); + public abstract , T extends ResourceSupport> Owned send(Owned send, int acquires); - abstract E attachTrace(E throwable); + public abstract E attachTrace(E throwable); private static final class NoOpTracer extends LifecycleTracer { @Override - void acquire(int acquires) { + public void acquire(int acquires) { } @Override - void drop(int acquires) { + public void drop(int acquires) { } @Override - void close(int acquires) { + public void close(int acquires) { } @Override - , T extends RcSupport> Owned send(Owned send, int acquires) { + public , T extends ResourceSupport> Owned send(Owned send, int acquires) { return send; } @Override - E attachTrace(E throwable) { + public E attachTrace(E throwable) { return throwable; } } @@ -76,7 +80,7 @@ abstract class LifecycleTracer { private boolean dropped; @Override - void acquire(int acquires) { + public void acquire(int acquires) { Trace trace = WALKER.walk(new Trace("acquire", acquires)); addTrace(trace); } @@ -91,20 +95,20 @@ abstract class LifecycleTracer { } @Override - void drop(int acquires) { + public void drop(int acquires) { dropped = true; addTrace(WALKER.walk(new Trace("drop", acquires))); } @Override - void close(int acquires) { + public void close(int acquires) { if (!dropped) { addTrace(WALKER.walk(new Trace("close", acquires))); } } @Override - , T extends RcSupport> Owned send(Owned send, int acquires) { + public , T extends ResourceSupport> Owned send(Owned send, int acquires) { Trace sendTrace = new Trace("send", acquires); sendTrace.sent = true; addTrace(WALKER.walk(sendTrace)); @@ -118,7 +122,7 @@ abstract class LifecycleTracer { } @Override - E attachTrace(E throwable) { + public E attachTrace(E throwable) { synchronized (traces) { long timestamp = System.nanoTime(); for (Trace trace : traces) { diff --git a/buffer-api/src/main/java/io/netty/buffer/api/RcSupport.java b/buffer-api/src/main/java/io/netty/buffer/api/internal/ResourceSupport.java similarity index 74% rename from buffer-api/src/main/java/io/netty/buffer/api/RcSupport.java rename to buffer-api/src/main/java/io/netty/buffer/api/internal/ResourceSupport.java index 6448adc..a7bb843 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/RcSupport.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/internal/ResourceSupport.java @@ -13,16 +13,27 @@ * License for the specific language governing permissions and limitations * under the License. */ -package io.netty.buffer.api; +package io.netty.buffer.api.internal; + +import io.netty.buffer.api.Drop; +import io.netty.buffer.api.Owned; +import io.netty.buffer.api.Resource; +import io.netty.buffer.api.Send; import java.util.Objects; -public abstract class RcSupport, T extends RcSupport> implements Rc { +/** + * Internal support class for resources. + * + * @param The public interface for the resource. + * @param The concrete implementation of the resource. + */ +public abstract class ResourceSupport, T extends ResourceSupport> implements Resource { private int acquires; // Closed if negative. private Drop drop; private final LifecycleTracer tracer; - protected RcSupport(Drop drop) { + protected ResourceSupport(Drop drop) { this.drop = drop; tracer = LifecycleTracer.get(); } @@ -30,11 +41,10 @@ public abstract class RcSupport, T extends RcSupport> impl /** * Increment the reference count. *

- * Note, this method is not thread-safe because Rc's are meant to thread-confined. + * Note, this method is not thread-safe because Resources are meant to thread-confined. * - * @return This Rc instance. + * @return This {@link Resource} instance. */ - @Override public final I acquire() { if (acquires < 0) { throw attachTrace(new IllegalStateException("This resource is closed: " + this + '.')); @@ -50,9 +60,9 @@ public abstract class RcSupport, T extends RcSupport> impl /** * Decrement the reference count, and despose of the resource if the last reference is closed. *

- * Note, this method is not thread-safe because Rc's are meant to be thread-confined. + * Note, this method is not thread-safe because Resources are meant to be thread-confined. * - * @throws IllegalStateException If this Rc has already been closed. + * @throws IllegalStateException If this Resource has already been closed. */ @Override public final void close() { @@ -68,11 +78,12 @@ public abstract class RcSupport, T extends RcSupport> impl } /** - * Send this Rc instance to another Thread, transferring the ownership to the recipient. This method can be used - * when the receiving thread is not known up front. + * Send this Resource instance to another Thread, transferring the ownership to the recipient. + * This method can be used when the receiving thread is not known up front. *

- * This instance immediately becomes inaccessible, and all attempts at accessing this Rc will throw. Calling {@link - * #close()} will have no effect, so this method is safe to call within a try-with-resources statement. + * This instance immediately becomes inaccessible, and all attempts at accessing this resource will throw. + * Calling {@link #close()} will have no effect, so this method is safe to call within a try-with-resources + * statement. * * @throws IllegalStateException if this object has any outstanding acquires; that is, if this object has been * {@link #acquire() acquired} more times than it has been {@link #close() closed}. @@ -92,8 +103,8 @@ public abstract class RcSupport, T extends RcSupport> impl } /** - * Create an {@link IllegalStateException} with a custom message, tailored to this particular {@link Rc} instance, - * for when the object cannot be sent for some reason. + * Create an {@link IllegalStateException} with a custom message, tailored to this particular + * {@link Resource} instance, for when the object cannot be sent for some reason. * @return An {@link IllegalStateException} to be thrown when this object cannot be sent. */ protected IllegalStateException notSendableException() { @@ -101,7 +112,6 @@ public abstract class RcSupport, T extends RcSupport> impl "Cannot send() a reference counted object with " + countBorrows() + " borrows: " + this + '.'); } - @Override public boolean isOwned() { return acquires == 0; } @@ -124,11 +134,12 @@ public abstract class RcSupport, T extends RcSupport> impl /** * Prepare this instance for ownsership transfer. This method is called from {@link #send()} in the sending thread. - * This method should put this Rc in a deactivated state where it is no longer accessible from the currently owning - * thread. In this state, the Rc instance should only allow a call to {@link Owned#transferOwnership(Drop)} in - * the recipient thread. + * This method should put this resource in a deactivated state where it is no longer accessible from the currently + * owning thread. + * In this state, the resource instance should only allow a call to {@link Owned#transferOwnership(Drop)} in the + * recipient thread. * - * @return This Rc instance in a deactivated state. + * @return This resource instance in a deactivated state. */ protected abstract Owned prepareSend(); diff --git a/buffer-api/src/main/java/io/netty/buffer/api/internal/Statics.java b/buffer-api/src/main/java/io/netty/buffer/api/internal/Statics.java index f6ab7a1..6f0cd2c 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/internal/Statics.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/internal/Statics.java @@ -160,6 +160,14 @@ public interface Statics { dest.position(destPos).put(bbslice(src, srcPos, length)); } + static & Buffer> T asRS(Buffer buf) { + if (!(buf instanceof ResourceSupport)) { + throw new IllegalArgumentException("Buffer instance is not an instance of ResourceSupport."); + } + //noinspection unchecked + return (T) buf; + } + static IllegalStateException bufferIsClosed() { return new IllegalStateException("This buffer is closed."); } diff --git a/buffer-api/src/main/java/io/netty/buffer/api/TransferSend.java b/buffer-api/src/main/java/io/netty/buffer/api/internal/TransferSend.java similarity index 84% rename from buffer-api/src/main/java/io/netty/buffer/api/TransferSend.java rename to buffer-api/src/main/java/io/netty/buffer/api/internal/TransferSend.java index 1b8a0f9..0e806a1 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/TransferSend.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/internal/TransferSend.java @@ -13,15 +13,18 @@ * License for the specific language governing permissions and limitations * under the License. */ -package io.netty.buffer.api; +package io.netty.buffer.api.internal; -import io.netty.buffer.api.internal.Statics; +import io.netty.buffer.api.Drop; +import io.netty.buffer.api.Owned; +import io.netty.buffer.api.Resource; +import io.netty.buffer.api.Send; import java.lang.invoke.VarHandle; import static java.lang.invoke.MethodHandles.lookup; -class TransferSend, T extends Rc> implements Send { +public class TransferSend, T extends ResourceSupport> implements Send { private static final VarHandle RECEIVED = Statics.findVarHandle(lookup(), TransferSend.class, "received", boolean.class); private final Owned outgoing; private final Drop drop; @@ -29,7 +32,7 @@ class TransferSend, T extends Rc> implements Send { @SuppressWarnings("unused") private volatile boolean received; // Accessed via VarHandle - TransferSend(Owned outgoing, Drop drop, Class concreteType) { + public TransferSend(Owned outgoing, Drop drop, Class concreteType) { this.outgoing = outgoing; this.drop = drop; this.concreteType = concreteType; diff --git a/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java b/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java index a40c743..1c4d21c 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java @@ -21,7 +21,7 @@ import io.netty.buffer.api.Buffer; import io.netty.buffer.api.ByteCursor; import io.netty.buffer.api.Drop; import io.netty.buffer.api.Owned; -import io.netty.buffer.api.RcSupport; +import io.netty.buffer.api.internal.ResourceSupport; import io.netty.buffer.api.ReadableComponent; import io.netty.buffer.api.ReadableComponentProcessor; import io.netty.buffer.api.WritableComponent; @@ -39,7 +39,7 @@ import static io.netty.buffer.api.internal.Statics.bufferIsClosed; import static io.netty.buffer.api.internal.Statics.bufferIsReadOnly; import static io.netty.util.internal.PlatformDependent.BIG_ENDIAN_NATIVE_ORDER; -class UnsafeBuffer extends RcSupport implements Buffer, ReadableComponent, +class UnsafeBuffer extends ResourceSupport implements Buffer, ReadableComponent, WritableComponent { private static final int CLOSED_SIZE = -1; private static final boolean ACCESS_UNALIGNED = PlatformDependent.isUnaligned(); diff --git a/buffer-api/src/main/java/module-info.java b/buffer-api/src/main/java/module-info.java index ee28484..3c7185a 100644 --- a/buffer-api/src/main/java/module-info.java +++ b/buffer-api/src/main/java/module-info.java @@ -27,15 +27,13 @@ module netty.incubator.buffer { exports io.netty.buffer.api; exports io.netty.buffer.api.adaptor; - exports io.netty.buffer.api.internal to - netty.incubator.buffer.memseg, - netty.incubator.buffer.tests; - uses MemoryManagers; // Permit reflective access to non-public members. // Also means we don't have to make all test methods etc. public for JUnit to access them. - opens io.netty.buffer.api;//todo remove + opens io.netty.buffer.api; + exports io.netty.buffer.api.internal; + opens io.netty.buffer.api.internal;//todo remove provides MemoryManagers with ByteBufferMemoryManagers, diff --git a/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java b/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java index 7b04aff..b35173e 100644 --- a/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java +++ b/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java @@ -31,7 +31,7 @@ import io.netty.buffer.api.WritableComponent; import io.netty.buffer.api.WritableComponentProcessor; import io.netty.buffer.api.Drop; import io.netty.buffer.api.Owned; -import io.netty.buffer.api.RcSupport; +import io.netty.buffer.api.internal.ResourceSupport; import jdk.incubator.foreign.MemorySegment; import jdk.incubator.foreign.ResourceScope; @@ -53,8 +53,8 @@ import static jdk.incubator.foreign.MemoryAccess.setIntAtOffset; import static jdk.incubator.foreign.MemoryAccess.setLongAtOffset; import static jdk.incubator.foreign.MemoryAccess.setShortAtOffset; -class MemSegBuffer extends RcSupport implements Buffer, ReadableComponent, WritableComponent, - BufferIntegratable { +class MemSegBuffer extends ResourceSupport implements Buffer, ReadableComponent, + WritableComponent, BufferIntegratable { private static final MemorySegment CLOSED_SEGMENT; static final Drop SEGMENT_CLOSE; diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferBulkAccessTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferBulkAccessTest.java index fd59c33..1715e95 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferBulkAccessTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferBulkAccessTest.java @@ -118,7 +118,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { int second = size - first; try (var bufFirst = a.allocate(first); var bufSecond = b.allocate(second)) { - return CompositeBuffer.compose(a, bufFirst, bufSecond); + return CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send()); } }); } @@ -134,7 +134,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { int second = size - first; try (var bufFirst = a.allocate(first); var bufSecond = b.allocate(second)) { - return CompositeBuffer.compose(a, bufFirst, bufSecond); + return CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send()); } }); } @@ -150,7 +150,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { int second = size - first; try (var bufFirst = a.allocate(first); var bufSecond = b.allocate(second)) { - return CompositeBuffer.compose(a, bufFirst, bufSecond); + return CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send()); } }); } @@ -166,7 +166,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { int second = size - first; try (var bufFirst = a.allocate(first); var bufSecond = b.allocate(second)) { - return CompositeBuffer.compose(a, bufFirst, bufSecond); + return CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send()); } }); } @@ -183,7 +183,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { int second = size - first; try (var bufFirst = a.allocate(first); var bufSecond = b.allocate(second)) { - return scope.add(CompositeBuffer.compose(a, bufFirst, bufSecond)).writerOffset(size).slice(); + return scope.add(CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send())).writerOffset(size).slice(); } }); } @@ -200,7 +200,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { int second = size - first; try (var bufFirst = a.allocate(first); var bufSecond = b.allocate(second)) { - return scope.add(CompositeBuffer.compose(a, bufFirst, bufSecond)).writerOffset(size).slice(); + return scope.add(CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send())).writerOffset(size).slice(); } }); } @@ -217,7 +217,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { int second = size - first; try (var bufFirst = a.allocate(first); var bufSecond = b.allocate(second)) { - return scope.add(CompositeBuffer.compose(a, bufFirst, bufSecond)).writerOffset(size).slice(); + return scope.add(CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send())).writerOffset(size).slice(); } }); } @@ -234,7 +234,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { int second = size - first; try (var bufFirst = a.allocate(first); var bufSecond = b.allocate(second)) { - return scope.add(CompositeBuffer.compose(a, bufFirst, bufSecond)).writerOffset(size).slice(); + return scope.add(CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send())).writerOffset(size).slice(); } }); } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompactTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompactTest.java index ba79516..4899bb0 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompactTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompactTest.java @@ -20,6 +20,7 @@ import io.netty.buffer.api.BufferAllocator; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; +import static io.netty.buffer.api.internal.Statics.asRS; import static java.nio.ByteOrder.BIG_ENDIAN; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -54,7 +55,7 @@ public class BufferCompactTest extends BufferTestSupport { Buffer buf = allocator.allocate(8, BIG_ENDIAN)) { buf.writeLong(0x0102030405060708L); assertEquals((byte) 0x01, buf.readByte()); - try (Buffer ignore = buf.acquire()) { + try (var ignore = asRS(buf).acquire()) { assertThrows(IllegalStateException.class, () -> buf.compact()); assertEquals(1, buf.readerOffset()); } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferComponentIterationTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferComponentIterationTest.java index 006481e..0f3b541 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferComponentIterationTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferComponentIterationTest.java @@ -73,8 +73,8 @@ public class BufferComponentIterationTest extends BufferTestSupport { try (Buffer a = allocator.allocate(8); Buffer b = allocator.allocate(8); Buffer c = allocator.allocate(8); - Buffer x = CompositeBuffer.compose(allocator, b, c)) { - buf = CompositeBuffer.compose(allocator, a, x); + Buffer x = CompositeBuffer.compose(allocator, b.send(), c.send())) { + buf = CompositeBuffer.compose(allocator, a.send(), x.send()); } assertThat(buf.countComponents()).isEqualTo(3); assertThat(buf.countReadableComponents()).isZero(); @@ -126,7 +126,7 @@ public class BufferComponentIterationTest extends BufferTestSupport { a.writeInt(1); b.writeInt(2); c.writeInt(3); - composite = CompositeBuffer.compose(allocator, a, b, c); + composite = CompositeBuffer.compose(allocator, a.send(), b.send(), c.send()); } var list = new LinkedList(List.of(1, 2, 3)); int count = composite.forEachReadable(0, (index, component) -> { @@ -163,7 +163,7 @@ public class BufferComponentIterationTest extends BufferTestSupport { a.writeInt(1); b.writeInt(2); c.writeInt(3); - composite = CompositeBuffer.compose(allocator, a, b, c); + composite = CompositeBuffer.compose(allocator, a.send(), b.send(), c.send()); } int readPos = composite.readerOffset(); int writePos = composite.writerOffset(); @@ -201,7 +201,7 @@ public class BufferComponentIterationTest extends BufferTestSupport { try (Buffer a = allocator.allocate(4); Buffer b = allocator.allocate(4); Buffer c = allocator.allocate(4)) { - buf = CompositeBuffer.compose(allocator, a, b, c); + buf = CompositeBuffer.compose(allocator, a.send(), b.send(), c.send()); } int i = 1; while (buf.writableBytes() > 0) { @@ -273,7 +273,7 @@ public class BufferComponentIterationTest extends BufferTestSupport { try (Buffer a = allocator.allocate(8); Buffer b = allocator.allocate(8); Buffer c = allocator.allocate(8)) { - buf = CompositeBuffer.compose(allocator, a, b, c); + buf = CompositeBuffer.compose(allocator, a.send(), b.send(), c.send()); } buf.order(BIG_ENDIAN); buf.forEachWritable(0, (index, component) -> { diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java index e003b5d..c57e191 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java @@ -18,12 +18,14 @@ package io.netty.buffer.api.tests; import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; import io.netty.buffer.api.CompositeBuffer; +import io.netty.buffer.api.Send; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; import java.nio.ByteOrder; +import static io.netty.buffer.api.internal.Statics.asRS; import static java.nio.ByteOrder.BIG_ENDIAN; import static java.nio.ByteOrder.LITTLE_ENDIAN; import static org.assertj.core.api.Assertions.assertThat; @@ -35,14 +37,14 @@ public class BufferCompositionTest extends BufferTestSupport { @Test public void compositeBufferCanOnlyBeOwnedWhenAllConstituentBuffersAreOwned() { try (BufferAllocator allocator = BufferAllocator.heap()) { - Buffer composite; - try (Buffer a = allocator.allocate(8)) { + var composite = asRS(CompositeBuffer.compose(allocator)); + try (var a = asRS(allocator.allocate(8))) { assertTrue(a.isOwned()); Buffer leakB; - try (Buffer b = allocator.allocate(8)) { + try (var b = asRS(allocator.allocate(8))) { assertTrue(a.isOwned()); assertTrue(b.isOwned()); - composite = CompositeBuffer.compose(allocator, a, b); + composite = asRS(CompositeBuffer.compose(allocator, a.send(), b.send())); assertFalse(composite.isOwned()); assertFalse(a.isOwned()); assertFalse(b.isOwned()); @@ -50,7 +52,7 @@ public class BufferCompositionTest extends BufferTestSupport { } assertFalse(composite.isOwned()); assertFalse(a.isOwned()); - assertTrue(leakB.isOwned()); + assertTrue(asRS(leakB).isOwned()); } assertTrue(composite.isOwned()); } @@ -58,20 +60,16 @@ public class BufferCompositionTest extends BufferTestSupport { @Test public void compositeBuffersCannotHaveDuplicateComponents() { - try (BufferAllocator allocator = BufferAllocator.heap(); - Buffer a = allocator.allocate(4)) { + try (BufferAllocator allocator = BufferAllocator.heap()) { + Send a = allocator.allocate(4).send(); var e = assertThrows(IllegalArgumentException.class, () -> CompositeBuffer.compose(allocator, a, a)); assertThat(e).hasMessageContaining("duplicate"); - try (CompositeBuffer composite = CompositeBuffer.compose(allocator, a)) { - a.close(); - try { - e = assertThrows(IllegalArgumentException.class, - () -> composite.extendWith(a)); - assertThat(e).hasMessageContaining("duplicate"); - } finally { - a.acquire(); - } + Send b = allocator.allocate(4).send(); + try (CompositeBuffer composite = CompositeBuffer.compose(allocator, b)) { + e = assertThrows(IllegalArgumentException.class, + () -> composite.extendWith(b)); + assertThat(e).hasMessageContaining("duplicate"); } } } @@ -84,7 +82,7 @@ public class BufferCompositionTest extends BufferTestSupport { allocator.allocate(8).send(), allocator.allocate(8).send())) { assertEquals(24, composite.capacity()); - assertTrue(composite.isOwned()); + assertTrue(asRS(composite).isOwned()); } } @@ -92,26 +90,26 @@ public class BufferCompositionTest extends BufferTestSupport { public void compositeBufferMustNotBeAllowedToContainThemselves() { try (BufferAllocator allocator = BufferAllocator.heap()) { Buffer a = allocator.allocate(4); - CompositeBuffer buf = CompositeBuffer.compose(allocator, a); + CompositeBuffer buf = CompositeBuffer.compose(allocator, a.send()); try (buf; a) { a.close(); try { - assertThrows(IllegalArgumentException.class, () -> buf.extendWith(buf)); + assertThrows(IllegalArgumentException.class, () -> buf.extendWith(buf.send())); assertTrue(buf.isOwned()); - try (Buffer composite = CompositeBuffer.compose(allocator, buf)) { + try (Buffer composite = CompositeBuffer.compose(allocator, buf.send())) { // the composing increments the reference count of constituent buffers... // counter-act this, so it can be extended: a.close(); // buf is now owned, so it can be extended. try { assertThrows(IllegalArgumentException.class, - () -> buf.extendWith(composite)); + () -> buf.extendWith(composite.send())); } finally { - a.acquire(); // restore the reference count to align with our try-with-resources structure. + asRS(a).acquire(); // restore the reference count to align with our try-with-resources structure. } } assertTrue(buf.isOwned()); } finally { - a.acquire(); + asRS(a).acquire(); } } } @@ -123,7 +121,7 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator()) { Buffer composite; try (Buffer a = allocator.allocate(4, BIG_ENDIAN)) { - composite = CompositeBuffer.compose(allocator, a); + composite = CompositeBuffer.compose(allocator, a.send()); } try (composite) { composite.writeInt(0x01020304); @@ -137,17 +135,12 @@ public class BufferCompositionTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") public void ensureWritableOnCompositeBuffersMustRespectExistingLittleEndianByteOrder(Fixture fixture) { - try (BufferAllocator allocator = fixture.createAllocator()) { - Buffer composite; - try (Buffer a = allocator.allocate(4, LITTLE_ENDIAN)) { - composite = CompositeBuffer.compose(allocator, a); - } - try (composite) { - composite.writeInt(0x05060708); - composite.ensureWritable(4); - composite.writeInt(0x01020304); - assertEquals(0x0102030405060708L, composite.readLong()); - } + try (BufferAllocator allocator = fixture.createAllocator(); + Buffer composite = CompositeBuffer.compose(allocator, allocator.allocate(4, LITTLE_ENDIAN).send())) { + composite.writeInt(0x05060708); + composite.ensureWritable(4); + composite.writeInt(0x01020304); + assertEquals(0x0102030405060708L, composite.readLong()); } } @@ -164,7 +157,7 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap(); Buffer a = allocator.allocate(8); Buffer b = allocator.allocate(8)) { - assertThrows(ClassCastException.class, () -> ((CompositeBuffer) a).extendWith(b)); + assertThrows(ClassCastException.class, () -> ((CompositeBuffer) a).extendWith(b.send())); } } @@ -173,9 +166,9 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap(); Buffer a = allocator.allocate(8); Buffer b = allocator.allocate(8); - CompositeBuffer composed = CompositeBuffer.compose(allocator, a)) { + CompositeBuffer composed = CompositeBuffer.compose(allocator, a.send())) { try (Buffer ignore = composed.acquire()) { - var exc = assertThrows(IllegalStateException.class, () -> composed.extendWith(b)); + var exc = assertThrows(IllegalStateException.class, () -> composed.extendWith(b.send())); assertThat(exc).hasMessageContaining("owned"); } } @@ -186,11 +179,11 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap()) { CompositeBuffer composite; try (Buffer a = allocator.allocate(8)) { - composite = CompositeBuffer.compose(allocator, a); + composite = CompositeBuffer.compose(allocator, a.send()); } try (composite) { var exc = assertThrows(IllegalArgumentException.class, - () -> composite.extendWith(composite)); + () -> composite.extendWith(composite.send())); assertThat(exc).hasMessageContaining("cannot be extended"); } } @@ -200,19 +193,18 @@ public class BufferCompositionTest extends BufferTestSupport { public void extendingWithZeroCapacityBufferHasNoEffect() { try (BufferAllocator allocator = BufferAllocator.heap(); CompositeBuffer composite = CompositeBuffer.compose(allocator)) { - composite.extendWith(composite); + composite.extendWith(CompositeBuffer.compose(allocator).send()); assertThat(composite.capacity()).isZero(); assertThat(composite.countComponents()).isZero(); } try (BufferAllocator allocator = BufferAllocator.heap()) { Buffer a = allocator.allocate(1); - CompositeBuffer composite = CompositeBuffer.compose(allocator, a); - a.close(); + CompositeBuffer composite = CompositeBuffer.compose(allocator, a.send()); assertTrue(composite.isOwned()); assertThat(composite.capacity()).isOne(); assertThat(composite.countComponents()).isOne(); try (Buffer b = CompositeBuffer.compose(allocator)) { - composite.extendWith(b); + composite.extendWith(b.send()); } assertTrue(composite.isOwned()); assertThat(composite.capacity()).isOne(); @@ -234,7 +226,7 @@ public class BufferCompositionTest extends BufferTestSupport { CompositeBuffer composite = CompositeBuffer.compose(allocator)) { assertThat(composite.capacity()).isZero(); try (Buffer buf = allocator.allocate(8, BIG_ENDIAN)) { - composite.extendWith(buf); + composite.extendWith(buf.send()); } assertThat(composite.capacity()).isEqualTo(8); composite.writeLong(0x0102030405060708L); @@ -248,7 +240,7 @@ public class BufferCompositionTest extends BufferTestSupport { CompositeBuffer composite = CompositeBuffer.compose(allocator)) { assertThat(composite.capacity()).isZero(); try (Buffer buf = allocator.allocate(8, LITTLE_ENDIAN)) { - composite.extendWith(buf); + composite.extendWith(buf.send()); } assertThat(composite.capacity()).isEqualTo(8); composite.writeLong(0x0102030405060708L); @@ -261,12 +253,12 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap()) { CompositeBuffer composite; try (Buffer a = allocator.allocate(8, BIG_ENDIAN)) { - composite = CompositeBuffer.compose(allocator, a); + composite = CompositeBuffer.compose(allocator, a.send()); } try (composite) { try (Buffer b = allocator.allocate(8, LITTLE_ENDIAN)) { var exc = assertThrows(IllegalArgumentException.class, - () -> composite.extendWith(b)); + () -> composite.extendWith(b.send())); assertThat(exc).hasMessageContaining("byte order"); } } @@ -278,12 +270,12 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap()) { CompositeBuffer composite; try (Buffer a = allocator.allocate(8, LITTLE_ENDIAN)) { - composite = CompositeBuffer.compose(allocator, a); + composite = CompositeBuffer.compose(allocator, a.send()); } try (composite) { try (Buffer b = allocator.allocate(8, BIG_ENDIAN)) { var exc = assertThrows(IllegalArgumentException.class, - () -> composite.extendWith(b)); + () -> composite.extendWith(b.send())); assertThat(exc).hasMessageContaining("byte order"); } } @@ -295,7 +287,7 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap()) { try (CompositeBuffer composite = CompositeBuffer.compose(allocator)) { try (Buffer b = allocator.allocate(8, BIG_ENDIAN)) { - composite.extendWith(b); + composite.extendWith(b.send()); assertThat(composite.order()).isEqualTo(BIG_ENDIAN); } } @@ -307,7 +299,7 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap()) { try (CompositeBuffer composite = CompositeBuffer.compose(allocator)) { try (Buffer b = allocator.allocate(8, LITTLE_ENDIAN)) { - composite.extendWith(b); + composite.extendWith(b.send()); assertThat(composite.order()).isEqualTo(LITTLE_ENDIAN); } } @@ -319,7 +311,7 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap()) { try (CompositeBuffer composite = CompositeBuffer.compose(allocator)) { try (Buffer b = allocator.allocate(8).makeReadOnly()) { - composite.extendWith(b); + composite.extendWith(b.send()); assertTrue(composite.readOnly()); } } @@ -331,13 +323,13 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap()) { CompositeBuffer composite; try (Buffer a = allocator.allocate(8)) { - composite = CompositeBuffer.compose(allocator, a); + composite = CompositeBuffer.compose(allocator, a.send()); } try (composite) { composite.writeLong(0); try (Buffer b = allocator.allocate(8)) { b.writeInt(1); - composite.extendWith(b); + composite.extendWith(b.send()); assertThat(composite.capacity()).isEqualTo(16); assertThat(composite.writerOffset()).isEqualTo(12); } @@ -350,17 +342,19 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap()) { CompositeBuffer composite; try (Buffer a = allocator.allocate(8)) { - composite = CompositeBuffer.compose(allocator, a); + composite = CompositeBuffer.compose(allocator, a.send()); } try (composite) { composite.writeInt(0); try (Buffer b = allocator.allocate(8)) { b.writeInt(1); var exc = assertThrows(IllegalArgumentException.class, - () -> composite.extendWith(b)); + () -> composite.extendWith(b.send())); assertThat(exc).hasMessageContaining("unwritten gap"); - b.writerOffset(0); - composite.extendWith(b); + } + try (Buffer b = allocator.allocate(8)) { + b.setInt(0, 1); + composite.extendWith(b.send()); assertThat(composite.capacity()).isEqualTo(16); assertThat(composite.writerOffset()).isEqualTo(4); } @@ -373,7 +367,7 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap()) { CompositeBuffer composite; try (Buffer a = allocator.allocate(8)) { - composite = CompositeBuffer.compose(allocator, a); + composite = CompositeBuffer.compose(allocator, a.send()); } try (composite) { composite.writeLong(0); @@ -381,7 +375,7 @@ public class BufferCompositionTest extends BufferTestSupport { try (Buffer b = allocator.allocate(8)) { b.writeInt(1); b.readInt(); - composite.extendWith(b); + composite.extendWith(b.send()); assertThat(composite.capacity()).isEqualTo(16); assertThat(composite.writerOffset()).isEqualTo(12); } @@ -394,7 +388,7 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap()) { CompositeBuffer composite; try (Buffer a = allocator.allocate(8)) { - composite = CompositeBuffer.compose(allocator, a); + composite = CompositeBuffer.compose(allocator, a.send()); } try (composite) { composite.writeLong(0); @@ -403,10 +397,10 @@ public class BufferCompositionTest extends BufferTestSupport { b.writeInt(1); b.readInt(); var exc = assertThrows(IllegalArgumentException.class, - () -> composite.extendWith(b)); + () -> composite.extendWith(b.send())); assertThat(exc).hasMessageContaining("unread gap"); b.readerOffset(0); - composite.extendWith(b); + composite.extendWith(b.send()); assertThat(composite.capacity()).isEqualTo(16); assertThat(composite.writerOffset()).isEqualTo(12); assertThat(composite.readerOffset()).isEqualTo(4); @@ -420,7 +414,7 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap(); Buffer a = allocator.allocate(4, BIG_ENDIAN); Buffer b = allocator.allocate(4, LITTLE_ENDIAN)) { - assertThrows(IllegalArgumentException.class, () -> CompositeBuffer.compose(allocator, a, b)); + assertThrows(IllegalArgumentException.class, () -> CompositeBuffer.compose(allocator, a.send(), b.send())); } } @@ -429,7 +423,7 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap(); Buffer a = allocator.allocate(4).makeReadOnly(); Buffer b = allocator.allocate(4).makeReadOnly(); - Buffer composite = CompositeBuffer.compose(allocator, a, b)) { + Buffer composite = CompositeBuffer.compose(allocator, a.send(), b.send())) { assertTrue(composite.readOnly()); verifyWriteInaccessible(composite); } @@ -437,13 +431,27 @@ public class BufferCompositionTest extends BufferTestSupport { @Test public void composingReadOnlyAndWritableBuffersMustThrow() { - try (BufferAllocator allocator = BufferAllocator.heap(); - Buffer a = allocator.allocate(8).makeReadOnly(); - Buffer b = allocator.allocate(8)) { - assertThrows(IllegalArgumentException.class, () -> CompositeBuffer.compose(allocator, a, b)); - assertThrows(IllegalArgumentException.class, () -> CompositeBuffer.compose(allocator, b, a)); - assertThrows(IllegalArgumentException.class, () -> CompositeBuffer.compose(allocator, a, b, a)); - assertThrows(IllegalArgumentException.class, () -> CompositeBuffer.compose(allocator, b, a, b)); + try (BufferAllocator allocator = BufferAllocator.heap()) { + try (Buffer a = allocator.allocate(8).makeReadOnly(); + Buffer b = allocator.allocate(8)) { + assertThrows(IllegalArgumentException.class, () -> CompositeBuffer.compose(allocator, a.send(), b.send())); + } + try (Buffer a = allocator.allocate(8).makeReadOnly(); + Buffer b = allocator.allocate(8)) { + assertThrows(IllegalArgumentException.class, () -> CompositeBuffer.compose(allocator, b.send(), a.send())); + } + try (Buffer a = allocator.allocate(8).makeReadOnly(); + Buffer b = allocator.allocate(8); + Buffer c = allocator.allocate(8).makeReadOnly()) { + assertThrows(IllegalArgumentException.class, + () -> CompositeBuffer.compose(allocator, a.send(), b.send(), c.send())); + } + try (Buffer a = allocator.allocate(8).makeReadOnly(); + Buffer b = allocator.allocate(8); + Buffer c = allocator.allocate(8)) { + assertThrows(IllegalArgumentException.class, + () -> CompositeBuffer.compose(allocator, b.send(), a.send(), c.send())); + } } } @@ -452,10 +460,10 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap()) { CompositeBuffer composite; try (Buffer a = allocator.allocate(8)) { - composite = CompositeBuffer.compose(allocator, a); + composite = CompositeBuffer.compose(allocator, a.send()); } try (composite; Buffer b = allocator.allocate(8).makeReadOnly()) { - assertThrows(IllegalArgumentException.class, () -> composite.extendWith(b)); + assertThrows(IllegalArgumentException.class, () -> composite.extendWith(b.send())); } } } @@ -465,10 +473,10 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap()) { CompositeBuffer composite; try (Buffer a = allocator.allocate(8).makeReadOnly()) { - composite = CompositeBuffer.compose(allocator, a); + composite = CompositeBuffer.compose(allocator, a.send()); } try (composite; Buffer b = allocator.allocate(8)) { - assertThrows(IllegalArgumentException.class, () -> composite.extendWith(b)); + assertThrows(IllegalArgumentException.class, () -> composite.extendWith(b.send())); } } } @@ -478,7 +486,7 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap(); Buffer a = allocator.allocate(8); Buffer b = allocator.allocate(8); - CompositeBuffer composite = CompositeBuffer.compose(allocator, a, b)) { + CompositeBuffer composite = CompositeBuffer.compose(allocator, a.send(), b.send())) { assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(0)); assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(4)); assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(7)); @@ -494,7 +502,7 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap(); Buffer a = allocator.allocate(8); Buffer b = allocator.allocate(8); - CompositeBuffer composite = CompositeBuffer.compose(allocator, a, b)) { + CompositeBuffer composite = CompositeBuffer.compose(allocator, a.send(), b.send())) { assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(0)); assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(4)); assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(7)); diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java index 8626cc0..8be646c 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java @@ -36,7 +36,7 @@ public class BufferEnsureWritableTest extends BufferTestSupport { assertThrows(IllegalStateException.class, () -> slice.ensureWritable(1)); assertThrows(IllegalStateException.class, () -> buf.ensureWritable(1)); } - try (Buffer compose = CompositeBuffer.compose(allocator, buf)) { + try (Buffer compose = CompositeBuffer.compose(allocator, buf.send())) { assertThrows(IllegalStateException.class, () -> compose.ensureWritable(1)); assertThrows(IllegalStateException.class, () -> buf.ensureWritable(1)); } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java index 152be4b..7e3d632 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java @@ -26,6 +26,7 @@ import org.junit.jupiter.params.provider.MethodSource; import java.nio.ByteOrder; import java.util.function.Supplier; +import static io.netty.buffer.api.internal.Statics.asRS; import static java.nio.ByteOrder.BIG_ENDIAN; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -164,7 +165,7 @@ public class BufferReadOnlyTest extends BufferTestSupport { assertThat(buf.readerOffset()).isZero(); assertThat(buf.capacity()).isEqualTo(4); assertThat(buf.writerOffset()).isEqualTo(4); - assertTrue(buf.isOwned()); + assertTrue(asRS(buf).isOwned()); assertTrue(buf.isAccessible()); assertThat(buf.countComponents()).isOne(); assertEquals((byte) 1, buf.readByte()); @@ -190,14 +191,14 @@ public class BufferReadOnlyTest extends BufferTestSupport { Buffer b = a.split(8)) { assertTrue(a.readOnly()); assertTrue(b.readOnly()); - assertTrue(a.isOwned()); - assertTrue(b.isOwned()); + assertTrue(asRS(a).isOwned()); + assertTrue(asRS(b).isOwned()); assertThat(a.capacity()).isEqualTo(8); assertThat(b.capacity()).isEqualTo(8); try (Buffer c = b.slice()) { assertTrue(c.readOnly()); - assertFalse(c.isOwned()); - assertFalse(b.isOwned()); + assertFalse(asRS(c).isOwned()); + assertFalse(asRS(b).isOwned()); assertThat(c.capacity()).isEqualTo(8); } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferRefTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferRefTest.java index d6f3abf..e71743a 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferRefTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferRefTest.java @@ -21,6 +21,8 @@ import io.netty.buffer.api.BufferRef; import io.netty.buffer.api.Send; import org.junit.jupiter.api.Test; +import java.util.concurrent.atomic.AtomicReference; + import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.*; @@ -30,7 +32,7 @@ class BufferRefTest { try (BufferAllocator allocator = BufferAllocator.heap()) { BufferRef ref; try (Buffer b = allocator.allocate(8)) { - ref = new BufferRef(b); + ref = new BufferRef(b.send()); } ref.contents().writeInt(42); assertThat(ref.contents().readInt()).isEqualTo(42); @@ -54,20 +56,23 @@ class BufferRefTest { @Test public void mustCloseOwnedBufferWhenReplaced() { try (BufferAllocator allocator = BufferAllocator.heap()) { - Buffer orig; + AtomicReference orig = new AtomicReference<>(); BufferRef ref; - try (Buffer buf = allocator.allocate(8)) { - ref = new BufferRef(orig = buf); - } + Send s = allocator.allocate(8).send(); + ref = new BufferRef(Send.sending(Buffer.class, () -> { + Buffer b = s.receive(); + orig.set(b); + return b; + })); - orig.writeInt(42); + orig.get().writeInt(42); assertThat(ref.contents().readInt()).isEqualTo(42); try (Buffer buf = allocator.allocate(8)) { - ref.replace(buf); // Pass replacement directly. + ref.replace(buf.send()); // Pass replacement directly. } - assertThrows(IllegalStateException.class, () -> orig.writeInt(32)); + assertThrows(IllegalStateException.class, () -> orig.get().writeInt(32)); ref.contents().writeInt(42); assertThat(ref.contents().readInt()).isEqualTo(42); ref.close(); @@ -78,20 +83,23 @@ class BufferRefTest { @Test public void mustCloseOwnedBufferWhenReplacedFromSend() { try (BufferAllocator allocator = BufferAllocator.heap()) { - Buffer orig; + AtomicReference orig = new AtomicReference<>(); BufferRef ref; - try (Buffer buf = allocator.allocate(8)) { - ref = new BufferRef(orig = buf); - } + Send s = allocator.allocate(8).send(); + ref = new BufferRef(Send.sending(Buffer.class, () -> { + Buffer b = s.receive(); + orig.set(b); + return b; + })); - orig.writeInt(42); + orig.get().writeInt(42); assertThat(ref.contents().readInt()).isEqualTo(42); try (Buffer buf = allocator.allocate(8)) { ref.replace(buf.send()); // Pass replacement via send(). } - assertThrows(IllegalStateException.class, () -> orig.writeInt(32)); + assertThrows(IllegalStateException.class, () -> orig.get().writeInt(32)); ref.contents().writeInt(42); assertThat(ref.contents().readInt()).isEqualTo(42); ref.close(); diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java index 147d936..8d89a82 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java @@ -22,10 +22,10 @@ import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; -import java.nio.ByteOrder; import java.util.concurrent.Future; import java.util.concurrent.ThreadLocalRandom; +import static io.netty.buffer.api.internal.Statics.asRS; import static java.nio.ByteOrder.BIG_ENDIAN; import static java.nio.ByteOrder.LITTLE_ENDIAN; import static org.assertj.core.api.Assertions.assertThat; @@ -41,7 +41,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { Buffer buf = allocator.allocate(8)) { buf.writeByte((byte) 1); buf.writeByte((byte) 2); - try (Buffer inner = buf.acquire()) { + try (Buffer inner = asRS(buf).acquire()) { inner.writeByte((byte) 3); inner.writeByte((byte) 4); inner.writeByte((byte) 5); @@ -75,7 +75,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator()) { var buf = allocator.allocate(8); buf.close(); - assertThrows(IllegalStateException.class, buf::acquire); + assertThrows(IllegalStateException.class, () -> asRS(buf).acquire()); } } @@ -166,10 +166,10 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { try (Buffer ignored = buf.slice()) { - assertFalse(buf.isOwned()); + assertFalse(asRS(buf).isOwned()); assertThrows(IllegalStateException.class, buf::send); } - assertTrue(buf.isOwned()); + assertTrue(asRS(buf).isOwned()); } } @@ -179,10 +179,10 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { try (Buffer ignored = buf.slice(0, 8)) { - assertFalse(buf.isOwned()); + assertFalse(asRS(buf).isOwned()); assertThrows(IllegalStateException.class, buf::send); } - assertTrue(buf.isOwned()); + assertTrue(asRS(buf).isOwned()); } } @@ -226,11 +226,11 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { try (Buffer slice = buf.slice()) { - assertFalse(buf.isOwned()); + assertFalse(asRS(buf).isOwned()); assertThrows(IllegalStateException.class, slice::send); } // Verify that the slice is closed properly afterwards. - assertTrue(buf.isOwned()); + assertTrue(asRS(buf).isOwned()); buf.send().receive().close(); } } @@ -241,11 +241,11 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { try (Buffer slice = buf.slice(0, 8)) { - assertFalse(buf.isOwned()); + assertFalse(asRS(buf).isOwned()); assertThrows(IllegalStateException.class, slice::send); } // Verify that the slice is closed properly afterwards. - assertTrue(buf.isOwned()); + assertTrue(asRS(buf).isOwned()); } } @@ -256,7 +256,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { Buffer buf = allocator.allocate(8)) { assertThrows(IndexOutOfBoundsException.class, () -> buf.slice(-1, 1)); // Verify that the slice is closed properly afterwards. - assertTrue(buf.isOwned()); + assertTrue(asRS(buf).isOwned()); } } @@ -268,7 +268,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { assertThrows(IllegalArgumentException.class, () -> buf.slice(0, -1)); assertThrows(IllegalArgumentException.class, () -> buf.slice(2, -1)); // Verify that the slice is closed properly afterwards. - assertTrue(buf.isOwned()); + assertTrue(asRS(buf).isOwned()); } } @@ -281,7 +281,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { buf.slice(0, 8).close(); // This is still fine. assertThrows(IndexOutOfBoundsException.class, () -> buf.slice(1, 8)); // Verify that the slice is closed properly afterwards. - assertTrue(buf.isOwned()); + assertTrue(asRS(buf).isOwned()); } } @@ -292,7 +292,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { Buffer buf = allocator.allocate(8)) { buf.slice(0, 0).close(); // This is fine. // Verify that the slice is closed properly afterwards. - assertTrue(buf.isOwned()); + assertTrue(asRS(buf).isOwned()); } } @@ -302,13 +302,13 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { int borrows = countBorrows(buf); - try (Buffer ignored = buf.acquire()) { + try (Buffer ignored = asRS(buf).acquire()) { assertEquals(borrows + 1, countBorrows(buf)); try (Buffer slice = buf.slice()) { assertEquals(0, slice.capacity()); // We haven't written anything, so the slice is empty. int sliceBorrows = countBorrows(slice); assertEquals(borrows + 2, countBorrows(buf)); - try (Buffer ignored1 = CompositeBuffer.compose(allocator, buf, slice)) { + try (Buffer ignored1 = CompositeBuffer.compose(allocator, buf.send(), slice.send())) { assertEquals(borrows + 3, countBorrows(buf)); // Note: Slice is empty; not acquired by the composite buffer. assertEquals(sliceBorrows, countBorrows(slice)); @@ -329,13 +329,13 @@ public class BufferReferenceCountingTest extends BufferTestSupport { Buffer buf = allocator.allocate(8)) { buf.writeByte((byte) 1); int borrows = countBorrows(buf); - try (Buffer ignored = buf.acquire()) { + try (Buffer ignored = asRS(buf).acquire()) { assertEquals(borrows + 1, countBorrows(buf)); try (Buffer slice = buf.slice()) { assertEquals(1, slice.capacity()); int sliceBorrows = countBorrows(slice); assertEquals(borrows + 2, countBorrows(buf)); - try (Buffer ignored1 = CompositeBuffer.compose(allocator, buf, slice)) { + try (Buffer ignored1 = CompositeBuffer.compose(allocator, buf.send(), slice.send())) { assertEquals(borrows + 3, countBorrows(buf)); assertEquals(sliceBorrows + 1, countBorrows(slice)); } @@ -345,7 +345,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { assertEquals(borrows + 1, countBorrows(buf)); } assertEquals(borrows, countBorrows(buf)); - assertTrue(buf.isOwned()); + assertTrue(asRS(buf).isOwned()); } } @@ -358,9 +358,9 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (Buffer slice = buf.slice()) { buf.close(); assertFalse(buf.isAccessible()); - assertTrue(slice.isOwned()); + assertTrue(asRS(slice).isOwned()); try (Buffer receive = slice.send().receive()) { - assertTrue(receive.isOwned()); + assertTrue(asRS(receive).isOwned()); assertFalse(slice.isAccessible()); } } @@ -425,7 +425,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { buf.writeInt(1); - try (Buffer acquired = buf.acquire()) { + try (Buffer acquired = asRS(buf).acquire()) { var exc = assertThrows(IllegalStateException.class, () -> acquired.split()); assertThat(exc).hasMessageContaining("owned"); } @@ -437,7 +437,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { public void splitOnOffsetOfNonOwnedBufferMustThrow(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { - try (Buffer acquired = buf.acquire()) { + try (Buffer acquired = asRS(buf).acquire()) { var exc = assertThrows(IllegalStateException.class, () -> acquired.split(4)); assertThat(exc).hasMessageContaining("owned"); } @@ -580,7 +580,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { buf.writeLong(0x0102030405060708L); try (Buffer slice = buf.slice()) { buf.close(); - assertTrue(slice.isOwned()); + assertTrue(asRS(slice).isOwned()); try (Buffer split = slice.split(4)) { split.reset().ensureWritable(Long.BYTES); slice.reset().ensureWritable(Long.BYTES); @@ -710,7 +710,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { buf.makeReadOnly(); - try (Buffer acquire = buf.acquire()) { + try (Buffer acquire = asRS(buf).acquire()) { assertTrue(acquire.readOnly()); } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferSendTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferSendTest.java index ba71bae..50aef5c 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferSendTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferSendTest.java @@ -27,6 +27,7 @@ import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.Future; import java.util.concurrent.SynchronousQueue; +import static io.netty.buffer.api.internal.Statics.asRS; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -77,12 +78,12 @@ public class BufferSendTest extends BufferTestSupport { void sendMustThrowWhenBufIsAcquired(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { - try (Buffer ignored = buf.acquire()) { - assertFalse(buf.isOwned()); + try (Buffer ignored = asRS(buf).acquire()) { + assertFalse(asRS(buf).isOwned()); assertThrows(IllegalStateException.class, buf::send); } // Now send() should work again. - assertTrue(buf.isOwned()); + assertTrue(asRS(buf).isOwned()); buf.send().receive().close(); } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java index 4fc8d48..602fd2c 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java @@ -19,7 +19,7 @@ import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; import io.netty.buffer.api.CompositeBuffer; import io.netty.buffer.api.MemoryManagers; -import io.netty.buffer.api.RcSupport; +import io.netty.buffer.api.internal.ResourceSupport; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -43,6 +43,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.stream.Stream.Builder; +import static io.netty.buffer.api.internal.Statics.asRS; import static java.nio.ByteOrder.BIG_ENDIAN; import static java.nio.ByteOrder.LITTLE_ENDIAN; import static org.assertj.core.api.Assertions.assertThat; @@ -176,7 +177,7 @@ public abstract class BufferTestSupport { int half = size / 2; try (Buffer firstHalf = a.allocate(half); Buffer secondHalf = b.allocate(size - half)) { - return CompositeBuffer.compose(a, firstHalf, secondHalf); + return CompositeBuffer.compose(a, firstHalf.send(), secondHalf.send()); } } @@ -207,7 +208,7 @@ public abstract class BufferTestSupport { try (Buffer a = alloc.allocate(part); Buffer b = alloc.allocate(part); Buffer c = alloc.allocate(size - part * 2)) { - return CompositeBuffer.compose(alloc, a, b, c); + return CompositeBuffer.compose(alloc, a.send(), b.send(), c.send()); } } @@ -384,13 +385,13 @@ public abstract class BufferTestSupport { assertThrows(IllegalStateException.class, () -> buf.copyInto(0, new byte[1], 0, 1)); assertThrows(IllegalStateException.class, () -> buf.copyInto(0, ByteBuffer.allocate(1), 0, 1)); if (CompositeBuffer.isComposite(buf)) { - assertThrows(IllegalStateException.class, () -> ((CompositeBuffer) buf).extendWith(target)); + assertThrows(IllegalStateException.class, () -> ((CompositeBuffer) buf).extendWith(target.send())); } } assertThrows(IllegalStateException.class, () -> buf.split()); - assertThrows(IllegalStateException.class, () -> buf.send()); - assertThrows(IllegalStateException.class, () -> buf.acquire()); + assertThrows(IllegalStateException.class, () -> asRS(buf).send()); + assertThrows(IllegalStateException.class, () -> asRS(buf).acquire()); assertThrows(IllegalStateException.class, () -> buf.slice()); assertThrows(IllegalStateException.class, () -> buf.openCursor()); assertThrows(IllegalStateException.class, () -> buf.openCursor(0, 0)); @@ -914,7 +915,7 @@ public abstract class BufferTestSupport { buf.setDouble(0, 3.2); assertThat(buf.getDouble(0)).isEqualTo(3.2); - if (buf.isOwned()) { + if (asRS(buf).isOwned()) { buf.ensureWritable(1); } buf.fill((byte) 0); @@ -999,7 +1000,7 @@ public abstract class BufferTestSupport { } public static int countBorrows(Buffer buf) { - return ((RcSupport) buf).countBorrows(); + return ((ResourceSupport) buf).countBorrows(); } public static void assertEquals(Buffer expected, Buffer actual) { diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/ScopeTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/ScopeTest.java index 3c31881..7ec2e41 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/ScopeTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/ScopeTest.java @@ -17,7 +17,7 @@ package io.netty.buffer.api.tests; import io.netty.buffer.api.Drop; import io.netty.buffer.api.Owned; -import io.netty.buffer.api.RcSupport; +import io.netty.buffer.api.internal.ResourceSupport; import io.netty.buffer.api.Scope; import org.junit.jupiter.api.Test; @@ -29,12 +29,12 @@ import static org.junit.jupiter.api.Assertions.assertTrue; public class ScopeTest { @Test - void scopeMustCloseContainedRcsInReverseInsertOrder() { + void scopeMustCloseContainedRecouresInReverseInsertOrder() { ArrayList closeOrder = new ArrayList<>(); try (Scope scope = new Scope()) { - scope.add(new SomeRc(new OrderingDrop(1, closeOrder))); - scope.add(new SomeRc(new OrderingDrop(2, closeOrder))); - scope.add(new SomeRc(new OrderingDrop(3, closeOrder))); + scope.add(new SomeResource(new OrderingDrop(1, closeOrder))); + scope.add(new SomeResource(new OrderingDrop(2, closeOrder))); + scope.add(new SomeResource(new OrderingDrop(3, closeOrder))); } var itr = closeOrder.iterator(); assertTrue(itr.hasNext()); @@ -46,18 +46,18 @@ public class ScopeTest { assertFalse(itr.hasNext()); } - private static final class SomeRc extends RcSupport { - SomeRc(Drop drop) { + private static final class SomeResource extends ResourceSupport { + SomeResource(Drop drop) { super(drop); } @Override - protected Owned prepareSend() { + protected Owned prepareSend() { return null; } } - private static final class OrderingDrop implements Drop { + private static final class OrderingDrop implements Drop { private final int order; private final ArrayList list; @@ -67,7 +67,7 @@ public class ScopeTest { } @Override - public void drop(SomeRc obj) { + public void drop(SomeResource obj) { list.add(order); } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/benchmarks/ByteIterationBenchmark.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/benchmarks/ByteIterationBenchmark.java index f00a9b1..3fc06e3 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/benchmarks/ByteIterationBenchmark.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/benchmarks/ByteIterationBenchmark.java @@ -63,14 +63,14 @@ public class ByteIterationBenchmark { allocator = BufferAllocator.heap(); try (var a = allocator.allocate(SIZE / 2); var b = allocator.allocate(SIZE / 2)) { - buf = CompositeBuffer.compose(allocator, a, b); + buf = CompositeBuffer.compose(allocator, a.send(), b.send()); } break; case "composite-direct": allocator = BufferAllocator.direct(); try (var a = allocator.allocate(SIZE / 2); var b = allocator.allocate(SIZE / 2)) { - buf = CompositeBuffer.compose(allocator, a, b); + buf = CompositeBuffer.compose(allocator, a.send(), b.send()); } break; default: diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSlicingExample.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSlicingExample.java index 946008c..b6519ab 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSlicingExample.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSlicingExample.java @@ -44,12 +44,10 @@ public final class ComposingAndSlicingExample { } private static Buffer createBigBuffer(BufferAllocator allocator) { - try (Scope scope = new Scope()) { - return CompositeBuffer.compose(allocator, - scope.add(allocator.allocate(64)), - scope.add(allocator.allocate(64)), - scope.add(allocator.allocate(64)), - scope.add(allocator.allocate(64))); - } + return CompositeBuffer.compose(allocator, + allocator.allocate(64).send(), + allocator.allocate(64).send(), + allocator.allocate(64).send(), + allocator.allocate(64).send()); } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/SendExample.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/SendExample.java index 8fcec60..e017461 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/SendExample.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/SendExample.java @@ -77,45 +77,6 @@ public class SendExample { executor.shutdown(); } - private static Future beginTask( - ExecutorService executor, BufferAllocator allocator) { - try (Buffer buf = allocator.allocate(32)) { - // !!! pit-fall: Rc decrement in other thread. - return executor.submit(new Task(buf.acquire())); - } - } - - private static class Task implements Runnable { - private final Buffer buf; - - Task(Buffer buf) { - this.buf = buf; - } - - @Override - public void run() { - try (buf) { - // !!! danger: access out-side owning thread. - while (buf.writableBytes() > 0) { - buf.writeByte((byte) 42); - } - } - } - } - } - - static final class Ex3 { - public static void main(String[] args) throws Exception { - ExecutorService executor = newSingleThreadExecutor(); - BufferAllocator allocator = BufferAllocator.heap(); - - var future = beginTask(executor, allocator); - future.get(); - - allocator.close(); - executor.shutdown(); - } - private static Future beginTask( ExecutorService executor, BufferAllocator allocator) { try (Buffer buf = allocator.allocate(32)) { @@ -141,7 +102,7 @@ public class SendExample { } } - static final class Ex4 { + static final class Ex3 { public static void main(String[] args) throws Exception { ExecutorService executor = newFixedThreadPool(4); BufferAllocator allocator = BufferAllocator.heap(); @@ -180,7 +141,7 @@ public class SendExample { } } - static final class Ex5 { + static final class Ex4 { public static void main(String[] args) throws Exception { ExecutorService executor = newFixedThreadPool(4); BufferAllocator allocator = BufferAllocator.heap(); @@ -220,7 +181,7 @@ public class SendExample { } } - static final class Ex6 { + static final class Ex5 { public static void main(String[] args) throws Exception { ExecutorService executor = newFixedThreadPool(4); BufferAllocator allocator = BufferAllocator.heap(); diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/AlternativeMessageDecoder.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/AlternativeMessageDecoder.java index 249095c..dcf42d6 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/AlternativeMessageDecoder.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/AlternativeMessageDecoder.java @@ -83,25 +83,15 @@ public abstract class AlternativeMessageDecoder extends ChannelHandlerAdapter { } private void processRead(ChannelHandlerContext ctx, Buffer input) { - if (collector.isOwned() && CompositeBuffer.isComposite(collector) && input.isOwned() + if (CompositeBuffer.isComposite(collector) && (collector.writableBytes() == 0 || input.writerOffset() == 0) && (collector.readableBytes() == 0 || input.readerOffset() == 0) && collector.order() == input.order()) { - ((CompositeBuffer) collector).extendWith(input); + ((CompositeBuffer) collector).extendWith(input.send()); drainCollector(ctx); return; } - if (collector.isOwned()) { - collector.ensureWritable(input.readableBytes(), DEFAULT_CHUNK_SIZE, true); - } else { - int requiredCapacity = input.readableBytes() + collector.readableBytes(); - int allocationSize = Math.max(requiredCapacity, DEFAULT_CHUNK_SIZE); - try (Buffer newBuffer = allocator.allocate(allocationSize, input.order())) { - newBuffer.writeBytes(collector); - collector.close(); - collector = newBuffer.acquire(); - } - } + collector.ensureWritable(input.readableBytes(), DEFAULT_CHUNK_SIZE, true); collector.writeBytes(input); drainCollector(ctx); } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoder.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoder.java index 871d39c..1c5de0d 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoder.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoder.java @@ -104,7 +104,7 @@ public abstract class ByteToMessageDecoder extends ChannelHandlerAdapter { // for whatever release (for example because of OutOfMemoryError) try (in) { final int required = in.readableBytes(); - if (required > cumulation.writableBytes() || !cumulation.isOwned() || cumulation.readOnly()) { + if (required > cumulation.writableBytes() || cumulation.readOnly()) { // Expand cumulation (by replacing it) under the following conditions: // - cumulation cannot be resized to accommodate the additional data // - cumulation can be expanded with a reallocation operation to accommodate but the buffer is @@ -129,7 +129,7 @@ public abstract class ByteToMessageDecoder extends ChannelHandlerAdapter { } Buffer composite; try (in) { - if (CompositeBuffer.isComposite(cumulation) && cumulation.isOwned()) { + if (CompositeBuffer.isComposite(cumulation)) { composite = cumulation; if (composite.writerOffset() != composite.capacity()) { // Writer index must equal capacity if we are going to "write" @@ -138,9 +138,9 @@ public abstract class ByteToMessageDecoder extends ChannelHandlerAdapter { cumulation.close(); } } else { - composite = CompositeBuffer.compose(alloc, cumulation); + composite = CompositeBuffer.compose(alloc, cumulation.send()); } - ((CompositeBuffer) composite).extendWith(in); + ((CompositeBuffer) composite).extendWith(in.send()); return composite; } }; @@ -313,7 +313,7 @@ public abstract class ByteToMessageDecoder extends ChannelHandlerAdapter { } protected final void discardSomeReadBytes() { - if (cumulation != null && !first && cumulation.isOwned()) { + if (cumulation != null && !first) { // discard some bytes if possible to make more room in the // buffer but only if the refCnt == 1 as otherwise the user may have // used slice().retain() or duplicate().retain(). diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java index cc72823..838bc7c 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java @@ -30,6 +30,7 @@ import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; +import static io.netty.buffer.api.internal.Statics.asRS; import static io.netty.buffer.api.tests.BufferTestSupport.assertEquals; import static io.netty.buffer.api.CompositeBuffer.compose; import static java.nio.ByteOrder.BIG_ENDIAN; @@ -131,7 +132,7 @@ public class ByteToMessageDecoderTest { @Override protected void decode(ChannelHandlerContext ctx, Buffer in) { Buffer buf = internalBuffer(); - assertTrue(buf.isOwned()); + buf.ensureWritable(8, 8, false); // Verify we have full access to the buffer. in.readByte(); // Removal from pipeline should clear internal buffer ctx.pipeline().remove(this); @@ -352,7 +353,7 @@ public class ByteToMessageDecoderTest { assertThat(thrown).hasMessage("boom"); assertFalse(in.isAccessible()); - assertTrue(oldCumulation.isOwned()); + oldCumulation.ensureWritable(8, 8, false); // Will throw if we don't have full access to the buffer. oldCumulation.close(); } @@ -407,13 +408,13 @@ public class ByteToMessageDecoderTest { if (shouldFail) { assertThat(thrown).hasMessage("boom"); - assertTrue(oldCumulation.isOwned()); + oldCumulation.ensureWritable(8, 8, false); // Will throw if we don't have full access to the buffer. oldCumulation.close(); assertFalse(newCumulation.isAccessible()); } else { assertNull(thrown); assertFalse(oldCumulation.isAccessible()); - assertTrue(newCumulation.isOwned()); + newCumulation.ensureWritable(8, 8, false); // Will throw if we don't have full access to the buffer. newCumulation.close(); } } From b8cfd0768e6a8047ce07d59bfc51285ce358f125 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Wed, 26 May 2021 18:31:28 +0200 Subject: [PATCH 02/12] Fixes for compilation and running tests with UnsafeBuffer implementation --- buffer-memseg/pom.xml | 4 -- buffer-tests/pom.xml | 97 +++++++++++++++++++++++++++++++++++++++++++ pom.xml | 89 --------------------------------------- 3 files changed, 97 insertions(+), 93 deletions(-) diff --git a/buffer-memseg/pom.xml b/buffer-memseg/pom.xml index 215d02c..16d8116 100644 --- a/buffer-memseg/pom.xml +++ b/buffer-memseg/pom.xml @@ -61,10 +61,6 @@ -Xlint:-options 256m 1024m - - --add-modules - jdk.incubator.foreign - diff --git a/buffer-tests/pom.xml b/buffer-tests/pom.xml index 2ed3b2d..78c2cce 100644 --- a/buffer-tests/pom.xml +++ b/buffer-tests/pom.xml @@ -30,6 +30,103 @@ Netty/Incubator/Buffer Tests jar + + 3.0.0-M5 + false + + -XX:+HeapDumpOnOutOfMemoryError + -Xmx2g + -Dio.netty.tryReflectionSetAccessible=true + --add-opens java.base/java.nio=io.netty.common + + + + + + + + maven-surefire-plugin + ${surefire.version} + + + **/*Test*.java + + ${argLine.common} ${argLine.mod} + + false + 600 + + false + 1C + + nosample + + + + + + org.codehaus.plexus + plexus-utils + 1.1 + + + org.apache.maven.surefire + surefire-junit-platform + ${surefire.version} + + + org.junit.jupiter + junit-jupiter-engine + 5.3.2 + pom + + + org.junit.jupiter + junit-jupiter-params + 5.3.2 + pom + + + org.mockito + mockito-core + 2.28.2 + pom + + + org.hamcrest + hamcrest-library + 1.3 + pom + + + junit + junit + 4.13 + pom + + + org.powermock + powermock-reflect + 2.0.5 + pom + + + org.assertj + assertj-core + 3.9.1 + pom + + + org.easytesting + fest-assert + 1.4 + pom + + + + + + io.netty.incubator diff --git a/pom.xml b/pom.xml index 4ea1568..2e98b93 100644 --- a/pom.xml +++ b/pom.xml @@ -75,15 +75,6 @@ 11 5.7.0 - 3.0.0-M5 - false - - -XX:+HeapDumpOnOutOfMemoryError - -Xmx2g - -Dio.netty.tryReflectionSetAccessible=true - --add-opens java.base/java.nio=io.netty.common - - @@ -192,86 +183,6 @@ - - maven-surefire-plugin - ${surefire.version} - - - **/*Test*.java - - ${argLine.common} ${argLine.mod} - - false - 600 - - false - 1C - - nosample - - - - - - org.codehaus.plexus - plexus-utils - 1.1 - - - org.apache.maven.surefire - surefire-junit-platform - ${surefire.version} - - - org.junit.jupiter - junit-jupiter-engine - 5.3.2 - pom - - - org.junit.jupiter - junit-jupiter-params - 5.3.2 - pom - - - org.mockito - mockito-core - 2.28.2 - pom - - - org.hamcrest - hamcrest-library - 1.3 - pom - - - junit - junit - 4.13 - pom - - - org.powermock - powermock-reflect - 2.0.5 - pom - - - org.assertj - assertj-core - 3.9.1 - pom - - - org.easytesting - fest-assert - 1.4 - pom - - - org.apache.felix From bfa8fd0b1f467e2629cf2f99811bfaad15030e2d Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Thu, 27 May 2021 11:39:57 +0200 Subject: [PATCH 03/12] Make tests pass after removing `acquire` from the public API --- buffer-memseg/pom.xml | 51 ++++--- buffer-tests/pom.xml | 1 - .../api/tests/BufferCompositionTest.java | 144 +++++------------- .../api/tests/BufferEnsureWritableTest.java | 17 --- .../tests/BufferReferenceCountingTest.java | 53 ------- pom.xml | 1 + 6 files changed, 67 insertions(+), 200 deletions(-) diff --git a/buffer-memseg/pom.xml b/buffer-memseg/pom.xml index 16d8116..3bd4f08 100644 --- a/buffer-memseg/pom.xml +++ b/buffer-memseg/pom.xml @@ -15,6 +15,34 @@ Netty/Incubator/Buffer MemorySegment jar + + + + maven-compiler-plugin + 3.8.1 + + ${java.version} + true + ${java.compatibility} + ${java.compatibility} + ${java.version} + true + true + true + true + -Xlint:-options + 256m + 1024m + + + + org.apache.maven.plugins + maven-surefire-plugin + ${surefire.version} + + + + io.netty.incubator @@ -42,27 +70,4 @@ test - - - - - maven-compiler-plugin - 3.8.1 - - ${java.version} - true - ${java.compatibility} - ${java.compatibility} - ${java.version} - true - true - true - true - -Xlint:-options - 256m - 1024m - - - - \ No newline at end of file diff --git a/buffer-tests/pom.xml b/buffer-tests/pom.xml index 78c2cce..66c5707 100644 --- a/buffer-tests/pom.xml +++ b/buffer-tests/pom.xml @@ -31,7 +31,6 @@ jar - 3.0.0-M5 false -XX:+HeapDumpOnOutOfMemoryError diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java index c57e191..a6d3715 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java @@ -29,47 +29,21 @@ import static io.netty.buffer.api.internal.Statics.asRS; import static java.nio.ByteOrder.BIG_ENDIAN; import static java.nio.ByteOrder.LITTLE_ENDIAN; import static org.assertj.core.api.Assertions.assertThat; -import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; public class BufferCompositionTest extends BufferTestSupport { - @Test - public void compositeBufferCanOnlyBeOwnedWhenAllConstituentBuffersAreOwned() { - try (BufferAllocator allocator = BufferAllocator.heap()) { - var composite = asRS(CompositeBuffer.compose(allocator)); - try (var a = asRS(allocator.allocate(8))) { - assertTrue(a.isOwned()); - Buffer leakB; - try (var b = asRS(allocator.allocate(8))) { - assertTrue(a.isOwned()); - assertTrue(b.isOwned()); - composite = asRS(CompositeBuffer.compose(allocator, a.send(), b.send())); - assertFalse(composite.isOwned()); - assertFalse(a.isOwned()); - assertFalse(b.isOwned()); - leakB = b; - } - assertFalse(composite.isOwned()); - assertFalse(a.isOwned()); - assertTrue(asRS(leakB).isOwned()); - } - assertTrue(composite.isOwned()); - } - } - @Test public void compositeBuffersCannotHaveDuplicateComponents() { try (BufferAllocator allocator = BufferAllocator.heap()) { Send a = allocator.allocate(4).send(); - var e = assertThrows(IllegalArgumentException.class, () -> CompositeBuffer.compose(allocator, a, a)); - assertThat(e).hasMessageContaining("duplicate"); + var e = assertThrows(IllegalStateException.class, () -> CompositeBuffer.compose(allocator, a, a)); + assertThat(e).hasMessageContaining("already been received"); Send b = allocator.allocate(4).send(); try (CompositeBuffer composite = CompositeBuffer.compose(allocator, b)) { - e = assertThrows(IllegalArgumentException.class, - () -> composite.extendWith(b)); - assertThat(e).hasMessageContaining("duplicate"); + e = assertThrows(IllegalStateException.class, () -> composite.extendWith(b)); + assertThat(e).hasMessageContaining("already been received"); } } } @@ -89,28 +63,20 @@ public class BufferCompositionTest extends BufferTestSupport { @Test public void compositeBufferMustNotBeAllowedToContainThemselves() { try (BufferAllocator allocator = BufferAllocator.heap()) { - Buffer a = allocator.allocate(4); - CompositeBuffer buf = CompositeBuffer.compose(allocator, a.send()); - try (buf; a) { - a.close(); - try { - assertThrows(IllegalArgumentException.class, () -> buf.extendWith(buf.send())); - assertTrue(buf.isOwned()); - try (Buffer composite = CompositeBuffer.compose(allocator, buf.send())) { - // the composing increments the reference count of constituent buffers... - // counter-act this, so it can be extended: - a.close(); // buf is now owned, so it can be extended. - try { - assertThrows(IllegalArgumentException.class, - () -> buf.extendWith(composite.send())); - } finally { - asRS(a).acquire(); // restore the reference count to align with our try-with-resources structure. - } - } - assertTrue(buf.isOwned()); - } finally { - asRS(a).acquire(); - } + CompositeBuffer bufA = CompositeBuffer.compose(allocator, allocator.allocate(4).send()); + Send sendA = bufA.send(); + try { + assertThrows(IllegalStateException.class, () -> bufA.extendWith(sendA)); + } finally { + sendA.discard(); + } + + CompositeBuffer bufB = CompositeBuffer.compose(allocator, allocator.allocate(4).send()); + Send sendB = bufB.send(); + try (CompositeBuffer compositeBuffer = CompositeBuffer.compose(allocator, sendB)) { + assertThrows(IllegalStateException.class, () -> compositeBuffer.extendWith(sendB)); + } finally { + sendB.discard(); } } } @@ -182,7 +148,7 @@ public class BufferCompositionTest extends BufferTestSupport { composite = CompositeBuffer.compose(allocator, a.send()); } try (composite) { - var exc = assertThrows(IllegalArgumentException.class, + var exc = assertThrows(IllegalStateException.class, () -> composite.extendWith(composite.send())); assertThat(exc).hasMessageContaining("cannot be extended"); } @@ -385,27 +351,25 @@ public class BufferCompositionTest extends BufferTestSupport { @Test public void whenExtendingCompositeBufferWithReadOffsetLessThanCapacityExtensionReadOffsetMustZero() { - try (BufferAllocator allocator = BufferAllocator.heap()) { - CompositeBuffer composite; - try (Buffer a = allocator.allocate(8)) { - composite = CompositeBuffer.compose(allocator, a.send()); - } - try (composite) { - composite.writeLong(0); - composite.readInt(); - try (Buffer b = allocator.allocate(8)) { - b.writeInt(1); - b.readInt(); - var exc = assertThrows(IllegalArgumentException.class, - () -> composite.extendWith(b.send())); - assertThat(exc).hasMessageContaining("unread gap"); - b.readerOffset(0); - composite.extendWith(b.send()); - assertThat(composite.capacity()).isEqualTo(16); - assertThat(composite.writerOffset()).isEqualTo(12); - assertThat(composite.readerOffset()).isEqualTo(4); - } - } + try (BufferAllocator allocator = BufferAllocator.heap(); + CompositeBuffer composite = CompositeBuffer.compose(allocator, allocator.allocate(8).send())) { + composite.writeLong(0); + composite.readInt(); + + Buffer b = allocator.allocate(8); + b.writeInt(1); + b.readInt(); + var exc = assertThrows(IllegalArgumentException.class, + () -> composite.extendWith(b.send())); + assertThat(exc).hasMessageContaining("unread gap"); + assertThat(composite.capacity()).isEqualTo(8); + assertThat(composite.writerOffset()).isEqualTo(8); + assertThat(composite.readerOffset()).isEqualTo(4); + + composite.extendWith(allocator.allocate(8).writeInt(1).send()); + assertThat(composite.capacity()).isEqualTo(16); + assertThat(composite.writerOffset()).isEqualTo(12); + assertThat(composite.readerOffset()).isEqualTo(4); } } @@ -481,38 +445,6 @@ public class BufferCompositionTest extends BufferTestSupport { } } - @Test - public void splitComponentsFloorMustThrowIfCompositeBufferIsNotOwned() { - try (BufferAllocator allocator = BufferAllocator.heap(); - Buffer a = allocator.allocate(8); - Buffer b = allocator.allocate(8); - CompositeBuffer composite = CompositeBuffer.compose(allocator, a.send(), b.send())) { - assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(0)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(4)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(7)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(8)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(9)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(12)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsFloor(16)); - } - } - - @Test - public void splitComponentsCeilMustThrowIfCompositeBufferIsNotOwned() { - try (BufferAllocator allocator = BufferAllocator.heap(); - Buffer a = allocator.allocate(8); - Buffer b = allocator.allocate(8); - CompositeBuffer composite = CompositeBuffer.compose(allocator, a.send(), b.send())) { - assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(0)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(4)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(7)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(8)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(9)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(12)); - assertThrows(IllegalStateException.class, () -> composite.splitComponentsCeil(16)); - } - } - @Test public void splitComponentsFloorMustThrowOnOutOfBounds() { try (BufferAllocator allocator = BufferAllocator.heap(); diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java index 8be646c..d301daf 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java @@ -26,23 +26,6 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertThrows; public class BufferEnsureWritableTest extends BufferTestSupport { - - @ParameterizedTest - @MethodSource("allocators") - public void ensureWritableMustThrowForBorrowedBuffers(Fixture fixture) { - try (BufferAllocator allocator = fixture.createAllocator(); - Buffer buf = allocator.allocate(8)) { - try (Buffer slice = buf.slice()) { - assertThrows(IllegalStateException.class, () -> slice.ensureWritable(1)); - assertThrows(IllegalStateException.class, () -> buf.ensureWritable(1)); - } - try (Buffer compose = CompositeBuffer.compose(allocator, buf.send())) { - assertThrows(IllegalStateException.class, () -> compose.ensureWritable(1)); - assertThrows(IllegalStateException.class, () -> buf.ensureWritable(1)); - } - } - } - @ParameterizedTest @MethodSource("allocators") public void ensureWritableMustThrowForNegativeSize(Fixture fixture) { diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java index 8d89a82..16fff44 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java @@ -296,59 +296,6 @@ public class BufferReferenceCountingTest extends BufferTestSupport { } } - @ParameterizedTest - @MethodSource("nonCompositeAllocators") - public void acquireComposingAndSlicingMustIncrementBorrows(Fixture fixture) { - try (BufferAllocator allocator = fixture.createAllocator(); - Buffer buf = allocator.allocate(8)) { - int borrows = countBorrows(buf); - try (Buffer ignored = asRS(buf).acquire()) { - assertEquals(borrows + 1, countBorrows(buf)); - try (Buffer slice = buf.slice()) { - assertEquals(0, slice.capacity()); // We haven't written anything, so the slice is empty. - int sliceBorrows = countBorrows(slice); - assertEquals(borrows + 2, countBorrows(buf)); - try (Buffer ignored1 = CompositeBuffer.compose(allocator, buf.send(), slice.send())) { - assertEquals(borrows + 3, countBorrows(buf)); - // Note: Slice is empty; not acquired by the composite buffer. - assertEquals(sliceBorrows, countBorrows(slice)); - } - assertEquals(sliceBorrows, countBorrows(slice)); - assertEquals(borrows + 2, countBorrows(buf)); - } - assertEquals(borrows + 1, countBorrows(buf)); - } - assertEquals(borrows, countBorrows(buf)); - } - } - - @ParameterizedTest - @MethodSource("nonCompositeAllocators") - public void acquireComposingAndSlicingMustIncrementBorrowsWithData(Fixture fixture) { - try (BufferAllocator allocator = fixture.createAllocator(); - Buffer buf = allocator.allocate(8)) { - buf.writeByte((byte) 1); - int borrows = countBorrows(buf); - try (Buffer ignored = asRS(buf).acquire()) { - assertEquals(borrows + 1, countBorrows(buf)); - try (Buffer slice = buf.slice()) { - assertEquals(1, slice.capacity()); - int sliceBorrows = countBorrows(slice); - assertEquals(borrows + 2, countBorrows(buf)); - try (Buffer ignored1 = CompositeBuffer.compose(allocator, buf.send(), slice.send())) { - assertEquals(borrows + 3, countBorrows(buf)); - assertEquals(sliceBorrows + 1, countBorrows(slice)); - } - assertEquals(sliceBorrows, countBorrows(slice)); - assertEquals(borrows + 2, countBorrows(buf)); - } - assertEquals(borrows + 1, countBorrows(buf)); - } - assertEquals(borrows, countBorrows(buf)); - assertTrue(asRS(buf).isOwned()); - } - } - @ParameterizedTest @MethodSource("allocators") public void sliceMustBecomeOwnedOnSourceBufferClose(Fixture fixture) { diff --git a/pom.xml b/pom.xml index 2e98b93..2dbf0e2 100644 --- a/pom.xml +++ b/pom.xml @@ -75,6 +75,7 @@ 11 5.7.0 + 3.0.0-M5 From 707e5e2afb60872efd5ffb48ed95992859b5a33e Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Thu, 27 May 2021 14:07:31 +0200 Subject: [PATCH 04/12] Remove the slice methods, add copy methods --- .../main/java/io/netty/buffer/api/Buffer.java | 77 ++++------ .../io/netty/buffer/api/CompositeBuffer.java | 18 +-- .../buffer/api/adaptor/ByteBufAdaptor.java | 7 +- .../buffer/api/bytebuffer/NioBuffer.java | 17 ++- .../netty/buffer/api/unsafe/UnsafeBuffer.java | 15 +- .../netty/buffer/api/memseg/MemSegBuffer.java | 16 +-- .../api/tests/BufferBulkAccessTest.java | 39 ++---- .../api/tests/BufferEnsureWritableTest.java | 6 +- .../buffer/api/tests/BufferReadOnlyTest.java | 4 +- .../tests/BufferReferenceCountingTest.java | 114 +++++++-------- .../buffer/api/tests/BufferTestSupport.java | 131 ++---------------- .../BufferWriteBytesCombinationsTest.java | 4 +- .../io/netty/buffer/api/tests/Fixture.java | 7 +- ...java => ComposingAndSplittingExample.java} | 4 +- .../api/tests/examples/SendExample.java | 79 ----------- .../AlternativeMessageDecoderTest.java | 5 +- .../ByteToMessageDecoder.java | 2 +- .../ByteToMessageDecoderTest.java | 69 ++++----- .../FixedLengthFrameDecoder.java | 6 +- 19 files changed, 178 insertions(+), 442 deletions(-) rename buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/{ComposingAndSlicingExample.java => ComposingAndSplittingExample.java} (95%) diff --git a/buffer-api/src/main/java/io/netty/buffer/api/Buffer.java b/buffer-api/src/main/java/io/netty/buffer/api/Buffer.java index 4b1b6bf..ca3f3ab 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/Buffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/Buffer.java @@ -74,35 +74,20 @@ import java.nio.ByteOrder; * 0 <= readerOffset <= writerOffset <= capacity * * - *

Slice vs. Split

+ *

Splitting buffers

* - * The {@link #slice()} and {@link #split()} methods both return new buffers on the memory of the buffer they're - * called on. - * However, there are also important differences between the two, as they are aimed at different use cases that were - * previously (in the {@code ByteBuf} API) covered by {@code slice()} alone. - * - *
    - *
  • - * Slices create a new view onto the memory, that is shared between the slice and the buffer. - * As long as both the slice, and the originating buffer are alive, neither will have ownership of the memory. - * Since the memory is shared, changes to the data made through one will be visible through the other. - *
    • - *
  • - * Split breaks the ownership of the memory in two. - * Both resulting buffers retain ownership of their respective region of memory. - * They can do this because the regions are guaranteed to not overlap; data changes through one buffer will not - * be visible through the other. - *
    • - *
- * - * These differences mean that slicing is mostly suitable for when you temporarily want to share a focused area of a - * buffer. - * Examples of this include doing IO, or decoding a bounded part of a larger message. - * On the other hand, split is suitable for when you want to hand over a region of a buffer to some other, + * The {@link #split()} method break a buffer into two. + * The two buffers will share the underlying memory, but their regions will not overlap, ensuring that the memory is + * safely shared between the two. + *

+ * Splitting a buffer is useful for when you want to hand over a region of a buffer to some other, * perhaps unknown, piece of code, and relinquish your ownership of that buffer region in the process. * Examples include aggregating messages into an accumulator buffer, and sending messages down the pipeline for * further processing, as split buffer regions, once their data has been received in its entirety. * + * If you instead wish to temporarily share a region of a buffer, you will have to pass offset and length along with the + * buffer, or you will have to make a copy of the region in a new buffer. + * *

Buffers as constants

* * Sometimes, the same bit of data will be processed or transmitted over and over again. In such cases, it can be @@ -439,47 +424,35 @@ public interface Buffer extends Resource, BufferAccessors { void ensureWritable(int size, int minimumGrowth, boolean allowCompaction); /** - * Returns a slice of this buffer's readable bytes. - * Modifying the content of the returned buffer or this buffer affects each other's content, - * while they maintain separate offsets. This method is identical to - * {@code buf.slice(buf.readerOffset(), buf.readableBytes())}. + * Returns a copy of this buffer's readable bytes. + * Modifying the content of the returned buffer will not affect this buffers contents. + * The two buffers will maintain separate offsets. This method is identical to + * {@code buf.copy(buf.readerOffset(), buf.readableBytes())}. * This method does not modify {@link #readerOffset()} or {@link #writerOffset()} of this buffer. *

- * This method increments the reference count of this buffer. - * The reference count is decremented again when the slice is deallocated. - *

- * The slice is created with a {@linkplain #writerOffset() write offset} equal to the length of the slice, - * so that the entire contents of the slice is ready to be read. - *

- * See the Slice vs. Split section for details on the difference between slice - * and split. + * The copy is created with a {@linkplain #writerOffset() write offset} equal to the length of the copied data, + * so that the entire contents of the copy is ready to be read. * * @return A new buffer instance, with independent {@link #readerOffset()} and {@link #writerOffset()}, - * that is a view of the readable region of this buffer. + * that contains a copy of the readable region of this buffer. */ - default Buffer slice() { - return slice(readerOffset(), readableBytes()); + default Buffer copy() { + return copy(readerOffset(), readableBytes()); } /** - * Returns a slice of the given region of this buffer. - * Modifying the content of the returned buffer or this buffer affects each other's content, - * while they maintain separate offsets. + * Returns a copy of the given region of this buffer. + * Modifying the content of the returned buffer will not affect this buffers contents. + * The two buffers will maintain separate offsets. * This method does not modify {@link #readerOffset()} or {@link #writerOffset()} of this buffer. *

- * This method increments the reference count of this buffer. - * The reference count is decremented again when the slice is deallocated. - *

- * The slice is created with a {@linkplain #writerOffset() write offset} equal to the length of the slice, - * so that the entire contents of the slice is ready to be read. - *

- * See the Slice vs. Split section for details on the difference between slice - * and split. + * The copy is created with a {@linkplain #writerOffset() write offset} equal to the length of the copy, + * so that the entire contents of the copy is ready to be read. * * @return A new buffer instance, with independent {@link #readerOffset()} and {@link #writerOffset()}, - * that is a view of the given region of this buffer. + * that contains a copy of the given region of this buffer. */ - Buffer slice(int offset, int length); + Buffer copy(int offset, int length); /** * Split the buffer into two, at the {@linkplain #writerOffset() write offset} position. diff --git a/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java b/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java index 7ff1fbb..06158dc 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java @@ -414,7 +414,7 @@ public final class CompositeBuffer extends ResourceSupport 0) { - slices = new Buffer[bufs.length]; + copies = new Buffer[bufs.length]; int off = subOffset; int cap = length; int i; for (i = searchOffsets(offset); cap > 0; i++) { var buf = bufs[i]; int avail = buf.capacity() - off; - slices[i] = buf.slice(off, Math.min(cap, avail)); + copies[i] = buf.copy(off, Math.min(cap, avail)); cap -= avail; off = 0; } - slices = Arrays.copyOf(slices, i); + copies = Arrays.copyOf(copies, i); } else { // Specialize for length == 0, since we must slice from at least one constituent buffer. - slices = new Buffer[] { choice.slice(subOffset, 0) }; + copies = new Buffer[] { choice.copy(subOffset, 0) }; } - // Use the constructor that skips filtering out empty buffers, and skips acquiring on the buffers. - // This is important because 1) slice() already acquired the buffers, and 2) if this slice is empty - // then we need to keep holding on to it to prevent this originating composite buffer from getting - // ownership. If it did, its behaviour would be inconsistent with that of a non-composite buffer. - return new CompositeBuffer(allocator, slices, COMPOSITE_DROP); + return new CompositeBuffer(allocator, copies, COMPOSITE_DROP); } @Override diff --git a/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java b/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java index c58148a..8c9631e 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java @@ -1418,11 +1418,8 @@ public final class ByteBufAdaptor extends ByteBuf { @Override public ByteBuf retainedSlice(int index, int length) { checkAccess(); - try { - return wrap(buffer.slice(index, length)); - } catch (IllegalStateException e) { - throw new IllegalReferenceCountException(e); - } + retain(); + return new Slice(this, index, length); } private static final class Slice extends SlicedByteBuf { diff --git a/buffer-api/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java b/buffer-api/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java index 809da74..6d5e6f0 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java @@ -188,23 +188,22 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } @Override - public Buffer slice(int offset, int length) { + public Buffer copy(int offset, int length) { if (length < 0) { throw new IllegalArgumentException("Length cannot be negative: " + length + '.'); } if (!isAccessible()) { throw new IllegalStateException("This buffer is closed: " + this + '.'); } - ByteBuffer slice = bbslice(rmem, offset, length); - ArcDrop drop = (ArcDrop) unsafeGetDrop(); - drop.increment(); - Buffer sliceBuffer = new NioBuffer(base, slice, control, drop) - .writerOffset(length) - .order(order()); + AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, length); + ByteBuffer byteBuffer = memory.memory(); + Buffer copy = new NioBuffer(byteBuffer, byteBuffer, control, memory.drop()); + copyInto(0, copy, 0, length); + copy.writerOffset(length).order(order()); if (readOnly()) { - sliceBuffer = sliceBuffer.makeReadOnly(); + copy = copy.makeReadOnly(); } - return sliceBuffer; + return copy; } @Override diff --git a/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java b/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java index 1c4d21c..eaddde7 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java @@ -165,20 +165,19 @@ class UnsafeBuffer extends ResourceSupport implements Buff } @Override - public Buffer slice(int offset, int length) { + public Buffer copy(int offset, int length) { if (length < 0) { throw new IllegalArgumentException("Length cannot be negative: " + length + '.'); } checkGet(offset, length); - ArcDrop drop = (ArcDrop) unsafeGetDrop(); - drop.increment(); - Buffer sliceBuffer = new UnsafeBuffer(memory, baseOffset + offset, length, control, drop) - .writerOffset(length) - .order(order); + AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, length); + UnsafeMemory unsafeMemory = memory.memory(); + Buffer copy = new UnsafeBuffer(unsafeMemory, unsafeMemory.address, length, control, memory.drop()); + copy.writerOffset(length).order(order); if (readOnly) { - sliceBuffer = sliceBuffer.makeReadOnly(); + copy = copy.makeReadOnly(); } - return sliceBuffer; + return copy; } @Override diff --git a/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java b/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java index b35173e..aeca528 100644 --- a/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java +++ b/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java @@ -293,22 +293,22 @@ class MemSegBuffer extends ResourceSupport implements Buff } @Override - public Buffer slice(int offset, int length) { + public Buffer copy(int offset, int length) { if (length < 0) { throw new IllegalArgumentException("Length cannot be negative: " + length + '.'); } if (!isAccessible()) { throw new IllegalStateException("This buffer is closed: " + this + '.'); } - var slice = seg.asSlice(offset, length); - Drop drop = ArcDrop.acquire(unsafeGetDrop()); - Buffer sliceBuffer = new MemSegBuffer(base, slice, drop, control) - .writerOffset(length) - .order(order()); + AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, length); + MemorySegment segment = memory.memory(); + Buffer copy = new MemSegBuffer(segment, segment, memory.drop(), control); + copyInto(0, copy, 0, length); + copy.writerOffset(length).order(order()); if (readOnly()) { - sliceBuffer = sliceBuffer.makeReadOnly(); + copy = copy.makeReadOnly(); } - return sliceBuffer; + return copy; } @Override diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferBulkAccessTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferBulkAccessTest.java index 1715e95..3458687 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferBulkAccessTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferBulkAccessTest.java @@ -29,7 +29,6 @@ import static java.nio.ByteOrder.LITTLE_ENDIAN; import static org.assertj.core.api.Assertions.assertThat; public class BufferBulkAccessTest extends BufferTestSupport { - @ParameterizedTest @MethodSource("allocators") void fill(Fixture fixture) { @@ -86,28 +85,6 @@ public class BufferBulkAccessTest extends BufferTestSupport { testCopyIntoBuf(fixture, BufferAllocator.direct()::allocate); } - @ParameterizedTest - @MethodSource("allocators") - void copyIntoOnHeapBufSlice(Fixture fixture) { - try (BufferAllocator allocator = BufferAllocator.heap(); - Scope scope = new Scope()) { - testCopyIntoBuf(fixture, size -> { - return scope.add(allocator.allocate(size)).writerOffset(size).slice(); - }); - } - } - - @ParameterizedTest - @MethodSource("allocators") - void copyIntoOffHeapBufSlice(Fixture fixture) { - try (BufferAllocator allocator = BufferAllocator.direct(); - Scope scope = new Scope()) { - testCopyIntoBuf(fixture, size -> { - return scope.add(allocator.allocate(size)).writerOffset(size).slice(); - }); - } - } - @ParameterizedTest @MethodSource("allocators") void copyIntoCompositeOnHeapOnHeapBuf(Fixture fixture) { @@ -174,7 +151,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - void copyIntoCompositeOnHeapOnHeapBufSlice(Fixture fixture) { + void copyIntoCompositeOnHeapOnHeapBufCopy(Fixture fixture) { try (var a = BufferAllocator.heap(); var b = BufferAllocator.heap(); var scope = new Scope()) { @@ -183,7 +160,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { int second = size - first; try (var bufFirst = a.allocate(first); var bufSecond = b.allocate(second)) { - return scope.add(CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send())).writerOffset(size).slice(); + return scope.add(CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send())).writerOffset(size).copy(); } }); } @@ -191,7 +168,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - void copyIntoCompositeOnHeapOffHeapBufSlice(Fixture fixture) { + void copyIntoCompositeOnHeapOffHeapBufCopy(Fixture fixture) { try (var a = BufferAllocator.heap(); var b = BufferAllocator.direct(); var scope = new Scope()) { @@ -200,7 +177,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { int second = size - first; try (var bufFirst = a.allocate(first); var bufSecond = b.allocate(second)) { - return scope.add(CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send())).writerOffset(size).slice(); + return scope.add(CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send())).writerOffset(size).copy(); } }); } @@ -208,7 +185,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - void copyIntoCompositeOffHeapOnHeapBufSlice(Fixture fixture) { + void copyIntoCompositeOffHeapOnHeapBufCopy(Fixture fixture) { try (var a = BufferAllocator.direct(); var b = BufferAllocator.heap(); var scope = new Scope()) { @@ -217,7 +194,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { int second = size - first; try (var bufFirst = a.allocate(first); var bufSecond = b.allocate(second)) { - return scope.add(CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send())).writerOffset(size).slice(); + return scope.add(CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send())).writerOffset(size).copy(); } }); } @@ -225,7 +202,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - void copyIntoCompositeOffHeapOffHeapBufSlice(Fixture fixture) { + void copyIntoCompositeOffHeapOffHeapBufCopy(Fixture fixture) { try (var a = BufferAllocator.direct(); var b = BufferAllocator.direct(); var scope = new Scope()) { @@ -234,7 +211,7 @@ public class BufferBulkAccessTest extends BufferTestSupport { int second = size - first; try (var bufFirst = a.allocate(first); var bufSecond = b.allocate(second)) { - return scope.add(CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send())).writerOffset(size).slice(); + return scope.add(CompositeBuffer.compose(a, bufFirst.send(), bufSecond.send())).writerOffset(size).copy(); } }); } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java index d301daf..f5da1d1 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java @@ -92,15 +92,15 @@ public class BufferEnsureWritableTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - public void mustBeAbleToSliceAfterEnsureWritable(Fixture fixture) { + public void mustBeAbleToCopyAfterEnsureWritable(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(4)) { buf.ensureWritable(8); assertThat(buf.writableBytes()).isGreaterThanOrEqualTo(8); assertThat(buf.capacity()).isGreaterThanOrEqualTo(8); buf.writeLong(0x0102030405060708L); - try (Buffer slice = buf.slice()) { - assertEquals(0x0102030405060708L, slice.readLong()); + try (Buffer copy = buf.copy()) { + assertEquals(0x0102030405060708L, copy.readLong()); } } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java index 7e3d632..0b4ea8c 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java @@ -195,7 +195,7 @@ public class BufferReadOnlyTest extends BufferTestSupport { assertTrue(asRS(b).isOwned()); assertThat(a.capacity()).isEqualTo(8); assertThat(b.capacity()).isEqualTo(8); - try (Buffer c = b.slice()) { + try (Buffer c = b.copy()) { assertTrue(c.readOnly()); assertFalse(asRS(c).isOwned()); assertFalse(asRS(b).isOwned()); @@ -211,7 +211,7 @@ public class BufferReadOnlyTest extends BufferTestSupport { Supplier supplier = allocator.constBufferSupplier(new byte[] {1, 2, 3, 4}); try (Buffer a = supplier.get(); Buffer b = supplier.get(); - Buffer c = a.slice()) { + Buffer c = a.copy()) { assertEquals(1, a.readByte()); assertEquals(2, a.readByte()); assertThrows(IllegalStateException.class, () -> a.compact()); // Can't compact read-only buffer. diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java index 16fff44..be26311 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java @@ -108,7 +108,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - void sliceWithoutOffsetAndSizeMustReturnReadableRegion(Fixture fixture) { + void copyWithoutOffsetAndSizeMustReturnReadableRegion(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { for (byte b : new byte[] { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08 }) { @@ -116,7 +116,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { } assertEquals(0x01, buf.readByte()); buf.writerOffset(buf.writerOffset() - 1); - try (Buffer slice = buf.slice()) { + try (Buffer slice = buf.copy()) { assertThat(toByteArray(slice)).containsExactly(0x02, 0x03, 0x04, 0x05, 0x06, 0x07); assertEquals(0, slice.readerOffset()); assertEquals(6, slice.readableBytes()); @@ -135,7 +135,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - void sliceWithOffsetAndSizeMustReturnGivenRegion(Fixture fixture) { + void copyWithOffsetAndSizeMustReturnGivenRegion(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { for (byte b : new byte[] { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08 }) { @@ -143,7 +143,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { } buf.readerOffset(3); // Reader and writer offsets must be ignored. buf.writerOffset(6); - try (Buffer slice = buf.slice(1, 6)) { + try (Buffer slice = buf.copy(1, 6)) { assertThat(toByteArray(slice)).containsExactly(0x02, 0x03, 0x04, 0x05, 0x06, 0x07); assertEquals(0, slice.readerOffset()); assertEquals(6, slice.readableBytes()); @@ -162,12 +162,14 @@ public class BufferReferenceCountingTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - void sliceWithoutOffsetAndSizeWillIncreaseReferenceCount(Fixture fixture) { + void copyWithoutOffsetAndSizeMustNotInfluenceOwnership(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { - try (Buffer ignored = buf.slice()) { - assertFalse(asRS(buf).isOwned()); - assertThrows(IllegalStateException.class, buf::send); + try (Buffer copy = buf.copy()) { + assertTrue(asRS(buf).isOwned()); + buf.send().discard(); + assertTrue(asRS(copy).isOwned()); + copy.send().discard(); } assertTrue(asRS(buf).isOwned()); } @@ -175,12 +177,14 @@ public class BufferReferenceCountingTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - void sliceWithOffsetAndSizeWillIncreaseReferenceCount(Fixture fixture) { + void copyWithOffsetAndSizeMustNotInfluenceOwnership(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { - try (Buffer ignored = buf.slice(0, 8)) { - assertFalse(asRS(buf).isOwned()); - assertThrows(IllegalStateException.class, buf::send); + try (Buffer copy = buf.copy(0, 8)) { + assertTrue(asRS(buf).isOwned()); + buf.send().discard(); + assertTrue(asRS(copy).isOwned()); + copy.send().discard(); } assertTrue(asRS(buf).isOwned()); } @@ -188,46 +192,46 @@ public class BufferReferenceCountingTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - void sliceWithoutOffsetAndSizeHasSameEndianAsParent(Fixture fixture) { + void copyWithoutOffsetAndSizeHasSameEndianAsParent(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { buf.order(BIG_ENDIAN); buf.writeLong(0x0102030405060708L); - try (Buffer slice = buf.slice()) { - assertEquals(0x0102030405060708L, slice.readLong()); + try (Buffer copy = buf.copy()) { + assertEquals(0x0102030405060708L, copy.readLong()); } buf.order(LITTLE_ENDIAN); - try (Buffer slice = buf.slice()) { - assertEquals(0x0807060504030201L, slice.readLong()); + try (Buffer copy = buf.copy()) { + assertEquals(0x0807060504030201L, copy.readLong()); } } } @ParameterizedTest @MethodSource("allocators") - void sliceWithOffsetAndSizeHasSameEndianAsParent(Fixture fixture) { + void copyWithOffsetAndSizeHasSameEndianAsParent(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { buf.order(BIG_ENDIAN); buf.writeLong(0x0102030405060708L); - try (Buffer slice = buf.slice(0, 8)) { - assertEquals(0x0102030405060708L, slice.readLong()); + try (Buffer copy = buf.copy(0, 8)) { + assertEquals(0x0102030405060708L, copy.readLong()); } buf.order(LITTLE_ENDIAN); - try (Buffer slice = buf.slice(0, 8)) { - assertEquals(0x0807060504030201L, slice.readLong()); + try (Buffer copy = buf.copy(0, 8)) { + assertEquals(0x0807060504030201L, copy.readLong()); } } } @ParameterizedTest @MethodSource("allocators") - void sendOnSliceWithoutOffsetAndSizeMustThrow(Fixture fixture) { + void sendOnCopyWithoutOffsetAndSizeMustNotThrow(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { - try (Buffer slice = buf.slice()) { + try (Buffer copy = buf.copy()) { assertFalse(asRS(buf).isOwned()); - assertThrows(IllegalStateException.class, slice::send); + copy.send().discard(); } // Verify that the slice is closed properly afterwards. assertTrue(asRS(buf).isOwned()); @@ -237,12 +241,12 @@ public class BufferReferenceCountingTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - void sendOnSliceWithOffsetAndSizeMustThrow(Fixture fixture) { + void sendOnCopyWithOffsetAndSizeMustThrow(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { - try (Buffer slice = buf.slice(0, 8)) { + try (Buffer copy = buf.copy(0, 8)) { assertFalse(asRS(buf).isOwned()); - assertThrows(IllegalStateException.class, slice::send); + copy.send().discard(); } // Verify that the slice is closed properly afterwards. assertTrue(asRS(buf).isOwned()); @@ -251,10 +255,10 @@ public class BufferReferenceCountingTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - void sliceWithNegativeOffsetMustThrow(Fixture fixture) { + void copyWithNegativeOffsetMustThrow(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { - assertThrows(IndexOutOfBoundsException.class, () -> buf.slice(-1, 1)); + assertThrows(IndexOutOfBoundsException.class, () -> buf.copy(-1, 1)); // Verify that the slice is closed properly afterwards. assertTrue(asRS(buf).isOwned()); } @@ -262,11 +266,11 @@ public class BufferReferenceCountingTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - void sliceWithNegativeSizeMustThrow(Fixture fixture) { + void copyWithNegativeSizeMustThrow(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { - assertThrows(IllegalArgumentException.class, () -> buf.slice(0, -1)); - assertThrows(IllegalArgumentException.class, () -> buf.slice(2, -1)); + assertThrows(IllegalArgumentException.class, () -> buf.copy(0, -1)); + assertThrows(IllegalArgumentException.class, () -> buf.copy(2, -1)); // Verify that the slice is closed properly afterwards. assertTrue(asRS(buf).isOwned()); } @@ -274,12 +278,12 @@ public class BufferReferenceCountingTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - void sliceWithSizeGreaterThanCapacityMustThrow(Fixture fixture) { + void copyWithSizeGreaterThanCapacityMustThrow(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { - assertThrows(IndexOutOfBoundsException.class, () -> buf.slice(0, 9)); - buf.slice(0, 8).close(); // This is still fine. - assertThrows(IndexOutOfBoundsException.class, () -> buf.slice(1, 8)); + assertThrows(IndexOutOfBoundsException.class, () -> buf.copy(0, 9)); + buf.copy(0, 8).close(); // This is still fine. + assertThrows(IndexOutOfBoundsException.class, () -> buf.copy(1, 8)); // Verify that the slice is closed properly afterwards. assertTrue(asRS(buf).isOwned()); } @@ -287,10 +291,10 @@ public class BufferReferenceCountingTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - void sliceWithZeroSizeMustBeAllowed(Fixture fixture) { + void copyWithZeroSizeMustBeAllowed(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { - buf.slice(0, 0).close(); // This is fine. + buf.copy(0, 0).close(); // This is fine. // Verify that the slice is closed properly afterwards. assertTrue(asRS(buf).isOwned()); } @@ -298,17 +302,19 @@ public class BufferReferenceCountingTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - public void sliceMustBecomeOwnedOnSourceBufferClose(Fixture fixture) { + public void copyMustBeOwned(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator()) { Buffer buf = allocator.allocate(8); buf.writeInt(42); - try (Buffer slice = buf.slice()) { + try (Buffer copy = buf.copy()) { + assertTrue(asRS(copy).isOwned()); + assertTrue(asRS(buf).isOwned()); buf.close(); assertFalse(buf.isAccessible()); - assertTrue(asRS(slice).isOwned()); - try (Buffer receive = slice.send().receive()) { + assertTrue(asRS(copy).isOwned()); + try (Buffer receive = copy.send().receive()) { assertTrue(asRS(receive).isOwned()); - assertFalse(slice.isAccessible()); + assertFalse(copy.isAccessible()); } } } @@ -521,20 +527,20 @@ public class BufferReferenceCountingTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - public void mustBePossibleToSplitOwnedSlices(Fixture fixture) { + public void mustBePossibleToSplitCopies(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator()) { Buffer buf = allocator.allocate(16).order(BIG_ENDIAN); buf.writeLong(0x0102030405060708L); - try (Buffer slice = buf.slice()) { + try (Buffer copy = buf.copy()) { buf.close(); - assertTrue(asRS(slice).isOwned()); - try (Buffer split = slice.split(4)) { + assertTrue(asRS(copy).isOwned()); + try (Buffer split = copy.split(4)) { split.reset().ensureWritable(Long.BYTES); - slice.reset().ensureWritable(Long.BYTES); + copy.reset().ensureWritable(Long.BYTES); assertThat(split.capacity()).isEqualTo(Long.BYTES); - assertThat(slice.capacity()).isEqualTo(Long.BYTES); + assertThat(copy.capacity()).isEqualTo(Long.BYTES); assertThat(split.getLong(0)).isEqualTo(0x01020304_00000000L); - assertThat(slice.getLong(0)).isEqualTo(0x05060708_00000000L); + assertThat(copy.getLong(0)).isEqualTo(0x05060708_00000000L); } } } @@ -665,13 +671,13 @@ public class BufferReferenceCountingTest extends BufferTestSupport { @ParameterizedTest @MethodSource("allocators") - public void sliceOfReadOnlyBufferMustBeReadOnly(Fixture fixture) { + public void copyOfReadOnlyBufferMustBeReadOnly(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { buf.writeLong(0x0102030405060708L); buf.makeReadOnly(); - try (Buffer slice = buf.slice()) { - assertTrue(slice.readOnly()); + try (Buffer copy = buf.copy()) { + assertTrue(copy.readOnly()); } } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java index 602fd2c..850ff85 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java @@ -19,7 +19,6 @@ import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; import io.netty.buffer.api.CompositeBuffer; import io.netty.buffer.api.MemoryManagers; -import io.netty.buffer.api.internal.ResourceSupport; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -272,8 +271,7 @@ public abstract class BufferTestSupport { } var stream = builder.build(); - return stream.flatMap(BufferTestSupport::injectSplits) - .flatMap(BufferTestSupport::injectSlices); + return stream.flatMap(BufferTestSupport::injectSplits); } private static Stream injectSplits(Fixture f) { @@ -303,59 +301,6 @@ public abstract class BufferTestSupport { return builder.build(); } - private static Stream injectSlices(Fixture f) { - Builder builder = Stream.builder(); - builder.add(f); - var props = concat(f.getProperties(), Fixture.Properties.SLICE); - builder.add(new Fixture(f + ".slice(0, capacity())", () -> { - return new BufferAllocator() { - BufferAllocator allocatorBase; - @Override - public Buffer allocate(int size) { - allocatorBase = f.get(); - try (Buffer base = allocatorBase.allocate(size)) { - return base.slice(0, base.capacity()).writerOffset(0); - } - } - - @Override - public void close() { - if (allocatorBase != null) { - allocatorBase.close(); - allocatorBase = null; - } - } - }; - }, props)); - builder.add(new Fixture(f + ".slice(1, capacity() - 2)", () -> { - return new BufferAllocator() { - BufferAllocator allocatorBase; - @Override - public Buffer allocate(int size) { - allocatorBase = f.get(); - try (Buffer base = allocatorBase.allocate(size + 2)) { - return base.slice(1, size).writerOffset(0); - } - } - - @Override - public void close() { - if (allocatorBase != null) { - allocatorBase.close(); - allocatorBase = null; - } - } - }; - }, props)); - return builder.build(); - } - - private static Fixture.Properties[] concat(Fixture.Properties[] props, Fixture.Properties prop) { - props = Arrays.copyOf(props, props.length + 1); - props[props.length - 1] = prop; - return props; - } - @BeforeAll static void startExecutor() throws IOException, ParseException { executor = Executors.newSingleThreadExecutor(new ThreadFactory() { @@ -392,7 +337,7 @@ public abstract class BufferTestSupport { assertThrows(IllegalStateException.class, () -> buf.split()); assertThrows(IllegalStateException.class, () -> asRS(buf).send()); assertThrows(IllegalStateException.class, () -> asRS(buf).acquire()); - assertThrows(IllegalStateException.class, () -> buf.slice()); + assertThrows(IllegalStateException.class, () -> buf.copy()); assertThrows(IllegalStateException.class, () -> buf.openCursor()); assertThrows(IllegalStateException.class, () -> buf.openCursor(0, 0)); assertThrows(IllegalStateException.class, () -> buf.openReverseCursor()); @@ -864,67 +809,6 @@ public abstract class BufferTestSupport { } } - public static void verifyWriteAccessible(Buffer buf) { - buf.reset().writeByte((byte) 32); - assertThat(buf.readByte()).isEqualTo((byte) 32); - buf.reset().writerOffset(0).writeUnsignedByte(32); - assertThat(buf.readUnsignedByte()).isEqualTo(32); - buf.reset().writerOffset(0).writeChar('3'); - assertThat(buf.readChar()).isEqualTo('3'); - buf.reset().writerOffset(0).writeShort((short) 32); - assertThat(buf.readShort()).isEqualTo((short) 32); - buf.reset().writerOffset(0).writeUnsignedShort(32); - assertThat(buf.readUnsignedShort()).isEqualTo(32); - buf.reset().writerOffset(0).writeMedium(32); - assertThat(buf.readMedium()).isEqualTo(32); - buf.reset().writerOffset(0).writeUnsignedMedium(32); - assertThat(buf.readUnsignedMedium()).isEqualTo(32); - buf.reset().writerOffset(0).writeInt(32); - assertThat(buf.readInt()).isEqualTo(32); - buf.reset().writerOffset(0).writeUnsignedInt(32); - assertThat(buf.readUnsignedInt()).isEqualTo(32L); - buf.reset().writerOffset(0).writeFloat(3.2f); - assertThat(buf.readFloat()).isEqualTo(3.2f); - buf.reset().writerOffset(0).writeLong(32); - assertThat(buf.readLong()).isEqualTo(32L); - buf.reset().writerOffset(0).writeDouble(3.2); - assertThat(buf.readDouble()).isEqualTo(3.2); - - buf.setByte(0, (byte) 32); - assertThat(buf.getByte(0)).isEqualTo((byte) 32); - buf.setUnsignedByte(0, 32); - assertThat(buf.getUnsignedByte(0)).isEqualTo(32); - buf.setChar(0, '3'); - assertThat(buf.getChar(0)).isEqualTo('3'); - buf.setShort(0, (short) 32); - assertThat(buf.getShort(0)).isEqualTo((short) 32); - buf.setUnsignedShort(0, 32); - assertThat(buf.getUnsignedShort(0)).isEqualTo(32); - buf.setMedium(0, 32); - assertThat(buf.getMedium(0)).isEqualTo(32); - buf.setUnsignedMedium(0, 32); - assertThat(buf.getUnsignedMedium(0)).isEqualTo(32); - buf.setInt(0, 32); - assertThat(buf.getInt(0)).isEqualTo(32); - buf.setUnsignedInt(0, 32); - assertThat(buf.getUnsignedInt(0)).isEqualTo(32L); - buf.setFloat(0, 3.2f); - assertThat(buf.getFloat(0)).isEqualTo(3.2f); - buf.setLong(0, 32); - assertThat(buf.getLong(0)).isEqualTo(32L); - buf.setDouble(0, 3.2); - assertThat(buf.getDouble(0)).isEqualTo(3.2); - - if (asRS(buf).isOwned()) { - buf.ensureWritable(1); - } - buf.fill((byte) 0); - try (BufferAllocator allocator = BufferAllocator.heap(); - Buffer source = allocator.allocate(8)) { - source.copyInto(0, buf, 0, 1); - } - } - public static void verifyForEachReadableSingleComponent(Fixture fixture, Buffer buf) { buf.forEachReadable(0, (index, component) -> { var buffer = component.readableBuffer(); @@ -999,14 +883,21 @@ public abstract class BufferTestSupport { return bs; } - public static int countBorrows(Buffer buf) { - return ((ResourceSupport) buf).countBorrows(); + public static byte[] readByteArray(Buffer buf) { + byte[] bs = new byte[buf.readableBytes()]; + buf.copyInto(buf.readerOffset(), bs, 0, bs.length); + buf.readerOffset(buf.writerOffset()); + return bs; } public static void assertEquals(Buffer expected, Buffer actual) { assertThat(toByteArray(actual)).containsExactly(toByteArray(expected)); } + public static void assertReadableEquals(Buffer expected, Buffer actual) { + assertThat(readByteArray(actual)).containsExactly(readByteArray(expected)); + } + public static void assertEquals(byte expected, byte actual) { if (expected != actual) { fail(String.format("expected: %1$s (0x%1$X) but was: %2$s (0x%2$X)", expected, actual)); diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferWriteBytesCombinationsTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferWriteBytesCombinationsTest.java index 001224f..0c21f5c 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferWriteBytesCombinationsTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferWriteBytesCombinationsTest.java @@ -76,8 +76,6 @@ public class BufferWriteBytesCombinationsTest extends BufferTestSupport { assertThat(target.writerOffset()).isEqualTo(35); assertThat(source.readerOffset()).isEqualTo(35); assertThat(source.writerOffset()).isEqualTo(35); - try (Buffer readableSlice = target.slice()) { - assertEquals(source, readableSlice); - } + assertReadableEquals(source, target); } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/Fixture.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/Fixture.java index 97a3289..3d3e5e5 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/Fixture.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/Fixture.java @@ -70,10 +70,6 @@ public final class Fixture implements Supplier { return properties.contains(Properties.CLEANER); } - public boolean isSlice() { - return properties.contains(Properties.SLICE); - } - public boolean isConst() { return properties.contains(Properties.CONST); } @@ -84,7 +80,6 @@ public final class Fixture implements Supplier { CONST, COMPOSITE, CLEANER, - POOLED, - SLICE + POOLED } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSlicingExample.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSplittingExample.java similarity index 95% rename from buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSlicingExample.java rename to buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSplittingExample.java index b6519ab..cffa5a7 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSlicingExample.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSplittingExample.java @@ -22,7 +22,7 @@ import io.netty.buffer.api.Scope; import java.util.concurrent.ThreadLocalRandom; -public final class ComposingAndSlicingExample { +public final class ComposingAndSplittingExample { public static void main(String[] args) { try (BufferAllocator allocator = BufferAllocator.pooledDirect(); Buffer buf = createBigBuffer(allocator)) { @@ -32,7 +32,7 @@ public final class ComposingAndSlicingExample { buf.writeByte((byte) tlr.nextInt()); } - try (Buffer slice = buf.slice()) { + try (Buffer slice = buf.split()) { slice.send(); System.out.println("buf.capacity() = " + buf.capacity()); System.out.println("buf.readableBytes() = " + buf.readableBytes()); diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/SendExample.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/SendExample.java index e017461..bc2f785 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/SendExample.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/SendExample.java @@ -103,85 +103,6 @@ public class SendExample { } static final class Ex3 { - public static void main(String[] args) throws Exception { - ExecutorService executor = newFixedThreadPool(4); - BufferAllocator allocator = BufferAllocator.heap(); - - try (Buffer buf = allocator.allocate(4096)) { - // !!! pit-fall: Rc decrement in other thread. - var futA = executor.submit(new Task(buf.slice(0, 1024))); - var futB = executor.submit(new Task(buf.slice(1024, 1024))); - var futC = executor.submit(new Task(buf.slice(2048, 1024))); - var futD = executor.submit(new Task(buf.slice(3072, 1024))); - futA.get(); - futB.get(); - futC.get(); - futD.get(); - } - - allocator.close(); - executor.shutdown(); - } - - private static class Task implements Runnable { - private final Buffer slice; - - Task(Buffer slice) { - this.slice = slice; - } - - @Override - public void run() { - try (slice) { - while (slice.writableBytes() > 0) { - slice.writeByte((byte) 42); - } - } - } - } - } - - static final class Ex4 { - public static void main(String[] args) throws Exception { - ExecutorService executor = newFixedThreadPool(4); - BufferAllocator allocator = BufferAllocator.heap(); - - try (Buffer buf = allocator.allocate(4096); - Buffer sliceA = buf.slice(0, 1024); - Buffer sliceB = buf.slice(1024, 1024); - Buffer sliceC = buf.slice(2048, 1024); - Buffer sliceD = buf.slice(3072, 1024)) { - var futA = executor.submit(new Task(sliceA)); - var futB = executor.submit(new Task(sliceB)); - var futC = executor.submit(new Task(sliceC)); - var futD = executor.submit(new Task(sliceD)); - futA.get(); - futB.get(); - futC.get(); - futD.get(); - } - - allocator.close(); - executor.shutdown(); - } - - private static class Task implements Runnable { - private final Buffer slice; - - Task(Buffer slice) { - this.slice = slice; - } - - @Override - public void run() { - while (slice.writableBytes() > 0) { - slice.writeByte((byte) 42); - } - } - } - } - - static final class Ex5 { public static void main(String[] args) throws Exception { ExecutorService executor = newFixedThreadPool(4); BufferAllocator allocator = BufferAllocator.heap(); diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/AlternativeMessageDecoderTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/AlternativeMessageDecoderTest.java index d9584ed..e4c1375 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/AlternativeMessageDecoderTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/AlternativeMessageDecoderTest.java @@ -26,6 +26,7 @@ import java.util.Iterator; import java.util.List; import java.util.SplittableRandom; +import static io.netty.buffer.api.tests.BufferTestSupport.readByteArray; import static io.netty.buffer.api.tests.BufferTestSupport.toByteArray; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -89,9 +90,7 @@ public class AlternativeMessageDecoderTest { while ((actualMessage = channel.readInbound()) != null) { try (Buffer ignore = actualMessage) { assertTrue(expectedItr.hasNext()); - try (Buffer actual = actualMessage.slice()) { - assertThat(toByteArray(actual)).containsExactly(expectedItr.next()); - } + assertThat(readByteArray(actualMessage)).containsExactly(expectedItr.next()); } } assertFalse(expectedItr.hasNext()); diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoder.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoder.java index 1c5de0d..d35670f 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoder.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoder.java @@ -134,7 +134,7 @@ public abstract class ByteToMessageDecoder extends ChannelHandlerAdapter { if (composite.writerOffset() != composite.capacity()) { // Writer index must equal capacity if we are going to "write" // new components to the end. - composite = cumulation.slice(0, composite.writerOffset()); + composite = cumulation.split(composite.writerOffset()); cumulation.close(); } } else { diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java index 838bc7c..ee16786 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java @@ -33,6 +33,7 @@ import java.util.concurrent.atomic.AtomicInteger; import static io.netty.buffer.api.internal.Statics.asRS; import static io.netty.buffer.api.tests.BufferTestSupport.assertEquals; import static io.netty.buffer.api.CompositeBuffer.compose; +import static io.netty.buffer.api.tests.BufferTestSupport.assertReadableEquals; import static java.nio.ByteOrder.BIG_ENDIAN; import static java.nio.ByteOrder.LITTLE_ENDIAN; import static org.assertj.core.api.Assertions.assertThat; @@ -61,11 +62,13 @@ public class ByteToMessageDecoderTest { } }); - try (Buffer buf = BufferAllocator.heap().allocate(4).writeInt(0x01020304)) { - channel.writeInbound(buf.slice()); + try (Buffer buf = BufferAllocator.heap().allocate(4, BIG_ENDIAN).writeInt(0x01020304)) { + channel.writeInbound(buf); try (Buffer b = channel.readInbound()) { - buf.readByte(); - assertEquals(b, buf); + assertEquals(3, b.readableBytes()); + assertEquals(0x02, b.readByte()); + assertEquals(0x03, b.readByte()); + assertEquals(0x04, b.readByte()); } } } @@ -88,12 +91,10 @@ public class ByteToMessageDecoderTest { } }); - channel.writeInbound(buf.slice()); + channel.writeInbound(buf); try (Buffer expected = BufferAllocator.heap().allocate(3, BIG_ENDIAN).writeShort((short) 0x0203).writeByte((byte) 0x04); - Buffer b = channel.readInbound(); - Buffer actual = b.slice(); // Only compare readable bytes. - buf) { - assertEquals(expected, actual); + Buffer actual = channel.readInbound()) { + assertReadableEquals(expected, actual); } } @@ -120,9 +121,8 @@ public class ByteToMessageDecoderTest { assertTrue(channel.writeInbound(buf)); assertTrue(channel.finish()); try (Buffer expected = BufferAllocator.heap().allocate(1).writeByte((byte) 0x02); - Buffer b = channel.readInbound(); - Buffer actual = b.slice()) { - assertEquals(expected, actual); + Buffer actual = channel.readInbound()) { + assertReadableEquals(expected, actual); assertNull(channel.readInbound()); } } @@ -243,11 +243,10 @@ public class ByteToMessageDecoderTest { }); try (Buffer buf = BufferAllocator.heap().allocate(4, BIG_ENDIAN).writeInt(0x01020304)) { - assertTrue(channel.writeInbound(buf.slice())); - try (Buffer expected = buf.slice(1, 3); - Buffer b = channel.readInbound(); - Buffer actual = b.slice()) { - assertEquals(expected, actual); + assertTrue(channel.writeInbound(buf.copy())); + try (Buffer expected = buf.copy(1, 3); + Buffer actual = channel.readInbound()) { + assertReadableEquals(expected, actual); assertFalse(channel.finish()); } } @@ -259,9 +258,8 @@ public class ByteToMessageDecoderTest { @Override protected void decode(ChannelHandlerContext ctx, Buffer in) { assertTrue(in.readableBytes() > 0); - Buffer slice = in.slice(); - in.readerOffset(in.readerOffset() + in.readableBytes()); - ctx.fireChannelRead(slice); + Buffer chunk = in.split(); + ctx.fireChannelRead(chunk); } }); byte[] bytes = new byte[1024]; @@ -271,9 +269,9 @@ public class ByteToMessageDecoderTest { for (byte b : bytes) { buf.writeByte(b); } - assertTrue(channel.writeInbound(buf.slice())); + assertTrue(channel.writeInbound(buf.copy())); try (Buffer b = channel.readInbound()) { - assertEquals(buf, b); + assertReadableEquals(buf, b); assertNull(channel.readInbound()); assertFalse(channel.finish()); assertNull(channel.readInbound()); @@ -294,9 +292,8 @@ public class ByteToMessageDecoderTest { return; } int read = decodeLast ? readable : readable - 1; - Buffer slice = in.slice(in.readerOffset(), read); - in.readerOffset(in.readerOffset() + read); - ctx.fireChannelRead(slice); + Buffer chunk = in.split(in.readerOffset() + read); + ctx.fireChannelRead(chunk); } @Override @@ -308,13 +305,10 @@ public class ByteToMessageDecoderTest { }); byte[] bytes = new byte[1024]; ThreadLocalRandom.current().nextBytes(bytes); - try (Buffer buf = BufferAllocator.heap().allocate(bytes.length, BIG_ENDIAN); - Buffer part1 = buf.slice(0, bytes.length - 1); - Buffer part2 = buf.slice(bytes.length - 1, 1)) { - for (byte b : bytes) { - buf.writeByte(b); - } - assertTrue(channel.writeInbound(buf.slice())); + try (Buffer buf = BufferAllocator.heap().allocate(bytes.length, BIG_ENDIAN).writeBytes(bytes); + Buffer part1 = buf.copy(0, bytes.length - 1); + Buffer part2 = buf.copy(bytes.length - 1, 1)) { + assertTrue(channel.writeInbound(buf)); try (Buffer actual = channel.readInbound()) { assertEquals(part1, actual); } @@ -523,7 +517,6 @@ public class ByteToMessageDecoderTest { public void testDecodeLast() { final AtomicBoolean removeHandler = new AtomicBoolean(); EmbeddedChannel channel = new EmbeddedChannel(new ByteToMessageDecoder() { - @Override protected void decode(ChannelHandlerContext ctx, Buffer in) { if (removeHandler.get()) { @@ -533,12 +526,8 @@ public class ByteToMessageDecoderTest { }); byte[] bytes = new byte[1024]; ThreadLocalRandom.current().nextBytes(bytes); - try (Buffer buf = BufferAllocator.heap().allocate(bytes.length)) { - for (byte b : bytes) { - buf.writeByte(b); - } - - assertFalse(channel.writeInbound(buf.slice())); + try (Buffer buf = BufferAllocator.heap().allocate(bytes.length).writeBytes(bytes)) { + assertFalse(channel.writeInbound(buf.copy())); assertNull(channel.readInbound()); removeHandler.set(true); // This should trigger channelInputClosed(...) @@ -546,7 +535,7 @@ public class ByteToMessageDecoderTest { assertTrue(channel.finish()); try (Buffer actual = channel.readInbound()) { - assertEquals(buf.slice(), actual); + assertReadableEquals(buf, actual); } assertNull(channel.readInbound()); } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/FixedLengthFrameDecoder.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/FixedLengthFrameDecoder.java index 5bc1807..a999d3b 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/FixedLengthFrameDecoder.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/FixedLengthFrameDecoder.java @@ -54,11 +54,7 @@ public class FixedLengthFrameDecoder extends ByteToMessageDecoder { if (in.readableBytes() < frameLength) { return null; } else { - try { - return in.slice(in.readerOffset(), frameLength); - } finally { - in.readerOffset(in.readerOffset() + frameLength); - } + return in.split(in.readerOffset() + frameLength); } } } From 1c25fa88b7e8cd542dc3d12f5ebf80d337e553a8 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Thu, 27 May 2021 17:06:30 +0200 Subject: [PATCH 05/12] Fix test failures coming from removal of `slice` and introduction of `copy`. --- .../main/java/io/netty/buffer/api/Buffer.java | 17 +++-- .../io/netty/buffer/api/CompositeBuffer.java | 2 +- .../buffer/api/bytebuffer/NioBuffer.java | 13 ++-- .../io/netty/buffer/api/internal/Statics.java | 5 ++ .../netty/buffer/api/unsafe/UnsafeBuffer.java | 10 +-- .../netty/buffer/api/memseg/MemSegBuffer.java | 22 +++++- .../api/tests/BufferEnsureWritableTest.java | 3 +- .../buffer/api/tests/BufferReadOnlyTest.java | 4 +- .../tests/BufferReferenceCountingTest.java | 72 +++++++++---------- .../BufferWriteBytesCombinationsTest.java | 1 + .../ComposingAndSplittingExample.java | 8 +-- .../ByteToMessageDecoder.java | 8 +-- 12 files changed, 92 insertions(+), 73 deletions(-) diff --git a/buffer-api/src/main/java/io/netty/buffer/api/Buffer.java b/buffer-api/src/main/java/io/netty/buffer/api/Buffer.java index ca3f3ab..63985c7 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/Buffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/Buffer.java @@ -74,7 +74,7 @@ import java.nio.ByteOrder; * 0 <= readerOffset <= writerOffset <= capacity * * - *

Splitting buffers

+ *

Splitting buffers

* * The {@link #split()} method break a buffer into two. * The two buffers will share the underlying memory, but their regions will not overlap, ensuring that the memory is @@ -185,9 +185,8 @@ public interface Buffer extends Resource, BufferAccessors { long nativeAddress(); /** - * Make this buffer read-only. This is irreversible. - * Unless a writable slice has previously been obtained from this buffer, there will no longer be any way to modify - * the data contained in this buffer. + * Make this buffer read-only. + * This is irreversible. * * @return this buffer. */ @@ -437,7 +436,9 @@ public interface Buffer extends Resource, BufferAccessors { * that contains a copy of the readable region of this buffer. */ default Buffer copy() { - return copy(readerOffset(), readableBytes()); + int offset = readerOffset(); + int length = readableBytes(); + return copy(offset, length); } /** @@ -494,8 +495,7 @@ public interface Buffer extends Resource, BufferAccessors { *

* Split buffers support all operations that normal buffers do, including {@link #ensureWritable(int)}. *

- * See the Slice vs. Split section for details on the difference between slice - * and split. + * See the Splitting buffers section for details. * * @return A new buffer with independent and exclusive ownership over the read and readable bytes from this buffer. */ @@ -543,8 +543,7 @@ public interface Buffer extends Resource, BufferAccessors { *

* Split buffers support all operations that normal buffers do, including {@link #ensureWritable(int)}. *

- * See the Slice vs. Split section for details on the difference between slice - * and split. + * See the Splitting buffers section for details. * * @return A new buffer with independent and exclusive ownership over the bytes from the beginning to the given * offset of this buffer. diff --git a/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java b/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java index 06158dc..5d27f76 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java @@ -438,7 +438,7 @@ public final class CompositeBuffer extends ResourceSupport implements Buffer, Re if (!isAccessible()) { throw new IllegalStateException("This buffer is closed: " + this + '.'); } - AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, length); - ByteBuffer byteBuffer = memory.memory(); - Buffer copy = new NioBuffer(byteBuffer, byteBuffer, control, memory.drop()); - copyInto(0, copy, 0, length); + int allocSize = Math.max(length, 1); // Allocators don't support allocating zero-sized buffers. + AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, allocSize); + ByteBuffer base = memory.memory(); + ByteBuffer buffer = length == 0? bbslice(base, 0, 0) : base; + Buffer copy = new NioBuffer(base, buffer, control, memory.drop()); + copyInto(offset, copy, 0, length); copy.writerOffset(length).order(order()); if (readOnly()) { copy = copy.makeReadOnly(); @@ -465,8 +467,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re if (readOnly) { splitBuffer.makeReadOnly(); } - // Note that split, unlike slice, does not deconstify, because data changes in either buffer are not visible - // in the other. The split buffers can later deconstify independently if needed. + // Split preserves const-state. splitBuffer.constBuffer = constBuffer; rmem = bbslice(rmem, splitOffset, rmem.capacity() - splitOffset); if (!readOnly) { diff --git a/buffer-api/src/main/java/io/netty/buffer/api/internal/Statics.java b/buffer-api/src/main/java/io/netty/buffer/api/internal/Statics.java index 6f0cd2c..ee0258c 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/internal/Statics.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/internal/Statics.java @@ -64,6 +64,11 @@ public interface Statics { } } + @SuppressWarnings("unchecked") + static Drop noOpDrop() { + return (Drop) NO_OP_DROP; + } + static VarHandle findVarHandle(Lookup lookup, Class recv, String name, Class type) { try { return lookup.findVarHandle(recv, name, type); diff --git a/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java b/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java index eaddde7..a5487fa 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java @@ -170,9 +170,12 @@ class UnsafeBuffer extends ResourceSupport implements Buff throw new IllegalArgumentException("Length cannot be negative: " + length + '.'); } checkGet(offset, length); - AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, length); + + int allocSize = Math.max(length, 1); // Allocators don't support allocating zero-sized buffers. + AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, allocSize); UnsafeMemory unsafeMemory = memory.memory(); - Buffer copy = new UnsafeBuffer(unsafeMemory, unsafeMemory.address, length, control, memory.drop()); + Buffer copy = new UnsafeBuffer(unsafeMemory, 0, length, control, memory.drop()); + copyInto(offset, copy, 0, length); copy.writerOffset(length).order(order); if (readOnly) { copy = copy.makeReadOnly(); @@ -509,8 +512,7 @@ class UnsafeBuffer extends ResourceSupport implements Buff if (readOnly) { splitBuffer.makeReadOnly(); } - // Note that split, unlike slice, does not deconstify, because data changes in either buffer are not visible - // in the other. The split buffers can later deconstify independently if needed. + // Split preserves const-state. splitBuffer.constBuffer = constBuffer; rsize -= splitOffset; baseOffset += splitOffset; diff --git a/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java b/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java index aeca528..8ca6939 100644 --- a/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java +++ b/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java @@ -56,6 +56,8 @@ import static jdk.incubator.foreign.MemoryAccess.setShortAtOffset; class MemSegBuffer extends ResourceSupport implements Buffer, ReadableComponent, WritableComponent, BufferIntegratable { private static final MemorySegment CLOSED_SEGMENT; + private static final MemorySegment ZERO_OFFHEAP_SEGMENT; + private static final MemorySegment ZERO_ONHEAP_SEGMENT; static final Drop SEGMENT_CLOSE; static { @@ -66,6 +68,8 @@ class MemSegBuffer extends ResourceSupport implements Buff MemorySegment segment = MemorySegment.allocateNative(1, scope); CLOSED_SEGMENT = segment.asSlice(0, 0); } + ZERO_OFFHEAP_SEGMENT = MemorySegment.allocateNative(1, ResourceScope.newImplicitScope()).asSlice(0, 0); + ZERO_ONHEAP_SEGMENT = MemorySegment.ofArray(new byte[0]); SEGMENT_CLOSE = new Drop() { @Override public void drop(MemSegBuffer buf) { @@ -294,16 +298,29 @@ class MemSegBuffer extends ResourceSupport implements Buff @Override public Buffer copy(int offset, int length) { + checkGet(offset, length); if (length < 0) { throw new IllegalArgumentException("Length cannot be negative: " + length + '.'); } if (!isAccessible()) { throw new IllegalStateException("This buffer is closed: " + this + '.'); } + + if (length == 0) { + // Special case zero-length segments, since allocators don't support allocating empty buffers. + final MemorySegment zero; + if (nativeAddress() == 0) { + zero = ZERO_ONHEAP_SEGMENT; + } else { + zero = ZERO_OFFHEAP_SEGMENT; + } + return new MemSegBuffer(zero, zero, Statics.noOpDrop(), control); + } + AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, length); MemorySegment segment = memory.memory(); Buffer copy = new MemSegBuffer(segment, segment, memory.drop(), control); - copyInto(0, copy, 0, length); + copyInto(offset, copy, 0, length); copy.writerOffset(length).order(order()); if (readOnly()) { copy = copy.makeReadOnly(); @@ -576,8 +593,7 @@ class MemSegBuffer extends ResourceSupport implements Buff if (readOnly) { splitBuffer.makeReadOnly(); } - // Note that split, unlike slice, does not deconstify, because data changes in either buffer are not visible - // in the other. The split buffers can later deconstify independently if needed. + // Split preserves const-state. splitBuffer.constBuffer = constBuffer; seg = seg.asSlice(splitOffset, seg.byteSize() - splitOffset); if (!readOnly) { diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java index f5da1d1..18b428e 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferEnsureWritableTest.java @@ -100,7 +100,8 @@ public class BufferEnsureWritableTest extends BufferTestSupport { assertThat(buf.capacity()).isGreaterThanOrEqualTo(8); buf.writeLong(0x0102030405060708L); try (Buffer copy = buf.copy()) { - assertEquals(0x0102030405060708L, copy.readLong()); + long actual = copy.readLong(); + assertEquals(0x0102030405060708L, actual); } } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java index 0b4ea8c..3ca1999 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java @@ -197,8 +197,8 @@ public class BufferReadOnlyTest extends BufferTestSupport { assertThat(b.capacity()).isEqualTo(8); try (Buffer c = b.copy()) { assertTrue(c.readOnly()); - assertFalse(asRS(c).isOwned()); - assertFalse(asRS(b).isOwned()); + assertTrue(asRS(c).isOwned()); + assertTrue(asRS(b).isOwned()); assertThat(c.capacity()).isEqualTo(8); } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java index be26311..822507a 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java @@ -116,19 +116,19 @@ public class BufferReferenceCountingTest extends BufferTestSupport { } assertEquals(0x01, buf.readByte()); buf.writerOffset(buf.writerOffset() - 1); - try (Buffer slice = buf.copy()) { - assertThat(toByteArray(slice)).containsExactly(0x02, 0x03, 0x04, 0x05, 0x06, 0x07); - assertEquals(0, slice.readerOffset()); - assertEquals(6, slice.readableBytes()); - assertEquals(6, slice.writerOffset()); - assertEquals(6, slice.capacity()); - assertEquals(0x02, slice.readByte()); - assertEquals(0x03, slice.readByte()); - assertEquals(0x04, slice.readByte()); - assertEquals(0x05, slice.readByte()); - assertEquals(0x06, slice.readByte()); - assertEquals(0x07, slice.readByte()); - assertThrows(IndexOutOfBoundsException.class, slice::readByte); + try (Buffer copy = buf.copy()) { + assertThat(toByteArray(copy)).containsExactly(0x02, 0x03, 0x04, 0x05, 0x06, 0x07); + assertEquals(0, copy.readerOffset()); + assertEquals(6, copy.readableBytes()); + assertEquals(6, copy.writerOffset()); + assertEquals(6, copy.capacity()); + assertEquals(0x02, copy.readByte()); + assertEquals(0x03, copy.readByte()); + assertEquals(0x04, copy.readByte()); + assertEquals(0x05, copy.readByte()); + assertEquals(0x06, copy.readByte()); + assertEquals(0x07, copy.readByte()); + assertThrows(IndexOutOfBoundsException.class, copy::readByte); } } } @@ -143,19 +143,19 @@ public class BufferReferenceCountingTest extends BufferTestSupport { } buf.readerOffset(3); // Reader and writer offsets must be ignored. buf.writerOffset(6); - try (Buffer slice = buf.copy(1, 6)) { - assertThat(toByteArray(slice)).containsExactly(0x02, 0x03, 0x04, 0x05, 0x06, 0x07); - assertEquals(0, slice.readerOffset()); - assertEquals(6, slice.readableBytes()); - assertEquals(6, slice.writerOffset()); - assertEquals(6, slice.capacity()); - assertEquals(0x02, slice.readByte()); - assertEquals(0x03, slice.readByte()); - assertEquals(0x04, slice.readByte()); - assertEquals(0x05, slice.readByte()); - assertEquals(0x06, slice.readByte()); - assertEquals(0x07, slice.readByte()); - assertThrows(IndexOutOfBoundsException.class, slice::readByte); + try (Buffer copy = buf.copy(1, 6)) { + assertThat(toByteArray(copy)).containsExactly(0x02, 0x03, 0x04, 0x05, 0x06, 0x07); + assertEquals(0, copy.readerOffset()); + assertEquals(6, copy.readableBytes()); + assertEquals(6, copy.writerOffset()); + assertEquals(6, copy.capacity()); + assertEquals(0x02, copy.readByte()); + assertEquals(0x03, copy.readByte()); + assertEquals(0x04, copy.readByte()); + assertEquals(0x05, copy.readByte()); + assertEquals(0x06, copy.readByte()); + assertEquals(0x07, copy.readByte()); + assertThrows(IndexOutOfBoundsException.class, copy::readByte); } } } @@ -167,11 +167,11 @@ public class BufferReferenceCountingTest extends BufferTestSupport { Buffer buf = allocator.allocate(8)) { try (Buffer copy = buf.copy()) { assertTrue(asRS(buf).isOwned()); - buf.send().discard(); assertTrue(asRS(copy).isOwned()); copy.send().discard(); } assertTrue(asRS(buf).isOwned()); + buf.send().discard(); } } @@ -182,11 +182,11 @@ public class BufferReferenceCountingTest extends BufferTestSupport { Buffer buf = allocator.allocate(8)) { try (Buffer copy = buf.copy(0, 8)) { assertTrue(asRS(buf).isOwned()); - buf.send().discard(); assertTrue(asRS(copy).isOwned()); copy.send().discard(); } assertTrue(asRS(buf).isOwned()); + buf.send().discard(); } } @@ -230,10 +230,10 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { try (Buffer copy = buf.copy()) { - assertFalse(asRS(buf).isOwned()); + assertTrue(asRS(buf).isOwned()); copy.send().discard(); } - // Verify that the slice is closed properly afterwards. + // Verify that the copy is closed properly afterwards. assertTrue(asRS(buf).isOwned()); buf.send().receive().close(); } @@ -245,10 +245,10 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { try (Buffer copy = buf.copy(0, 8)) { - assertFalse(asRS(buf).isOwned()); + assertTrue(asRS(buf).isOwned()); copy.send().discard(); } - // Verify that the slice is closed properly afterwards. + // Verify that the copy is closed properly afterwards. assertTrue(asRS(buf).isOwned()); } } @@ -259,7 +259,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { assertThrows(IndexOutOfBoundsException.class, () -> buf.copy(-1, 1)); - // Verify that the slice is closed properly afterwards. + // Verify that the copy is closed properly afterwards. assertTrue(asRS(buf).isOwned()); } } @@ -271,7 +271,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { Buffer buf = allocator.allocate(8)) { assertThrows(IllegalArgumentException.class, () -> buf.copy(0, -1)); assertThrows(IllegalArgumentException.class, () -> buf.copy(2, -1)); - // Verify that the slice is closed properly afterwards. + // Verify that the copy is closed properly afterwards. assertTrue(asRS(buf).isOwned()); } } @@ -284,7 +284,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { assertThrows(IndexOutOfBoundsException.class, () -> buf.copy(0, 9)); buf.copy(0, 8).close(); // This is still fine. assertThrows(IndexOutOfBoundsException.class, () -> buf.copy(1, 8)); - // Verify that the slice is closed properly afterwards. + // Verify that the copy is closed properly afterwards. assertTrue(asRS(buf).isOwned()); } } @@ -295,7 +295,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { buf.copy(0, 0).close(); // This is fine. - // Verify that the slice is closed properly afterwards. + // Verify that the copy is closed properly afterwards. assertTrue(asRS(buf).isOwned()); } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferWriteBytesCombinationsTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferWriteBytesCombinationsTest.java index 0c21f5c..500344c 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferWriteBytesCombinationsTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferWriteBytesCombinationsTest.java @@ -76,6 +76,7 @@ public class BufferWriteBytesCombinationsTest extends BufferTestSupport { assertThat(target.writerOffset()).isEqualTo(35); assertThat(source.readerOffset()).isEqualTo(35); assertThat(source.writerOffset()).isEqualTo(35); + source.readerOffset(0); assertReadableEquals(source, target); } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSplittingExample.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSplittingExample.java index cffa5a7..a05b2a1 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSplittingExample.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/ComposingAndSplittingExample.java @@ -32,13 +32,13 @@ public final class ComposingAndSplittingExample { buf.writeByte((byte) tlr.nextInt()); } - try (Buffer slice = buf.split()) { - slice.send(); + try (Buffer split = buf.split()) { + split.send(); System.out.println("buf.capacity() = " + buf.capacity()); System.out.println("buf.readableBytes() = " + buf.readableBytes()); System.out.println("---"); - System.out.println("slice.capacity() = " + slice.capacity()); - System.out.println("slice.readableBytes() = " + slice.readableBytes()); + System.out.println("split.capacity() = " + split.capacity()); + System.out.println("split.readableBytes() = " + split.readableBytes()); } } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoder.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoder.java index d35670f..14a14f5 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoder.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoder.java @@ -314,13 +314,7 @@ public abstract class ByteToMessageDecoder extends ChannelHandlerAdapter { protected final void discardSomeReadBytes() { if (cumulation != null && !first) { - // discard some bytes if possible to make more room in the - // buffer but only if the refCnt == 1 as otherwise the user may have - // used slice().retain() or duplicate().retain(). - // - // See: - // - https://github.com/netty/netty/issues/2327 - // - https://github.com/netty/netty/issues/1764 + // Discard some bytes if possible to make more room in the buffer. cumulation.compact(); } } From 05d76c27c19d83774c270019ee0cab7419b87e27 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Thu, 27 May 2021 17:34:40 +0200 Subject: [PATCH 06/12] Hide `isOwned`, `countBorrows`, and `acquire` from the public API, even on `CompositeBuffer` --- .../io/netty/buffer/api/CompositeBuffer.java | 6 +-- .../buffer/api/adaptor/ByteBufAdaptor.java | 13 ++--- .../buffer/api/internal/ResourceSupport.java | 19 +++++-- .../io/netty/buffer/api/internal/Statics.java | 20 ++++--- .../buffer/api/tests/BufferCompactTest.java | 5 +- .../api/tests/BufferCompositionTest.java | 52 ++++++++++--------- .../buffer/api/tests/BufferReadOnlyTest.java | 13 ++--- .../tests/BufferReferenceCountingTest.java | 52 ++++++++++--------- .../buffer/api/tests/BufferSendTest.java | 10 ++-- .../buffer/api/tests/BufferTestSupport.java | 7 +-- .../ByteToMessageDecoderTest.java | 1 - 11 files changed, 112 insertions(+), 86 deletions(-) diff --git a/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java b/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java index 5d27f76..5d0d8e1 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/CompositeBuffer.java @@ -16,6 +16,7 @@ package io.netty.buffer.api; import io.netty.buffer.api.internal.ResourceSupport; +import io.netty.buffer.api.internal.Statics; import java.nio.ByteBuffer; import java.nio.ByteOrder; @@ -1378,15 +1379,14 @@ public final class CompositeBuffer extends ResourceSupport) buf).isOwned(); + result &= Statics.isOwned((ResourceSupport) buf); } return result; } diff --git a/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java b/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java index 8c9631e..a2f7670 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java @@ -40,7 +40,8 @@ import java.nio.channels.ScatteringByteChannel; import java.nio.charset.Charset; import java.util.concurrent.atomic.AtomicReference; -import static io.netty.buffer.api.internal.Statics.asRS; +import static io.netty.buffer.api.internal.Statics.acquire; +import static io.netty.buffer.api.internal.Statics.isOwned; public final class ByteBufAdaptor extends ByteBuf { private final ByteBufAllocatorAdaptor alloc; @@ -82,7 +83,7 @@ public final class ByteBufAdaptor extends ByteBuf { try { buffer.ensureWritable(diff); } catch (IllegalStateException e) { - if (!asRS(buffer).isOwned()) { + if (!isOwned((ResourceSupport) buffer)) { throw new UnsupportedOperationException(e); } throw e; @@ -217,7 +218,7 @@ public final class ByteBufAdaptor extends ByteBuf { checkAccess(); if (writableBytes() < minWritableBytes) { try { - if (asRS(buffer).isOwned()) { + if (isOwned((ResourceSupport) buffer)) { // Good place. buffer.ensureWritable(minWritableBytes); } else { @@ -1628,7 +1629,7 @@ public final class ByteBufAdaptor extends ByteBuf { @Override public ByteBuf retain(int increment) { for (int i = 0; i < increment; i++) { - asRS(buffer).acquire(); + acquire((ResourceSupport) buffer); } return this; } @@ -1644,9 +1645,9 @@ public final class ByteBufAdaptor extends ByteBuf { } if (buffer instanceof ResourceSupport) { var rc = (ResourceSupport) buffer; - return rc.countBorrows(); + return Statics.countBorrows(rc); } - return asRS(buffer).isOwned()? 0 : 1; + return isOwned((ResourceSupport) buffer)? 0 : 1; } @Override diff --git a/buffer-api/src/main/java/io/netty/buffer/api/internal/ResourceSupport.java b/buffer-api/src/main/java/io/netty/buffer/api/internal/ResourceSupport.java index a7bb843..1d8dbc9 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/internal/ResourceSupport.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/internal/ResourceSupport.java @@ -38,6 +38,11 @@ public abstract class ResourceSupport, T extends ResourceS tracer = LifecycleTracer.get(); } + @SuppressWarnings("unchecked") + static T acquire(ResourceSupport obj) { + return (T) obj.acquire(); + } + /** * Increment the reference count. *

@@ -45,7 +50,7 @@ public abstract class ResourceSupport, T extends ResourceS * * @return This {@link Resource} instance. */ - public final I acquire() { + protected final I acquire() { if (acquires < 0) { throw attachTrace(new IllegalStateException("This resource is closed: " + this + '.')); } @@ -112,10 +117,18 @@ public abstract class ResourceSupport, T extends ResourceS "Cannot send() a reference counted object with " + countBorrows() + " borrows: " + this + '.'); } - public boolean isOwned() { + static boolean isOwned(ResourceSupport obj) { + return obj.isOwned(); + } + + protected boolean isOwned() { return acquires == 0; } + static int countBorrows(ResourceSupport obj) { + return obj.countBorrows(); + } + /** * Count the number of borrows of this object. * Note that even if the number of borrows is {@code 0}, this object might not be {@linkplain #isOwned() owned} @@ -123,7 +136,7 @@ public abstract class ResourceSupport, T extends ResourceS * * @return The number of borrows, if any, of this object. */ - public int countBorrows() { + protected int countBorrows() { return Math.max(acquires, 0); } diff --git a/buffer-api/src/main/java/io/netty/buffer/api/internal/Statics.java b/buffer-api/src/main/java/io/netty/buffer/api/internal/Statics.java index ee0258c..de58053 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/internal/Statics.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/internal/Statics.java @@ -165,14 +165,6 @@ public interface Statics { dest.position(destPos).put(bbslice(src, srcPos, length)); } - static & Buffer> T asRS(Buffer buf) { - if (!(buf instanceof ResourceSupport)) { - throw new IllegalArgumentException("Buffer instance is not an instance of ResourceSupport."); - } - //noinspection unchecked - return (T) buf; - } - static IllegalStateException bufferIsClosed() { return new IllegalStateException("This buffer is closed."); } @@ -180,4 +172,16 @@ public interface Statics { static IllegalStateException bufferIsReadOnly() { return new IllegalStateException("This buffer is read-only."); } + + static T acquire(ResourceSupport obj) { + return ResourceSupport.acquire(obj); + } + + static boolean isOwned(ResourceSupport obj) { + return ResourceSupport.isOwned(obj); + } + + static int countBorrows(ResourceSupport obj) { + return ResourceSupport.countBorrows(obj); + } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompactTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompactTest.java index 4899bb0..cc45689 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompactTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompactTest.java @@ -17,10 +17,11 @@ package io.netty.buffer.api.tests; import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; +import io.netty.buffer.api.internal.ResourceSupport; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; -import static io.netty.buffer.api.internal.Statics.asRS; +import static io.netty.buffer.api.internal.Statics.acquire; import static java.nio.ByteOrder.BIG_ENDIAN; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -55,7 +56,7 @@ public class BufferCompactTest extends BufferTestSupport { Buffer buf = allocator.allocate(8, BIG_ENDIAN)) { buf.writeLong(0x0102030405060708L); assertEquals((byte) 0x01, buf.readByte()); - try (var ignore = asRS(buf).acquire()) { + try (Buffer ignore = acquire((ResourceSupport) buf)) { assertThrows(IllegalStateException.class, () -> buf.compact()); assertEquals(1, buf.readerOffset()); } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java index a6d3715..f4ded93 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferCompositionTest.java @@ -19,13 +19,15 @@ import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; import io.netty.buffer.api.CompositeBuffer; import io.netty.buffer.api.Send; +import io.netty.buffer.api.internal.ResourceSupport; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; import java.nio.ByteOrder; -import static io.netty.buffer.api.internal.Statics.asRS; +import static io.netty.buffer.api.internal.Statics.acquire; +import static io.netty.buffer.api.internal.Statics.isOwned; import static java.nio.ByteOrder.BIG_ENDIAN; import static java.nio.ByteOrder.LITTLE_ENDIAN; import static org.assertj.core.api.Assertions.assertThat; @@ -56,7 +58,7 @@ public class BufferCompositionTest extends BufferTestSupport { allocator.allocate(8).send(), allocator.allocate(8).send())) { assertEquals(24, composite.capacity()); - assertTrue(asRS(composite).isOwned()); + assertTrue(isOwned((ResourceSupport) composite)); } } @@ -133,7 +135,7 @@ public class BufferCompositionTest extends BufferTestSupport { Buffer a = allocator.allocate(8); Buffer b = allocator.allocate(8); CompositeBuffer composed = CompositeBuffer.compose(allocator, a.send())) { - try (Buffer ignore = composed.acquire()) { + try (Buffer ignore = acquire(composed)) { var exc = assertThrows(IllegalStateException.class, () -> composed.extendWith(b.send())); assertThat(exc).hasMessageContaining("owned"); } @@ -166,13 +168,13 @@ public class BufferCompositionTest extends BufferTestSupport { try (BufferAllocator allocator = BufferAllocator.heap()) { Buffer a = allocator.allocate(1); CompositeBuffer composite = CompositeBuffer.compose(allocator, a.send()); - assertTrue(composite.isOwned()); + assertTrue(isOwned(composite)); assertThat(composite.capacity()).isOne(); assertThat(composite.countComponents()).isOne(); try (Buffer b = CompositeBuffer.compose(allocator)) { composite.extendWith(b.send()); } - assertTrue(composite.isOwned()); + assertTrue(isOwned(composite)); assertThat(composite.capacity()).isOne(); assertThat(composite.countComponents()).isOne(); } @@ -482,11 +484,11 @@ public class BufferCompositionTest extends BufferTestSupport { allocator.allocate(8).send(), allocator.allocate(8).send())) { try (CompositeBuffer split = composite.splitComponentsFloor(4)) { - assertTrue(split.isOwned()); + assertTrue(isOwned(split)); assertTrue(split.isAccessible()); assertThat(split.capacity()).isZero(); - assertTrue(composite.isOwned()); + assertTrue(isOwned(composite)); assertTrue(composite.isAccessible()); assertThat(composite.capacity()).isEqualTo(16); } @@ -500,11 +502,11 @@ public class BufferCompositionTest extends BufferTestSupport { allocator.allocate(8).send(), allocator.allocate(8).send())) { try (CompositeBuffer split = composite.splitComponentsFloor(7)) { - assertTrue(split.isOwned()); + assertTrue(isOwned(split)); assertTrue(split.isAccessible()); assertThat(split.capacity()).isZero(); - assertTrue(composite.isOwned()); + assertTrue(isOwned(composite)); assertTrue(composite.isAccessible()); assertThat(composite.capacity()).isEqualTo(16); } @@ -518,11 +520,11 @@ public class BufferCompositionTest extends BufferTestSupport { allocator.allocate(8).send(), allocator.allocate(8).send())) { try (CompositeBuffer split = composite.splitComponentsFloor(12)) { - assertTrue(split.isOwned()); + assertTrue(isOwned(split)); assertTrue(split.isAccessible()); assertThat(split.capacity()).isEqualTo(8); - assertTrue(composite.isOwned()); + assertTrue(isOwned(composite)); assertTrue(composite.isAccessible()); assertThat(composite.capacity()).isEqualTo(8); } @@ -536,11 +538,11 @@ public class BufferCompositionTest extends BufferTestSupport { allocator.allocate(8).send(), allocator.allocate(8).send())) { try (CompositeBuffer split = composite.splitComponentsFloor(8)) { - assertTrue(split.isOwned()); + assertTrue(isOwned(split)); assertTrue(split.isAccessible()); assertThat(split.capacity()).isEqualTo(8); - assertTrue(composite.isOwned()); + assertTrue(isOwned(composite)); assertTrue(composite.isAccessible()); assertThat(composite.capacity()).isEqualTo(8); } @@ -554,11 +556,11 @@ public class BufferCompositionTest extends BufferTestSupport { allocator.allocate(8).send(), allocator.allocate(8).send())) { try (CompositeBuffer split = composite.splitComponentsCeil(4)) { - assertTrue(split.isOwned()); + assertTrue(isOwned(split)); assertTrue(split.isAccessible()); assertThat(split.capacity()).isEqualTo(8); - assertTrue(composite.isOwned()); + assertTrue(isOwned(composite)); assertTrue(composite.isAccessible()); assertThat(composite.capacity()).isEqualTo(8); } @@ -572,11 +574,11 @@ public class BufferCompositionTest extends BufferTestSupport { allocator.allocate(8).send(), allocator.allocate(8).send())) { try (CompositeBuffer split = composite.splitComponentsCeil(7)) { - assertTrue(split.isOwned()); + assertTrue(isOwned(split)); assertTrue(split.isAccessible()); assertThat(split.capacity()).isEqualTo(8); - assertTrue(composite.isOwned()); + assertTrue(isOwned(composite)); assertTrue(composite.isAccessible()); assertThat(composite.capacity()).isEqualTo(8); } @@ -590,11 +592,11 @@ public class BufferCompositionTest extends BufferTestSupport { allocator.allocate(8).send(), allocator.allocate(8).send())) { try (CompositeBuffer split = composite.splitComponentsCeil(12)) { - assertTrue(split.isOwned()); + assertTrue(isOwned(split)); assertTrue(split.isAccessible()); assertThat(split.capacity()).isEqualTo(16); - assertTrue(composite.isOwned()); + assertTrue(isOwned(composite)); assertTrue(composite.isAccessible()); assertThat(composite.capacity()).isEqualTo(0); } @@ -606,11 +608,11 @@ public class BufferCompositionTest extends BufferTestSupport { allocator.allocate(8).send(), allocator.allocate(8).send())) { try (CompositeBuffer split = composite.splitComponentsCeil(12)) { - assertTrue(split.isOwned()); + assertTrue(isOwned(split)); assertTrue(split.isAccessible()); assertThat(split.capacity()).isEqualTo(16); - assertTrue(composite.isOwned()); + assertTrue(isOwned(composite)); assertTrue(composite.isAccessible()); assertThat(composite.capacity()).isEqualTo(8); } @@ -624,11 +626,11 @@ public class BufferCompositionTest extends BufferTestSupport { allocator.allocate(8).send(), allocator.allocate(8).send())) { try (CompositeBuffer split = composite.splitComponentsCeil(7)) { - assertTrue(split.isOwned()); + assertTrue(isOwned(split)); assertTrue(split.isAccessible()); assertThat(split.capacity()).isEqualTo(8); - assertTrue(composite.isOwned()); + assertTrue(isOwned(composite)); assertTrue(composite.isAccessible()); assertThat(composite.capacity()).isEqualTo(8); } @@ -642,11 +644,11 @@ public class BufferCompositionTest extends BufferTestSupport { allocator.allocate(8).send(), allocator.allocate(8).send())) { try (CompositeBuffer split = composite.splitComponentsCeil(0)) { - assertTrue(split.isOwned()); + assertTrue(isOwned(split)); assertTrue(split.isAccessible()); assertThat(split.capacity()).isZero(); - assertTrue(composite.isOwned()); + assertTrue(isOwned(composite)); assertTrue(composite.isAccessible()); assertThat(composite.capacity()).isEqualTo(16); } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java index 3ca1999..3d4bf01 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReadOnlyTest.java @@ -19,6 +19,7 @@ import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; import io.netty.buffer.api.CompositeBuffer; import io.netty.buffer.api.Send; +import io.netty.buffer.api.internal.ResourceSupport; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -26,7 +27,7 @@ import org.junit.jupiter.params.provider.MethodSource; import java.nio.ByteOrder; import java.util.function.Supplier; -import static io.netty.buffer.api.internal.Statics.asRS; +import static io.netty.buffer.api.internal.Statics.isOwned; import static java.nio.ByteOrder.BIG_ENDIAN; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -165,7 +166,7 @@ public class BufferReadOnlyTest extends BufferTestSupport { assertThat(buf.readerOffset()).isZero(); assertThat(buf.capacity()).isEqualTo(4); assertThat(buf.writerOffset()).isEqualTo(4); - assertTrue(asRS(buf).isOwned()); + assertTrue(isOwned((ResourceSupport) buf)); assertTrue(buf.isAccessible()); assertThat(buf.countComponents()).isOne(); assertEquals((byte) 1, buf.readByte()); @@ -191,14 +192,14 @@ public class BufferReadOnlyTest extends BufferTestSupport { Buffer b = a.split(8)) { assertTrue(a.readOnly()); assertTrue(b.readOnly()); - assertTrue(asRS(a).isOwned()); - assertTrue(asRS(b).isOwned()); + assertTrue(isOwned((ResourceSupport) a)); + assertTrue(isOwned((ResourceSupport) b)); assertThat(a.capacity()).isEqualTo(8); assertThat(b.capacity()).isEqualTo(8); try (Buffer c = b.copy()) { assertTrue(c.readOnly()); - assertTrue(asRS(c).isOwned()); - assertTrue(asRS(b).isOwned()); + assertTrue(isOwned((ResourceSupport) c)); + assertTrue(isOwned((ResourceSupport) b)); assertThat(c.capacity()).isEqualTo(8); } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java index 822507a..956795b 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferReferenceCountingTest.java @@ -18,6 +18,7 @@ package io.netty.buffer.api.tests; import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; import io.netty.buffer.api.CompositeBuffer; +import io.netty.buffer.api.internal.ResourceSupport; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -25,7 +26,8 @@ import org.junit.jupiter.params.provider.MethodSource; import java.util.concurrent.Future; import java.util.concurrent.ThreadLocalRandom; -import static io.netty.buffer.api.internal.Statics.asRS; +import static io.netty.buffer.api.internal.Statics.acquire; +import static io.netty.buffer.api.internal.Statics.isOwned; import static java.nio.ByteOrder.BIG_ENDIAN; import static java.nio.ByteOrder.LITTLE_ENDIAN; import static org.assertj.core.api.Assertions.assertThat; @@ -41,7 +43,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { Buffer buf = allocator.allocate(8)) { buf.writeByte((byte) 1); buf.writeByte((byte) 2); - try (Buffer inner = asRS(buf).acquire()) { + try (Buffer inner = acquire((ResourceSupport) buf)) { inner.writeByte((byte) 3); inner.writeByte((byte) 4); inner.writeByte((byte) 5); @@ -75,7 +77,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator()) { var buf = allocator.allocate(8); buf.close(); - assertThrows(IllegalStateException.class, () -> asRS(buf).acquire()); + assertThrows(IllegalStateException.class, () -> acquire((ResourceSupport) buf)); } } @@ -166,11 +168,11 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { try (Buffer copy = buf.copy()) { - assertTrue(asRS(buf).isOwned()); - assertTrue(asRS(copy).isOwned()); + assertTrue(isOwned((ResourceSupport) buf)); + assertTrue(isOwned((ResourceSupport) copy)); copy.send().discard(); } - assertTrue(asRS(buf).isOwned()); + assertTrue(isOwned((ResourceSupport) buf)); buf.send().discard(); } } @@ -181,11 +183,11 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { try (Buffer copy = buf.copy(0, 8)) { - assertTrue(asRS(buf).isOwned()); - assertTrue(asRS(copy).isOwned()); + assertTrue(isOwned((ResourceSupport) buf)); + assertTrue(isOwned((ResourceSupport) copy)); copy.send().discard(); } - assertTrue(asRS(buf).isOwned()); + assertTrue(isOwned((ResourceSupport) buf)); buf.send().discard(); } } @@ -230,11 +232,11 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { try (Buffer copy = buf.copy()) { - assertTrue(asRS(buf).isOwned()); + assertTrue(isOwned((ResourceSupport) buf)); copy.send().discard(); } // Verify that the copy is closed properly afterwards. - assertTrue(asRS(buf).isOwned()); + assertTrue(isOwned((ResourceSupport) buf)); buf.send().receive().close(); } } @@ -245,11 +247,11 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { try (Buffer copy = buf.copy(0, 8)) { - assertTrue(asRS(buf).isOwned()); + assertTrue(isOwned((ResourceSupport) buf)); copy.send().discard(); } // Verify that the copy is closed properly afterwards. - assertTrue(asRS(buf).isOwned()); + assertTrue(isOwned((ResourceSupport) buf)); } } @@ -260,7 +262,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { Buffer buf = allocator.allocate(8)) { assertThrows(IndexOutOfBoundsException.class, () -> buf.copy(-1, 1)); // Verify that the copy is closed properly afterwards. - assertTrue(asRS(buf).isOwned()); + assertTrue(isOwned((ResourceSupport) buf)); } } @@ -272,7 +274,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { assertThrows(IllegalArgumentException.class, () -> buf.copy(0, -1)); assertThrows(IllegalArgumentException.class, () -> buf.copy(2, -1)); // Verify that the copy is closed properly afterwards. - assertTrue(asRS(buf).isOwned()); + assertTrue(isOwned((ResourceSupport) buf)); } } @@ -285,7 +287,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { buf.copy(0, 8).close(); // This is still fine. assertThrows(IndexOutOfBoundsException.class, () -> buf.copy(1, 8)); // Verify that the copy is closed properly afterwards. - assertTrue(asRS(buf).isOwned()); + assertTrue(isOwned((ResourceSupport) buf)); } } @@ -296,7 +298,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { Buffer buf = allocator.allocate(8)) { buf.copy(0, 0).close(); // This is fine. // Verify that the copy is closed properly afterwards. - assertTrue(asRS(buf).isOwned()); + assertTrue(isOwned((ResourceSupport) buf)); } } @@ -307,13 +309,13 @@ public class BufferReferenceCountingTest extends BufferTestSupport { Buffer buf = allocator.allocate(8); buf.writeInt(42); try (Buffer copy = buf.copy()) { - assertTrue(asRS(copy).isOwned()); - assertTrue(asRS(buf).isOwned()); + assertTrue(isOwned((ResourceSupport) copy)); + assertTrue(isOwned((ResourceSupport) buf)); buf.close(); assertFalse(buf.isAccessible()); - assertTrue(asRS(copy).isOwned()); + assertTrue(isOwned((ResourceSupport) copy)); try (Buffer receive = copy.send().receive()) { - assertTrue(asRS(receive).isOwned()); + assertTrue(isOwned((ResourceSupport) receive)); assertFalse(copy.isAccessible()); } } @@ -378,7 +380,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { buf.writeInt(1); - try (Buffer acquired = asRS(buf).acquire()) { + try (Buffer acquired = acquire((ResourceSupport) buf)) { var exc = assertThrows(IllegalStateException.class, () -> acquired.split()); assertThat(exc).hasMessageContaining("owned"); } @@ -390,7 +392,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { public void splitOnOffsetOfNonOwnedBufferMustThrow(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { - try (Buffer acquired = asRS(buf).acquire()) { + try (Buffer acquired = acquire((ResourceSupport) buf)) { var exc = assertThrows(IllegalStateException.class, () -> acquired.split(4)); assertThat(exc).hasMessageContaining("owned"); } @@ -533,7 +535,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { buf.writeLong(0x0102030405060708L); try (Buffer copy = buf.copy()) { buf.close(); - assertTrue(asRS(copy).isOwned()); + assertTrue(isOwned((ResourceSupport) copy)); try (Buffer split = copy.split(4)) { split.reset().ensureWritable(Long.BYTES); copy.reset().ensureWritable(Long.BYTES); @@ -663,7 +665,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { buf.makeReadOnly(); - try (Buffer acquire = asRS(buf).acquire()) { + try (Buffer acquire = acquire((ResourceSupport) buf)) { assertTrue(acquire.readOnly()); } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferSendTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferSendTest.java index 50aef5c..7161a2c 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferSendTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferSendTest.java @@ -19,6 +19,7 @@ import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; import io.netty.buffer.api.BufferRef; import io.netty.buffer.api.Send; +import io.netty.buffer.api.internal.ResourceSupport; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.MethodSource; @@ -27,7 +28,8 @@ import java.util.concurrent.ArrayBlockingQueue; import java.util.concurrent.Future; import java.util.concurrent.SynchronousQueue; -import static io.netty.buffer.api.internal.Statics.asRS; +import static io.netty.buffer.api.internal.Statics.acquire; +import static io.netty.buffer.api.internal.Statics.isOwned; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -78,12 +80,12 @@ public class BufferSendTest extends BufferTestSupport { void sendMustThrowWhenBufIsAcquired(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { - try (Buffer ignored = asRS(buf).acquire()) { - assertFalse(asRS(buf).isOwned()); + try (Buffer ignored = acquire((ResourceSupport) buf)) { + assertFalse(isOwned((ResourceSupport) buf)); assertThrows(IllegalStateException.class, buf::send); } // Now send() should work again. - assertTrue(asRS(buf).isOwned()); + assertTrue(isOwned((ResourceSupport) buf)); buf.send().receive().close(); } } diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java index 850ff85..5e5cfa1 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java @@ -19,6 +19,7 @@ import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; import io.netty.buffer.api.CompositeBuffer; import io.netty.buffer.api.MemoryManagers; +import io.netty.buffer.api.internal.ResourceSupport; import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.BeforeAll; @@ -42,7 +43,7 @@ import java.util.stream.Collectors; import java.util.stream.Stream; import java.util.stream.Stream.Builder; -import static io.netty.buffer.api.internal.Statics.asRS; +import static io.netty.buffer.api.internal.Statics.acquire; import static java.nio.ByteOrder.BIG_ENDIAN; import static java.nio.ByteOrder.LITTLE_ENDIAN; import static org.assertj.core.api.Assertions.assertThat; @@ -335,8 +336,8 @@ public abstract class BufferTestSupport { } assertThrows(IllegalStateException.class, () -> buf.split()); - assertThrows(IllegalStateException.class, () -> asRS(buf).send()); - assertThrows(IllegalStateException.class, () -> asRS(buf).acquire()); + assertThrows(IllegalStateException.class, () -> buf.send()); + assertThrows(IllegalStateException.class, () -> acquire((ResourceSupport) buf)); assertThrows(IllegalStateException.class, () -> buf.copy()); assertThrows(IllegalStateException.class, () -> buf.openCursor()); assertThrows(IllegalStateException.class, () -> buf.openCursor(0, 0)); diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java index ee16786..01a61b2 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java @@ -30,7 +30,6 @@ import java.util.concurrent.ThreadLocalRandom; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; -import static io.netty.buffer.api.internal.Statics.asRS; import static io.netty.buffer.api.tests.BufferTestSupport.assertEquals; import static io.netty.buffer.api.CompositeBuffer.compose; import static io.netty.buffer.api.tests.BufferTestSupport.assertReadableEquals; From b1b2c983f889537122b4378c8adc3666a3854155 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Thu, 27 May 2021 17:38:38 +0200 Subject: [PATCH 07/12] Do less aggressive test sampling when running tests locally Instead of filtering out 95% of test samples, now only filter out 85%. --- .../java/io/netty/buffer/api/tests/BufferTestSupport.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java index 5e5cfa1..b6acd29 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/BufferTestSupport.java @@ -94,8 +94,8 @@ public abstract class BufferTestSupport { if ("nosample".equalsIgnoreCase(sampleSetting)) { return fixture -> true; } - // Filter out 95% of tests. - return filterOfTheDay(5); + // Filter out 85% of tests. + return filterOfTheDay(15); } protected static Predicate filterOfTheDay(int percentage) { From 050db15e079638d8922d66e55562dbada885b890 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Thu, 27 May 2021 17:41:40 +0200 Subject: [PATCH 08/12] Fix bug where `copy` with over-sized length threw a wrong exception --- .../java/io/netty/buffer/api/bytebuffer/NioBuffer.java | 7 +------ .../main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java | 4 ---- .../main/java/io/netty/buffer/api/memseg/MemSegBuffer.java | 6 ------ 3 files changed, 1 insertion(+), 16 deletions(-) diff --git a/buffer-api/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java b/buffer-api/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java index 87360fa..8fa5a7e 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java @@ -189,12 +189,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re @Override public Buffer copy(int offset, int length) { - if (length < 0) { - throw new IllegalArgumentException("Length cannot be negative: " + length + '.'); - } - if (!isAccessible()) { - throw new IllegalStateException("This buffer is closed: " + this + '.'); - } + checkGet(offset, length); int allocSize = Math.max(length, 1); // Allocators don't support allocating zero-sized buffers. AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, allocSize); ByteBuffer base = memory.memory(); diff --git a/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java b/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java index a5487fa..8f875f0 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java @@ -166,11 +166,7 @@ class UnsafeBuffer extends ResourceSupport implements Buff @Override public Buffer copy(int offset, int length) { - if (length < 0) { - throw new IllegalArgumentException("Length cannot be negative: " + length + '.'); - } checkGet(offset, length); - int allocSize = Math.max(length, 1); // Allocators don't support allocating zero-sized buffers. AllocatorControl.UntetheredMemory memory = control.allocateUntethered(this, allocSize); UnsafeMemory unsafeMemory = memory.memory(); diff --git a/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java b/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java index 8ca6939..e8af309 100644 --- a/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java +++ b/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java @@ -299,12 +299,6 @@ class MemSegBuffer extends ResourceSupport implements Buff @Override public Buffer copy(int offset, int length) { checkGet(offset, length); - if (length < 0) { - throw new IllegalArgumentException("Length cannot be negative: " + length + '.'); - } - if (!isAccessible()) { - throw new IllegalStateException("This buffer is closed: " + this + '.'); - } if (length == 0) { // Special case zero-length segments, since allocators don't support allocating empty buffers. From b2bf0029beede37598d0ed2e4e874ecdc296db94 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Fri, 28 May 2021 10:58:37 +0200 Subject: [PATCH 09/12] Fix adaptor tests --- .../io/netty/buffer/api/adaptor/ByteBufAdaptor.java | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java b/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java index a2f7670..fcb545b 100644 --- a/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java +++ b/buffer-api/src/main/java/io/netty/buffer/api/adaptor/ByteBufAdaptor.java @@ -1435,7 +1435,8 @@ public final class ByteBufAdaptor extends ByteBuf { @Override public ByteBuf retainedDuplicate() { - return new Slice(unwrap().retainedDuplicate(), indexAdjustment, lengthAdjustment); + return new Slice(unwrap().retainedDuplicate(), indexAdjustment, lengthAdjustment) + .setIndex(readerIndex(), writerIndex()); } @Override @@ -1453,7 +1454,8 @@ public final class ByteBufAdaptor extends ByteBuf { @Override public ByteBuf duplicate() { ((ByteBufAdaptor) unwrap()).checkAccess(); - return new Duplicate((ByteBufAdaptor) unwrap()); + return new Duplicate((ByteBufAdaptor) unwrap()) + .setIndex(readerIndex(), writerIndex()); } @Override @@ -1476,7 +1478,10 @@ public final class ByteBufAdaptor extends ByteBuf { @Override public ByteBuf retainedDuplicate() { - return retainedSlice(0, capacity()).setIndex(readerIndex(), writerIndex()); + checkAccess(); + retain(); + Duplicate duplicatedByteBuf = new Duplicate(this); + return duplicatedByteBuf.setIndex(readerIndex(), writerIndex()); } @Override From a6b81c89ef5f6f8f1eb038ef257124987aca8390 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Fri, 28 May 2021 12:23:16 +0200 Subject: [PATCH 10/12] Fix test failures in ByteToMessageDecoderTest --- .../ByteToMessageDecoderTest.java | 48 ++++++++++++++----- 1 file changed, 36 insertions(+), 12 deletions(-) diff --git a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java index 01a61b2..782d9a2 100644 --- a/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java +++ b/buffer-tests/src/test/java/io/netty/buffer/api/tests/examples/bytetomessagedecoder/ByteToMessageDecoderTest.java @@ -61,20 +61,44 @@ public class ByteToMessageDecoderTest { } }); - try (Buffer buf = BufferAllocator.heap().allocate(4, BIG_ENDIAN).writeInt(0x01020304)) { - channel.writeInbound(buf); - try (Buffer b = channel.readInbound()) { - assertEquals(3, b.readableBytes()); - assertEquals(0x02, b.readByte()); - assertEquals(0x03, b.readByte()); - assertEquals(0x04, b.readByte()); - } + channel.writeInbound(BufferAllocator.heap().allocate(4, BIG_ENDIAN).writeInt(0x01020304)); + try (Buffer b = channel.readInbound()) { + assertEquals(3, b.readableBytes()); + assertEquals(0x02, b.readByte()); + assertEquals(0x03, b.readByte()); + assertEquals(0x04, b.readByte()); } } @Test public void testRemoveItselfWriteBuffer() { - final Buffer buf = BufferAllocator.heap().allocate(5, BIG_ENDIAN).writeInt(0x01020304); + try (Buffer buf = BufferAllocator.heap().allocate(5, BIG_ENDIAN).writeInt(0x01020304)) { + EmbeddedChannel channel = new EmbeddedChannel(new ByteToMessageDecoder() { + private boolean removed; + + @Override + protected void decode(ChannelHandlerContext ctx, Buffer in) { + assertFalse(removed); + in.readByte(); + ctx.pipeline().remove(this); + + // This should not let it keep call decode + buf.writeByte((byte) 0x05); + removed = true; + } + }); + + channel.writeInbound(buf.copy()); + try (Buffer expected = BufferAllocator.heap().allocate(3, BIG_ENDIAN).writeShort((short) 0x0203).writeByte((byte) 0x04); + Buffer actual = channel.readInbound()) { + assertReadableEquals(expected, actual); + } + } + } + + @Test + public void testRemoveItselfWriteBuffer2() { + Buffer buf = BufferAllocator.heap().allocate(5, BIG_ENDIAN).writeInt(0x01020304); EmbeddedChannel channel = new EmbeddedChannel(new ByteToMessageDecoder() { private boolean removed; @@ -91,7 +115,7 @@ public class ByteToMessageDecoderTest { }); channel.writeInbound(buf); - try (Buffer expected = BufferAllocator.heap().allocate(3, BIG_ENDIAN).writeShort((short) 0x0203).writeByte((byte) 0x04); + try (Buffer expected = BufferAllocator.heap().allocate(4, BIG_ENDIAN).writeInt(0x02030405); Buffer actual = channel.readInbound()) { assertReadableEquals(expected, actual); } @@ -304,8 +328,8 @@ public class ByteToMessageDecoderTest { }); byte[] bytes = new byte[1024]; ThreadLocalRandom.current().nextBytes(bytes); - try (Buffer buf = BufferAllocator.heap().allocate(bytes.length, BIG_ENDIAN).writeBytes(bytes); - Buffer part1 = buf.copy(0, bytes.length - 1); + Buffer buf = BufferAllocator.heap().allocate(bytes.length, BIG_ENDIAN).writeBytes(bytes); + try (Buffer part1 = buf.copy(0, bytes.length - 1); Buffer part2 = buf.copy(bytes.length - 1, 1)) { assertTrue(channel.writeInbound(buf)); try (Buffer actual = channel.readInbound()) { From 0aa2853cf33d3361de7062a3a3c6e1321fb80494 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Fri, 28 May 2021 13:54:16 +0200 Subject: [PATCH 11/12] Fix a bug in MemSegBuffer --- .../src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java b/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java index e8af309..07b6b2f 100644 --- a/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java +++ b/buffer-memseg/src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java @@ -299,6 +299,9 @@ class MemSegBuffer extends ResourceSupport implements Buff @Override public Buffer copy(int offset, int length) { checkGet(offset, length); + if (length < 0) { + throw new IllegalArgumentException("Length cannot be negative: " + length + '.'); + } if (length == 0) { // Special case zero-length segments, since allocators don't support allocating empty buffers. From d4a54d3828193ce22f4e4f2b2c31f23d809dd4fe Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Fri, 28 May 2021 13:54:55 +0200 Subject: [PATCH 12/12] Fix typo --- RATIONALE.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/RATIONALE.adoc b/RATIONALE.adoc index d212391..710e8b9 100644 --- a/RATIONALE.adoc +++ b/RATIONALE.adoc @@ -76,7 +76,7 @@ In this section we’ll outline the major changes, and most prominent points of Buffers are now `AutoCloseable` and can be used in try-with-resources clauses. Every allocation and acquire call on a buffer (any `Resource` object, really) must be paired with a `close()`, and every `receive()` call on a `Send` must also be paired with a `close()`. -While referene counting is a useful thing for tracking resource life-cycles internally, it is not itself exposed in the public API. +While reference counting is a useful thing for tracking resource life-cycles internally, it is not itself exposed in the public API. Instead, the public API effectively has a boolean open/closed state. This simplifies the API a great deal; buffers are created, and in the end they are closed. The code in between needs to be arranged such that it just avoids ever holding on to buffers that might be closed.