Fix numerous review comments

- Mostly javadocs have been fixed or added.
- A couple of small code changes in ByteBufAdaptor to better cope with unlikely exceptions.
This commit is contained in:
Chris Vest 2021-05-28 16:23:35 +02:00
parent 78b30a1b37
commit 53a8d71c64
8 changed files with 81 additions and 10 deletions

View File

@ -76,7 +76,7 @@ import java.nio.ByteOrder;
*
* <h3 name="split">Splitting buffers</h3>
*
* The {@link #split()} method break a buffer into two.
* The {@link #split()} method breaks 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.
* <p>
@ -86,7 +86,7 @@ import java.nio.ByteOrder;
* 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.
* buffer, or you will have to make a copy of the region.
*
* <h3>Buffers as constants</h3>
*
@ -377,7 +377,7 @@ public interface Buffer extends Resource<Buffer>, 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 a bad state, or is {@linkplain #readOnly() read-only}.
* @throws IllegalStateException if this buffer is in a bad state, or is {@linkplain #readOnly() read-only}.
*/
default void ensureWritable(int size) {
ensureWritable(size, 1, true);
@ -418,7 +418,7 @@ public interface Buffer extends Resource<Buffer>, 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 a bad state, or is {@linkplain #readOnly() read-only}.
* @throws IllegalStateException if this buffer is in a bad state, or is {@linkplain #readOnly() read-only}.
*/
void ensureWritable(int size, int minimumGrowth, boolean allowCompaction);
@ -553,7 +553,7 @@ public interface Buffer extends Resource<Buffer>, BufferAccessors {
/**
* Discards the read bytes, and moves the buffer contents to the beginning of the buffer.
*
* @throws IllegalStateException if this buffer is not in a bad state, or is {@linkplain #readOnly() read-only}.
* @throws IllegalStateException if this buffer is in a bad state, or is {@linkplain #readOnly() read-only}.
*/
void compact();

View File

@ -97,8 +97,9 @@ public abstract class BufferHolder<T extends BufferHolder<T>> implements Resourc
* @param send The new {@link Buffer} instance that is replacing the currently held buffer.
*/
protected final void replaceBuffer(Send<Buffer> send) {
Buffer received = send.receive();
buf.close();
buf = send.receive();
buf = received;
}
/**
@ -114,7 +115,8 @@ public abstract class BufferHolder<T extends BufferHolder<T>> implements Resourc
* @param send The {@link Send} with the new {@link Buffer} instance that is replacing the currently held buffer.
*/
protected final void replaceBufferVolatile(Send<Buffer> send) {
var prev = (Buffer) BUF.getAndSet(this, send.receive());
Buffer received = send.receive();
var prev = (Buffer) BUF.getAndSet(this, received);
prev.close();
}

View File

@ -54,6 +54,8 @@ public interface MemoryManagers {
/**
* Get a lazy-loading stream of all available memory managers.
* <p>
* Note: All available {@link MemoryManagers} instances are service loaded and instantiated on every call.
*
* @return A stream of providers of memory managers instances.
*/
@ -64,6 +66,9 @@ public interface MemoryManagers {
/**
* Find a {@link MemoryManagers} implementation by its {@linkplain #implementationName() implementation name}.
* <p>
* Note: All available {@link MemoryManagers} instances are service loaded and instantiated every time this
* method is called.
*
* @param implementationName The named implementation to look for.
* @return A {@link MemoryManagers} implementation, if any was found.

View File

@ -1,3 +1,18 @@
/*
* Copyright 2021 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;
/**

View File

@ -1419,8 +1419,9 @@ public final class ByteBufAdaptor extends ByteBuf {
@Override
public ByteBuf retainedSlice(int index, int length) {
checkAccess();
Slice slice = new Slice(this, index, length);
retain();
return new Slice(this, index, length);
return slice;
}
private static final class Slice extends SlicedByteBuf {
@ -1678,7 +1679,11 @@ public final class ByteBufAdaptor extends ByteBuf {
@Override
public boolean release(int decrement) {
for (int i = 0; i < decrement; i++) {
buffer.close();
try {
buffer.close();
} catch (IllegalStateException e) {
throw new IllegalReferenceCountException(e);
}
}
return !buffer.isAccessible();
}

View File

@ -38,6 +38,17 @@ public abstract class ResourceSupport<I extends Resource<I>, T extends ResourceS
tracer = LifecycleTracer.get();
}
/**
* Encapsulation bypass for calling {@link #acquire()} on the given object.
* <p>
* Note: this {@code acquire} method does not check the type of the return value from acquire at compile time.
* The type is instead checked at runtime, and will cause a {@link ClassCastException} to be thrown if done
* incorrectly.
*
* @param obj The object we wish to acquire (increment reference count) on.
* @param <T> The type of the acquired object, given by target-typing.
* @return The acquired object.
*/
@SuppressWarnings("unchecked")
static <T> T acquire(ResourceSupport<?, ?> obj) {
return (T) obj.acquire();
@ -103,6 +114,13 @@ public abstract class ResourceSupport<I extends Resource<I>, T extends ResourceS
return new TransferSend<I, T>(owned, drop, getClass());
}
/**
* Attach a trace of the life-cycle of this object as suppressed exceptions to the given throwable.
*
* @param throwable The throwable to attach a life-cycle trace to.
* @param <E> The concrete exception type.
* @return The given exception, which can then be thrown.
*/
protected <E extends Throwable> E attachTrace(E throwable) {
return tracer.attachTrace(throwable);
}
@ -117,14 +135,34 @@ public abstract class ResourceSupport<I extends Resource<I>, T extends ResourceS
"Cannot send() a reference counted object with " + countBorrows() + " borrows: " + this + '.');
}
/**
* Encapsulation bypass to call {@link #isOwned()} on the given object.
*
* @param obj The object to query the ownership state on.
* @return {@code true} if the given object is owned, otherwise {@code false}.
*/
static boolean isOwned(ResourceSupport<?, ?> obj) {
return obj.isOwned();
}
/**
* Query if this object is in an "owned" state, which means no other references have been
* {@linkplain #acquire() acquired} to it.
*
* This would usually be the case, since there are no public methods for acquiring references to these objects.
*
* @return {@code true} if this object is in an owned state, otherwise {@code false}.
*/
protected boolean isOwned() {
return acquires == 0;
}
/**
* Encapsulation bypass to call {@link #countBorrows()} on the given object.
*
* @param obj The object to count borrows on.
* @return The number of borrows, or outstanding {@linkplain #acquire() acquires}, if any, of the given object.
*/
static int countBorrows(ResourceSupport<?, ?> obj) {
return obj.countBorrows();
}

View File

@ -54,6 +54,7 @@ public interface Statics {
}
}
@SuppressWarnings("JavaLangInvokeHandleSignature")
static MethodHandle getByteBufferPutOffsetsMethodHandle() {
try {
Lookup lookup = MethodHandles.lookup();
@ -64,7 +65,7 @@ public interface Statics {
}
}
@SuppressWarnings("unchecked")
@SuppressWarnings({"unchecked", "unused"})
static <T extends Buffer> Drop<T> noOpDrop() {
return (Drop<T>) NO_OP_DROP;
}

View File

@ -3458,6 +3458,11 @@ public abstract class AbstractByteBufTest {
assertThrows(IllegalReferenceCountException.class, () -> releasedBuffer().retainedDuplicate());
}
@Test
public void testReleaseAfterRelease() {
assertThrows(IllegalReferenceCountException.class, () -> releasedBuffer().release());
}
private static void assertDuplicateFailAfterRelease(ByteBuf... bufs) {
for (ByteBuf buf : bufs) {
if (buf.refCnt() > 0) {