From c840a41e319a792c69d60b840103d7a27f7e9d0e Mon Sep 17 00:00:00 2001 From: Riley Park Date: Fri, 28 May 2021 06:13:16 -0700 Subject: [PATCH 1/6] Move to custom exception types --- .../main/java/io/netty/buffer/api/Buffer.java | 16 ++- .../buffer/api/BufferClosedException.java | 27 ++++ .../buffer/api/BufferReadOnlyException.java | 27 ++++ .../io/netty/buffer/api/CompositeBuffer.java | 24 ++-- .../buffer/api/adaptor/ByteBufAdaptor.java | 101 +++++++------- .../buffer/api/bytebuffer/NioBuffer.java | 68 +++++---- .../buffer/api/internal/ResourceSupport.java | 4 +- .../io/netty/buffer/api/internal/Statics.java | 10 +- .../netty/buffer/api/unsafe/UnsafeBuffer.java | 26 ++-- .../netty/buffer/api/memseg/MemSegBuffer.java | 25 ++-- .../tests/BufferComponentIterationTest.java | 8 +- .../api/tests/BufferCompositionTest.java | 3 +- .../buffer/api/tests/BufferReadOnlyTest.java | 23 +-- .../netty/buffer/api/tests/BufferRefTest.java | 40 +----- .../tests/BufferReferenceCountingTest.java | 3 +- .../buffer/api/tests/BufferTestSupport.java | 132 +++++++++--------- .../io/netty/buffer/api/tests/ScopeTest.java | 5 + 17 files changed, 307 insertions(+), 235 deletions(-) create mode 100644 buffer-api/src/main/java/io/netty/buffer/api/BufferClosedException.java create mode 100644 buffer-api/src/main/java/io/netty/buffer/api/BufferReadOnlyException.java 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 88cba0b..665a986 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 @@ -149,7 +149,8 @@ public interface Buffer extends Resource, BufferAccessors { * @return This Buffer. * @throws IndexOutOfBoundsException if the specified {@code offset} is less than the current * {@link #readerOffset()} or greater than {@link #capacity()}. - * @throws IllegalStateException if this buffer is {@linkplain #readOnly() read-only}. + * @throws BufferClosedException if this buffer is closed. + * @throws BufferReadOnlyException if this buffer is {@linkplain #readOnly() read-only}. */ Buffer writerOffset(int offset); @@ -174,7 +175,7 @@ public interface Buffer extends Resource, BufferAccessors { * * @param value The byte value to write at every position in the buffer. * @return This Buffer. - * @throws IllegalStateException if this buffer is {@linkplain #readOnly() read-only}. + * @throws BufferReadOnlyException if this buffer is {@linkplain #readOnly() read-only}. */ Buffer fill(byte value); @@ -377,7 +378,9 @@ public interface Buffer extends Resource, BufferAccessors { * {@code false}. * * @param size The requested number of bytes of space that should be available for writing. - * @throws IllegalStateException if this buffer is in a bad state, or is {@linkplain #readOnly() read-only}. + * @throws IllegalStateException if this buffer is in a bad state. + * @throws BufferClosedException if this buffer is closed. + * @throws BufferReadOnlyException if this buffer is {@linkplain #readOnly() read-only}. */ default void ensureWritable(int size) { ensureWritable(size, 1, true); @@ -418,7 +421,9 @@ public interface Buffer extends Resource, 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 in a bad state, or is {@linkplain #readOnly() read-only}. + * @throws BufferReadOnlyException if this buffer is {@linkplain #readOnly() read-only}. + * @throws IllegalArgumentException if {@code size} or {@code minimumGrowth} are negative. + * @throws IllegalStateException if this buffer is in a bad state. */ void ensureWritable(int size, int minimumGrowth, boolean allowCompaction); @@ -553,7 +558,8 @@ public interface Buffer extends Resource, BufferAccessors { /** * Discards the read bytes, and moves the buffer contents to the beginning of the buffer. * - * @throws IllegalStateException if this buffer is in a bad state, or is {@linkplain #readOnly() read-only}. + * @throws BufferReadOnlyException if this buffer is {@linkplain #readOnly() read-only}. + * @throws IllegalStateException if this buffer is in a bad state. */ void compact(); diff --git a/buffer-api/src/main/java/io/netty/buffer/api/BufferClosedException.java b/buffer-api/src/main/java/io/netty/buffer/api/BufferClosedException.java new file mode 100644 index 0000000..a47058e --- /dev/null +++ b/buffer-api/src/main/java/io/netty/buffer/api/BufferClosedException.java @@ -0,0 +1,27 @@ +/* + * 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; + +/** + * An exception thrown when an operation is attempted on a {@link Buffer} when it has been closed. + */ +public final class BufferClosedException extends UnsupportedOperationException { + private static final long serialVersionUID = 85913332711192868L; + + public BufferClosedException(final String message) { + super(message); + } +} diff --git a/buffer-api/src/main/java/io/netty/buffer/api/BufferReadOnlyException.java b/buffer-api/src/main/java/io/netty/buffer/api/BufferReadOnlyException.java new file mode 100644 index 0000000..aaf712a --- /dev/null +++ b/buffer-api/src/main/java/io/netty/buffer/api/BufferReadOnlyException.java @@ -0,0 +1,27 @@ +/* + * 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; + +/** + * An exception thrown when an operation is attempted on a {@linkplain Buffer#readOnly() read-only} {@link Buffer}. + */ +public final class BufferReadOnlyException extends UnsupportedOperationException { + private static final long serialVersionUID = 4855825594125231593L; + + public BufferReadOnlyException(final String message) { + super(message); + } +} 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 d9746e3..76713ac 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 @@ -27,6 +27,9 @@ import java.util.Objects; import java.util.Set; import java.util.stream.Stream; +import static io.netty.buffer.api.internal.Statics.bufferIsClosed; +import static io.netty.buffer.api.internal.Statics.bufferIsReadOnly; + /** * The {@code CompositeBuffer} is a concrete {@link Buffer} implementation that make a number of other buffers appear * as one. A composite buffer behaves the same as a normal, non-composite buffer in every way, so you normally don't @@ -332,6 +335,11 @@ public final class CompositeBuffer extends ResourceSupport= size) { // We already have enough space. @@ -975,7 +983,7 @@ public final class CompositeBuffer extends ResourceSupport implements Buffer, Re return "Buffer[roff:" + roff + ", woff:" + woff + ", cap:" + rmem.capacity() + ']'; } + @Override + protected RuntimeException createResourceClosedException() { + return bufferIsClosed(this); + } + @Override public Buffer order(ByteOrder order) { rmem.order(order); @@ -170,7 +176,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re int capacity = capacity(); checkSet(0, capacity); if (rmem == CLOSED_BUFFER) { - throw bufferIsClosed(); + throw bufferIsClosed(this); } for (int i = 0; i < capacity; i++) { wmem.put(i, value); @@ -218,7 +224,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re @Override public void copyInto(int srcPos, ByteBuffer dest, int destPos, int length) { if (rmem == CLOSED_BUFFER) { - throw bufferIsClosed(); + throw bufferIsClosed(this); } if (srcPos < 0) { throw new IllegalArgumentException("The srcPos cannot be negative: " + srcPos + '.'); @@ -254,7 +260,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re @Override public ByteCursor openCursor(int fromOffset, int length) { if (rmem == CLOSED_BUFFER) { - throw bufferIsClosed(); + throw bufferIsClosed(this); } if (fromOffset < 0) { throw new IllegalArgumentException("The fromOffset cannot be negative: " + fromOffset + '.'); @@ -319,7 +325,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re @Override public ByteCursor openReverseCursor(int fromOffset, int length) { if (rmem == CLOSED_BUFFER) { - throw bufferIsClosed(); + throw bufferIsClosed(this); } if (fromOffset < 0) { throw new IllegalArgumentException("The fromOffset cannot be negative: " + fromOffset + '.'); @@ -397,7 +403,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re throw new IllegalArgumentException("The minimum growth cannot be negative: " + minimumGrowth + '.'); } if (rmem != wmem) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } if (writableBytes() >= size) { // We already have enough space. @@ -487,7 +493,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re throw attachTrace(new IllegalStateException("Buffer must be owned in order to compact.")); } if (readOnly()) { - throw new IllegalStateException("Buffer must be writable in order to compact, but was read-only."); + throw new BufferReadOnlyException("Buffer must be writable in order to compact, but was read-only."); } if (roff == 0) { return; @@ -622,7 +628,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -634,7 +640,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -647,7 +653,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -659,7 +665,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -686,7 +692,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -698,7 +704,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -739,7 +745,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -751,7 +757,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -764,7 +770,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -776,7 +782,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -931,7 +937,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -943,7 +949,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, this.woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -956,7 +962,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -968,7 +974,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, this.woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -995,7 +1001,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -1007,7 +1013,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -1034,7 +1040,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -1046,7 +1052,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -1073,7 +1079,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } @@ -1085,7 +1091,7 @@ class NioBuffer extends ResourceSupport implements Buffer, Re } catch (IndexOutOfBoundsException e) { throw checkWriteState(e, woff); } catch (ReadOnlyBufferException e) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } } // @@ -1160,10 +1166,10 @@ class NioBuffer extends ResourceSupport implements Buffer, Re private RuntimeException checkWriteState(IndexOutOfBoundsException ioobe, int offset) { if (rmem == CLOSED_BUFFER) { - return bufferIsClosed(); + return bufferIsClosed(this); } if (wmem != rmem) { - return bufferIsReadOnly(); + return bufferIsReadOnly(this); } IndexOutOfBoundsException exception = outOfBounds(offset); @@ -1173,17 +1179,17 @@ class NioBuffer extends ResourceSupport implements Buffer, Re private RuntimeException readAccessCheckException(int index) { if (rmem == CLOSED_BUFFER) { - throw bufferIsClosed(); + throw bufferIsClosed(this); } return outOfBounds(index); } private RuntimeException writeAccessCheckException(int index) { if (rmem == CLOSED_BUFFER) { - throw bufferIsClosed(); + throw bufferIsClosed(this); } if (wmem != rmem) { - return bufferIsReadOnly(); + return bufferIsReadOnly(this); } return outOfBounds(index); } 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 1af1bc4..139512f 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 @@ -63,7 +63,7 @@ public abstract class ResourceSupport, T extends ResourceS */ protected final I acquire() { if (acquires < 0) { - throw attachTrace(new IllegalStateException("This resource is closed: " + this + '.')); + throw attachTrace(createResourceClosedException()); } if (acquires == Integer.MAX_VALUE) { throw new IllegalStateException("Cannot acquire more references; counter would overflow."); @@ -73,6 +73,8 @@ public abstract class ResourceSupport, T extends ResourceS return self(); } + protected abstract RuntimeException createResourceClosedException(); + /** * Decrement the reference count, and despose of the resource if the last reference is closed. *

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 f9e9a11..ab01fc2 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 @@ -16,6 +16,8 @@ package io.netty.buffer.api.internal; import io.netty.buffer.api.Buffer; +import io.netty.buffer.api.BufferClosedException; +import io.netty.buffer.api.BufferReadOnlyException; import io.netty.buffer.api.Drop; import java.lang.invoke.MethodHandle; @@ -166,12 +168,12 @@ public interface Statics { dest.position(destPos).put(bbslice(src, srcPos, length)); } - static IllegalStateException bufferIsClosed() { - return new IllegalStateException("This buffer is closed."); + static BufferClosedException bufferIsClosed(Buffer buffer) { + return new BufferClosedException("This buffer is closed: " + buffer); } - static IllegalStateException bufferIsReadOnly() { - return new IllegalStateException("This buffer is read-only."); + static BufferReadOnlyException bufferIsReadOnly(Buffer buffer) { + return new BufferReadOnlyException("This buffer is read-only: " + buffer); } static T acquire(ResourceSupport obj) { 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 78353ab..d97d750 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 @@ -19,6 +19,7 @@ import io.netty.buffer.ByteBuf; import io.netty.buffer.api.BufferAllocator; import io.netty.buffer.api.AllocatorControl; import io.netty.buffer.api.Buffer; +import io.netty.buffer.api.BufferReadOnlyException; import io.netty.buffer.api.ByteCursor; import io.netty.buffer.api.Drop; import io.netty.buffer.api.Owned; @@ -98,6 +99,11 @@ class UnsafeBuffer extends ResourceSupport implements Buff return "Buffer[roff:" + roff + ", woff:" + woff + ", cap:" + rsize + ']'; } + @Override + protected RuntimeException createResourceClosedException() { + return bufferIsClosed(this); + } + @Override public Buffer order(ByteOrder order) { this.order = order; @@ -143,7 +149,7 @@ class UnsafeBuffer extends ResourceSupport implements Buff public Buffer fill(byte value) { checkSet(0, capacity()); if (rsize == CLOSED_SIZE) { - throw bufferIsClosed(); + throw bufferIsClosed(this); } try { PlatformDependent.setMemory(base, address, rsize, value); @@ -220,7 +226,7 @@ class UnsafeBuffer extends ResourceSupport implements Buff private void checkCopyIntoArgs(int srcPos, int length, int destPos, int destLength) { if (rsize == CLOSED_SIZE) { - throw bufferIsClosed(); + throw bufferIsClosed(this); } if (srcPos < 0) { throw new IllegalArgumentException("The srcPos cannot be negative: " + srcPos + '.'); @@ -245,7 +251,7 @@ class UnsafeBuffer extends ResourceSupport implements Buff public void copyInto(int srcPos, Buffer dest, int destPos, int length) { checkCopyIntoArgs(srcPos, length, destPos, dest.capacity()); if (dest.readOnly()) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } long nativeAddress = dest.nativeAddress(); try { @@ -272,7 +278,7 @@ class UnsafeBuffer extends ResourceSupport implements Buff @Override public ByteCursor openCursor(int fromOffset, int length) { if (rsize == CLOSED_SIZE) { - throw bufferIsClosed(); + throw bufferIsClosed(this); } if (fromOffset < 0) { throw new IllegalArgumentException("The fromOffset cannot be negative: " + fromOffset + '.'); @@ -347,7 +353,7 @@ class UnsafeBuffer extends ResourceSupport implements Buff @Override public ByteCursor openReverseCursor(int fromOffset, int length) { if (rsize == CLOSED_SIZE) { - throw bufferIsClosed(); + throw bufferIsClosed(this); } if (fromOffset < 0) { throw new IllegalArgumentException("The fromOffset cannot be negative: " + fromOffset + '.'); @@ -436,7 +442,7 @@ class UnsafeBuffer extends ResourceSupport implements Buff throw new IllegalArgumentException("The minimum growth cannot be negative: " + minimumGrowth + '.'); } if (rsize != wsize) { - throw bufferIsReadOnly(); + throw bufferIsReadOnly(this); } if (writableBytes() >= size) { // We already have enough space. @@ -533,7 +539,7 @@ class UnsafeBuffer extends ResourceSupport implements Buff throw attachTrace(new IllegalStateException("Buffer must be owned in order to compact.")); } if (readOnly()) { - throw new IllegalStateException("Buffer must be writable in order to compact, but was read-only."); + throw new BufferReadOnlyException("Buffer must be writable in order to compact, but was read-only."); } if (roff == 0) { return; @@ -1340,17 +1346,17 @@ class UnsafeBuffer extends ResourceSupport implements Buff private RuntimeException readAccessCheckException(int index) { if (rsize == CLOSED_SIZE) { - throw bufferIsClosed(); + throw bufferIsClosed(this); } return outOfBounds(index); } private RuntimeException writeAccessCheckException(int index) { if (rsize == CLOSED_SIZE) { - throw bufferIsClosed(); + throw bufferIsClosed(this); } if (wsize != rsize) { - return bufferIsReadOnly(); + return bufferIsReadOnly(this); } return outOfBounds(index); } 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 234fe26..840ee94 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 @@ -39,6 +39,8 @@ import jdk.incubator.foreign.ResourceScope; import java.nio.ByteBuffer; import java.nio.ByteOrder; +import static io.netty.buffer.api.internal.Statics.bufferIsClosed; +import static io.netty.buffer.api.internal.Statics.bufferIsReadOnly; import static jdk.incubator.foreign.MemoryAccess.getByteAtOffset; import static jdk.incubator.foreign.MemoryAccess.getCharAtOffset; import static jdk.incubator.foreign.MemoryAccess.getDoubleAtOffset; @@ -164,6 +166,11 @@ class MemSegBuffer extends ResourceSupport implements Buff return "Buffer[roff:" + roff + ", woff:" + woff + ", cap:" + seg.byteSize() + ']'; } + @Override + protected RuntimeException createResourceClosedException() { + return bufferIsClosed(this); + } + @Override public Buffer order(ByteOrder order) { this.order = order; @@ -338,7 +345,7 @@ class MemSegBuffer extends ResourceSupport implements Buff private void copyInto(int srcPos, MemorySegment dest, int destPos, int length) { if (seg == CLOSED_SEGMENT) { - throw Statics.bufferIsClosed(); + throw bufferIsClosed(this); } if (srcPos < 0) { throw new IllegalArgumentException("The srcPos cannot be negative: " + srcPos + '.'); @@ -373,7 +380,7 @@ class MemSegBuffer extends ResourceSupport implements Buff @Override public ByteCursor openCursor(int fromOffset, int length) { if (seg == CLOSED_SEGMENT) { - throw Statics.bufferIsClosed(); + throw bufferIsClosed(this); } if (fromOffset < 0) { throw new IllegalArgumentException("The fromOffset cannot be negative: " + fromOffset + '.'); @@ -443,7 +450,7 @@ class MemSegBuffer extends ResourceSupport implements Buff @Override public ByteCursor openReverseCursor(int fromOffset, int length) { if (seg == CLOSED_SEGMENT) { - throw Statics.bufferIsClosed(); + throw bufferIsClosed(this); } if (fromOffset < 0) { throw new IllegalArgumentException("The fromOffset cannot be negative: " + fromOffset + '.'); @@ -521,7 +528,7 @@ class MemSegBuffer extends ResourceSupport implements Buff throw new IllegalArgumentException("The minimum growth cannot be negative: " + minimumGrowth + '.'); } if (seg != wseg) { - throw Statics.bufferIsReadOnly(); + throw bufferIsReadOnly(this); } if (writableBytes() >= size) { // We already have enough space. @@ -1184,27 +1191,27 @@ class MemSegBuffer extends ResourceSupport implements Buff private RuntimeException checkWriteState(IndexOutOfBoundsException ioobe) { if (seg == CLOSED_SEGMENT) { - return Statics.bufferIsClosed(); + return bufferIsClosed(this); } if (wseg != seg) { - return Statics.bufferIsReadOnly(); + return bufferIsReadOnly(this); } return ioobe; } private RuntimeException readAccessCheckException(int index) { if (seg == CLOSED_SEGMENT) { - throw Statics.bufferIsClosed(); + throw bufferIsClosed(this); } return outOfBounds(index); } private RuntimeException writeAccessCheckException(int index) { if (seg == CLOSED_SEGMENT) { - throw Statics.bufferIsClosed(); + throw bufferIsClosed(this); } if (wseg != seg) { - return Statics.bufferIsReadOnly(); + return bufferIsReadOnly(this); } return outOfBounds(index); } 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 0f3b541..ab4c78e 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 @@ -17,6 +17,8 @@ package io.netty.buffer.api.tests; import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; +import io.netty.buffer.api.BufferClosedException; +import io.netty.buffer.api.BufferReadOnlyException; import io.netty.buffer.api.ByteCursor; import io.netty.buffer.api.CompositeBuffer; import org.junit.jupiter.api.Test; @@ -189,7 +191,7 @@ public class BufferComponentIterationTest extends BufferTestSupport { var buf = allocator.allocate(8); buf.writeLong(0); buf.close(); - assertThrows(IllegalStateException.class, () -> buf.forEachReadable(0, (component, index) -> true)); + assertThrows(BufferClosedException.class, () -> buf.forEachReadable(0, (component, index) -> true)); } } @@ -357,7 +359,7 @@ public class BufferComponentIterationTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator()) { Buffer buf = allocator.allocate(8); buf.close(); - assertThrows(IllegalStateException.class, () -> buf.forEachWritable(0, (index, component) -> true)); + assertThrows(BufferClosedException.class, () -> buf.forEachWritable(0, (index, component) -> true)); } } @@ -366,7 +368,7 @@ public class BufferComponentIterationTest extends BufferTestSupport { public void forEachWritableOnReadOnlyBufferMustThrow(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8).makeReadOnly()) { - assertThrows(IllegalStateException.class, () -> buf.forEachWritable(0, (index, component) -> true)); + assertThrows(BufferReadOnlyException.class, () -> buf.forEachWritable(0, (index, component) -> true)); } } 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 5d396fe..bc37a46 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 @@ -17,6 +17,7 @@ package io.netty.buffer.api.tests; import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; +import io.netty.buffer.api.BufferReadOnlyException; import io.netty.buffer.api.CompositeBuffer; import io.netty.buffer.api.Send; import io.netty.buffer.api.internal.ResourceSupport; @@ -391,7 +392,7 @@ public class BufferCompositionTest extends BufferTestSupport { Buffer b = allocator.allocate(4).makeReadOnly(); Buffer composite = CompositeBuffer.compose(allocator, a.send(), b.send())) { assertTrue(composite.readOnly()); - verifyWriteInaccessible(composite); + verifyWriteInaccessible(composite, BufferReadOnlyException.class, BufferReadOnlyException.class); } } 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 3d4bf01..78961fe 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 @@ -17,6 +17,7 @@ package io.netty.buffer.api.tests; import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; +import io.netty.buffer.api.BufferReadOnlyException; import io.netty.buffer.api.CompositeBuffer; import io.netty.buffer.api.Send; import io.netty.buffer.api.internal.ResourceSupport; @@ -43,7 +44,7 @@ public class BufferReadOnlyTest extends BufferTestSupport { Buffer buf = allocator.allocate(8)) { var b = buf.makeReadOnly(); assertThat(b).isSameAs(buf); - verifyWriteInaccessible(buf); + verifyWriteInaccessible(buf, BufferReadOnlyException.class, BufferReadOnlyException.class); } } @@ -66,12 +67,12 @@ public class BufferReadOnlyTest extends BufferTestSupport { assertFalse(buf.readOnly()); buf.makeReadOnly(); assertTrue(buf.readOnly()); - verifyWriteInaccessible(buf); + verifyWriteInaccessible(buf, BufferReadOnlyException.class, BufferReadOnlyException.class); buf.makeReadOnly(); assertTrue(buf.readOnly()); - verifyWriteInaccessible(buf); + verifyWriteInaccessible(buf, BufferReadOnlyException.class, BufferReadOnlyException.class); } } @@ -84,7 +85,7 @@ public class BufferReadOnlyTest extends BufferTestSupport { var send = buf.send(); try (Buffer receive = send.receive()) { assertTrue(receive.readOnly()); - verifyWriteInaccessible(receive); + verifyWriteInaccessible(receive, BufferReadOnlyException.class, BufferReadOnlyException.class); } } } @@ -121,7 +122,7 @@ public class BufferReadOnlyTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { buf.makeReadOnly(); - assertThrows(IllegalStateException.class, () -> buf.compact()); + assertThrows(BufferReadOnlyException.class, () -> buf.compact()); } } @@ -131,7 +132,7 @@ public class BufferReadOnlyTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { buf.makeReadOnly(); - assertThrows(IllegalStateException.class, () -> buf.ensureWritable(1)); + assertThrows(BufferReadOnlyException.class, () -> buf.ensureWritable(1)); } } @@ -142,7 +143,7 @@ public class BufferReadOnlyTest extends BufferTestSupport { Buffer dest = allocator.allocate(8)) { dest.makeReadOnly(); try (Buffer src = allocator.allocate(8)) { - assertThrows(IllegalStateException.class, () -> src.copyInto(0, dest, 0, 1)); + assertThrows(BufferReadOnlyException.class, () -> src.copyInto(0, dest, 0, 1)); } } } @@ -152,7 +153,7 @@ public class BufferReadOnlyTest extends BufferTestSupport { public void readOnlyBuffersCannotChangeWriteOffset(Fixture fixture) { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8).makeReadOnly()) { - assertThrows(IllegalStateException.class, () -> buf.writerOffset(4)); + assertThrows(BufferReadOnlyException.class, () -> buf.writerOffset(4)); } } @@ -215,19 +216,19 @@ public class BufferReadOnlyTest extends BufferTestSupport { Buffer c = a.copy()) { assertEquals(1, a.readByte()); assertEquals(2, a.readByte()); - assertThrows(IllegalStateException.class, () -> a.compact()); // Can't compact read-only buffer. + assertThrows(BufferReadOnlyException.class, () -> a.compact()); // Can't compact read-only buffer. assertEquals(3, a.readByte()); assertEquals(4, a.readByte()); assertEquals(1, b.readByte()); assertEquals(2, b.readByte()); - assertThrows(IllegalStateException.class, () -> b.compact()); // Can't compact read-only buffer. + assertThrows(BufferReadOnlyException.class, () -> b.compact()); // Can't compact read-only buffer. assertEquals(3, b.readByte()); assertEquals(4, b.readByte()); assertEquals(1, c.readByte()); assertEquals(2, c.readByte()); - assertThrows(IllegalStateException.class, () -> c.compact()); // Can't compact read-only buffer. + assertThrows(BufferReadOnlyException.class, () -> c.compact()); // Can't compact read-only buffer. assertEquals(3, c.readByte()); assertEquals(4, c.readByte()); } 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 e71743a..d5d8a55 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 @@ -17,6 +17,7 @@ package io.netty.buffer.api.tests; import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; +import io.netty.buffer.api.BufferClosedException; import io.netty.buffer.api.BufferRef; import io.netty.buffer.api.Send; import org.junit.jupiter.api.Test; @@ -24,7 +25,7 @@ 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.*; +import static org.junit.jupiter.api.Assertions.assertThrows; class BufferRefTest { @Test @@ -37,7 +38,7 @@ class BufferRefTest { ref.contents().writeInt(42); assertThat(ref.contents().readInt()).isEqualTo(42); ref.close(); - assertThrows(IllegalStateException.class, () -> ref.contents().writeInt(32)); + assertThrows(BufferClosedException.class, () -> ref.contents().writeInt(32)); } } @@ -49,34 +50,7 @@ class BufferRefTest { ref.contents().writeInt(42); assertThat(ref.contents().readInt()).isEqualTo(42); ref.close(); - assertThrows(IllegalStateException.class, () -> ref.contents().writeInt(32)); - } - } - - @Test - public void mustCloseOwnedBufferWhenReplaced() { - try (BufferAllocator allocator = BufferAllocator.heap()) { - AtomicReference orig = new AtomicReference<>(); - BufferRef ref; - Send s = allocator.allocate(8).send(); - ref = new BufferRef(Send.sending(Buffer.class, () -> { - Buffer b = s.receive(); - orig.set(b); - return b; - })); - - orig.get().writeInt(42); - assertThat(ref.contents().readInt()).isEqualTo(42); - - try (Buffer buf = allocator.allocate(8)) { - ref.replace(buf.send()); // Pass replacement directly. - } - - assertThrows(IllegalStateException.class, () -> orig.get().writeInt(32)); - ref.contents().writeInt(42); - assertThat(ref.contents().readInt()).isEqualTo(42); - ref.close(); - assertThrows(IllegalStateException.class, () -> ref.contents().writeInt(32)); + assertThrows(BufferClosedException.class, () -> ref.contents().writeInt(32)); } } @@ -99,11 +73,11 @@ class BufferRefTest { ref.replace(buf.send()); // Pass replacement via send(). } - assertThrows(IllegalStateException.class, () -> orig.get().writeInt(32)); + assertThrows(BufferClosedException.class, () -> orig.get().writeInt(32)); ref.contents().writeInt(42); assertThat(ref.contents().readInt()).isEqualTo(42); ref.close(); - assertThrows(IllegalStateException.class, () -> ref.contents().writeInt(32)); + assertThrows(BufferClosedException.class, () -> ref.contents().writeInt(32)); } } @@ -113,7 +87,7 @@ class BufferRefTest { BufferRef refA = new BufferRef(allocator.allocate(8).send())) { refA.contents().writeInt(42); Send send = refA.send(); - assertThrows(IllegalStateException.class, () -> refA.contents().readInt()); + assertThrows(BufferClosedException.class, () -> refA.contents().readInt()); try (BufferRef refB = send.receive()) { assertThat(refB.contents().readInt()).isEqualTo(42); } 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 956795b..b534719 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 @@ -17,6 +17,7 @@ package io.netty.buffer.api.tests; import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; +import io.netty.buffer.api.BufferClosedException; import io.netty.buffer.api.CompositeBuffer; import io.netty.buffer.api.internal.ResourceSupport; import org.junit.jupiter.api.Test; @@ -77,7 +78,7 @@ public class BufferReferenceCountingTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator()) { var buf = allocator.allocate(8); buf.close(); - assertThrows(IllegalStateException.class, () -> acquire((ResourceSupport) buf)); + assertThrows(BufferClosedException.class, () -> acquire((ResourceSupport) buf)); } } 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 b6acd29..7535481 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 @@ -17,6 +17,7 @@ package io.netty.buffer.api.tests; import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; +import io.netty.buffer.api.BufferClosedException; import io.netty.buffer.api.CompositeBuffer; import io.netty.buffer.api.MemoryManagers; import io.netty.buffer.api.internal.ResourceSupport; @@ -323,88 +324,91 @@ public abstract class BufferTestSupport { public static void verifyInaccessible(Buffer buf) { verifyReadInaccessible(buf); - verifyWriteInaccessible(buf); + verifyWriteInaccessible(buf, BufferClosedException.class, IllegalStateException.class); try (BufferAllocator allocator = BufferAllocator.heap(); Buffer target = allocator.allocate(24)) { - assertThrows(IllegalStateException.class, () -> buf.copyInto(0, target, 0, 1)); - assertThrows(IllegalStateException.class, () -> buf.copyInto(0, new byte[1], 0, 1)); - assertThrows(IllegalStateException.class, () -> buf.copyInto(0, ByteBuffer.allocate(1), 0, 1)); + assertThrows(BufferClosedException.class, () -> buf.copyInto(0, target, 0, 1)); + assertThrows(BufferClosedException.class, () -> buf.copyInto(0, new byte[1], 0, 1)); + assertThrows(BufferClosedException.class, () -> buf.copyInto(0, ByteBuffer.allocate(1), 0, 1)); if (CompositeBuffer.isComposite(buf)) { assertThrows(IllegalStateException.class, () -> ((CompositeBuffer) buf).extendWith(target.send())); } } - assertThrows(IllegalStateException.class, () -> buf.split()); - 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)); - assertThrows(IllegalStateException.class, () -> buf.openReverseCursor()); - assertThrows(IllegalStateException.class, () -> buf.openReverseCursor(0, 0)); + assertThrows(BufferClosedException.class, () -> buf.split()); + assertThrows(BufferClosedException.class, () -> buf.send()); + assertThrows(UnsupportedOperationException.class, () -> acquire((ResourceSupport) buf)); + final RuntimeException c1e = assertThrows(RuntimeException.class, () -> buf.copy()); + assertTrue(c1e instanceof UnsupportedOperationException || c1e instanceof IllegalStateException); + assertThrows(BufferClosedException.class, () -> buf.openCursor()); + assertThrows(BufferClosedException.class, () -> buf.openCursor(0, 0)); + assertThrows(BufferClosedException.class, () -> buf.openReverseCursor()); + assertThrows(BufferClosedException.class, () -> buf.openReverseCursor(0, 0)); } public static void verifyReadInaccessible(Buffer buf) { - assertThrows(IllegalStateException.class, () -> buf.readByte()); - assertThrows(IllegalStateException.class, () -> buf.readUnsignedByte()); - assertThrows(IllegalStateException.class, () -> buf.readChar()); - assertThrows(IllegalStateException.class, () -> buf.readShort()); - assertThrows(IllegalStateException.class, () -> buf.readUnsignedShort()); - assertThrows(IllegalStateException.class, () -> buf.readMedium()); - assertThrows(IllegalStateException.class, () -> buf.readUnsignedMedium()); - assertThrows(IllegalStateException.class, () -> buf.readInt()); - assertThrows(IllegalStateException.class, () -> buf.readUnsignedInt()); - assertThrows(IllegalStateException.class, () -> buf.readFloat()); - assertThrows(IllegalStateException.class, () -> buf.readLong()); - assertThrows(IllegalStateException.class, () -> buf.readDouble()); + assertThrows(UnsupportedOperationException.class, () -> buf.readByte()); + assertThrows(UnsupportedOperationException.class, () -> buf.readUnsignedByte()); + assertThrows(UnsupportedOperationException.class, () -> buf.readChar()); + assertThrows(UnsupportedOperationException.class, () -> buf.readShort()); + assertThrows(UnsupportedOperationException.class, () -> buf.readUnsignedShort()); + assertThrows(UnsupportedOperationException.class, () -> buf.readMedium()); + assertThrows(UnsupportedOperationException.class, () -> buf.readUnsignedMedium()); + assertThrows(UnsupportedOperationException.class, () -> buf.readInt()); + assertThrows(UnsupportedOperationException.class, () -> buf.readUnsignedInt()); + assertThrows(UnsupportedOperationException.class, () -> buf.readFloat()); + assertThrows(UnsupportedOperationException.class, () -> buf.readLong()); + assertThrows(UnsupportedOperationException.class, () -> buf.readDouble()); - assertThrows(IllegalStateException.class, () -> buf.getByte(0)); - assertThrows(IllegalStateException.class, () -> buf.getUnsignedByte(0)); - assertThrows(IllegalStateException.class, () -> buf.getChar(0)); - assertThrows(IllegalStateException.class, () -> buf.getShort(0)); - assertThrows(IllegalStateException.class, () -> buf.getUnsignedShort(0)); - assertThrows(IllegalStateException.class, () -> buf.getMedium(0)); - assertThrows(IllegalStateException.class, () -> buf.getUnsignedMedium(0)); - assertThrows(IllegalStateException.class, () -> buf.getInt(0)); - assertThrows(IllegalStateException.class, () -> buf.getUnsignedInt(0)); - assertThrows(IllegalStateException.class, () -> buf.getFloat(0)); - assertThrows(IllegalStateException.class, () -> buf.getLong(0)); - assertThrows(IllegalStateException.class, () -> buf.getDouble(0)); + assertThrows(UnsupportedOperationException.class, () -> buf.getByte(0)); + assertThrows(UnsupportedOperationException.class, () -> buf.getUnsignedByte(0)); + assertThrows(UnsupportedOperationException.class, () -> buf.getChar(0)); + assertThrows(UnsupportedOperationException.class, () -> buf.getShort(0)); + assertThrows(UnsupportedOperationException.class, () -> buf.getUnsignedShort(0)); + assertThrows(UnsupportedOperationException.class, () -> buf.getMedium(0)); + assertThrows(UnsupportedOperationException.class, () -> buf.getUnsignedMedium(0)); + assertThrows(UnsupportedOperationException.class, () -> buf.getInt(0)); + assertThrows(UnsupportedOperationException.class, () -> buf.getUnsignedInt(0)); + assertThrows(UnsupportedOperationException.class, () -> buf.getFloat(0)); + assertThrows(UnsupportedOperationException.class, () -> buf.getLong(0)); + assertThrows(UnsupportedOperationException.class, () -> buf.getDouble(0)); } - public static void verifyWriteInaccessible(Buffer buf) { - assertThrows(IllegalStateException.class, () -> buf.writeByte((byte) 32)); - assertThrows(IllegalStateException.class, () -> buf.writeUnsignedByte(32)); - assertThrows(IllegalStateException.class, () -> buf.writeChar('3')); - assertThrows(IllegalStateException.class, () -> buf.writeShort((short) 32)); - assertThrows(IllegalStateException.class, () -> buf.writeUnsignedShort(32)); - assertThrows(IllegalStateException.class, () -> buf.writeMedium(32)); - assertThrows(IllegalStateException.class, () -> buf.writeUnsignedMedium(32)); - assertThrows(IllegalStateException.class, () -> buf.writeInt(32)); - assertThrows(IllegalStateException.class, () -> buf.writeUnsignedInt(32)); - assertThrows(IllegalStateException.class, () -> buf.writeFloat(3.2f)); - assertThrows(IllegalStateException.class, () -> buf.writeLong(32)); - assertThrows(IllegalStateException.class, () -> buf.writeDouble(32)); + public static void verifyWriteInaccessible(Buffer buf, Class expected, + Class expectedEnsureWritable) { + assertThrows(expected, () -> buf.writeByte((byte) 32)); + assertThrows(expected, () -> buf.writeUnsignedByte(32)); + assertThrows(expected, () -> buf.writeChar('3')); + assertThrows(expected, () -> buf.writeShort((short) 32)); + assertThrows(expected, () -> buf.writeUnsignedShort(32)); + assertThrows(expected, () -> buf.writeMedium(32)); + assertThrows(expected, () -> buf.writeUnsignedMedium(32)); + assertThrows(expected, () -> buf.writeInt(32)); + assertThrows(expected, () -> buf.writeUnsignedInt(32)); + assertThrows(expected, () -> buf.writeFloat(3.2f)); + assertThrows(expected, () -> buf.writeLong(32)); + assertThrows(expected, () -> buf.writeDouble(32)); - assertThrows(IllegalStateException.class, () -> buf.setByte(0, (byte) 32)); - assertThrows(IllegalStateException.class, () -> buf.setUnsignedByte(0, 32)); - assertThrows(IllegalStateException.class, () -> buf.setChar(0, '3')); - assertThrows(IllegalStateException.class, () -> buf.setShort(0, (short) 32)); - assertThrows(IllegalStateException.class, () -> buf.setUnsignedShort(0, 32)); - assertThrows(IllegalStateException.class, () -> buf.setMedium(0, 32)); - assertThrows(IllegalStateException.class, () -> buf.setUnsignedMedium(0, 32)); - assertThrows(IllegalStateException.class, () -> buf.setInt(0, 32)); - assertThrows(IllegalStateException.class, () -> buf.setUnsignedInt(0, 32)); - assertThrows(IllegalStateException.class, () -> buf.setFloat(0, 3.2f)); - assertThrows(IllegalStateException.class, () -> buf.setLong(0, 32)); - assertThrows(IllegalStateException.class, () -> buf.setDouble(0, 32)); + assertThrows(expected, () -> buf.setByte(0, (byte) 32)); + assertThrows(expected, () -> buf.setUnsignedByte(0, 32)); + assertThrows(expected, () -> buf.setChar(0, '3')); + assertThrows(expected, () -> buf.setShort(0, (short) 32)); + assertThrows(expected, () -> buf.setUnsignedShort(0, 32)); + assertThrows(expected, () -> buf.setMedium(0, 32)); + assertThrows(expected, () -> buf.setUnsignedMedium(0, 32)); + assertThrows(expected, () -> buf.setInt(0, 32)); + assertThrows(expected, () -> buf.setUnsignedInt(0, 32)); + assertThrows(expected, () -> buf.setFloat(0, 3.2f)); + assertThrows(expected, () -> buf.setLong(0, 32)); + assertThrows(expected, () -> buf.setDouble(0, 32)); - assertThrows(IllegalStateException.class, () -> buf.ensureWritable(1)); - assertThrows(IllegalStateException.class, () -> buf.fill((byte) 0)); + assertThrows(expectedEnsureWritable, () -> buf.ensureWritable(1)); + final RuntimeException f1e = assertThrows(RuntimeException.class, () -> buf.fill((byte) 0)); + assertTrue(f1e instanceof IllegalStateException || f1e instanceof UnsupportedOperationException); try (BufferAllocator allocator = BufferAllocator.heap(); Buffer source = allocator.allocate(8)) { - assertThrows(IllegalStateException.class, () -> source.copyInto(0, buf, 0, 1)); + assertThrows(expected, () -> source.copyInto(0, buf, 0, 1)); } } 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 7ec2e41..99030e0 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 @@ -51,6 +51,11 @@ public class ScopeTest { super(drop); } + @Override + protected RuntimeException createResourceClosedException() { + return new IllegalStateException("This resource is closed: " + this); + } + @Override protected Owned prepareSend() { return null; From 60df2393f38bddda8d89d25d8156a280b3623aba Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Tue, 1 Jun 2021 10:28:42 +0200 Subject: [PATCH 2/6] Get the tests passing again --- .../src/main/java/io/netty/buffer/api/CompositeBuffer.java | 3 +++ .../main/java/io/netty/buffer/api/bytebuffer/NioBuffer.java | 3 +++ .../java/io/netty/buffer/api/internal/ResourceSupport.java | 3 +++ .../main/java/io/netty/buffer/api/unsafe/UnsafeBuffer.java | 3 +++ .../main/java/io/netty/buffer/api/memseg/MemSegBuffer.java | 3 +++ .../test/java/io/netty/buffer/api/tests/BufferSendTest.java | 5 +++-- 6 files changed, 18 insertions(+), 2 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 76713ac..cf10acd 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 @@ -891,6 +891,9 @@ public final class CompositeBuffer extends ResourceSupport implements Buffer, Re throw new IllegalArgumentException("The split offset cannot be greater than the buffer capacity, " + "but the split offset was " + splitOffset + ", and capacity is " + capacity() + '.'); } + if (!isAccessible()) { + throw attachTrace(bufferIsClosed(this)); + } if (!isOwned()) { throw attachTrace(new IllegalStateException("Cannot split a buffer that is not owned.")); } 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 139512f..6def829 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 @@ -108,6 +108,9 @@ public abstract class ResourceSupport, T extends ResourceS */ @Override public final Send send() { + if (acquires < 0) { + throw attachTrace(createResourceClosedException()); + } if (!isOwned()) { throw notSendableException(); } 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 d97d750..115ecd2 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 @@ -506,6 +506,9 @@ class UnsafeBuffer extends ResourceSupport implements Buff throw new IllegalArgumentException("The split offset cannot be greater than the buffer capacity, " + "but the split offset was " + splitOffset + ", and capacity is " + capacity() + '.'); } + if (!isAccessible()) { + throw attachTrace(bufferIsClosed(this)); + } if (!isOwned()) { throw attachTrace(new IllegalStateException("Cannot split a buffer that is not owned.")); } 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 840ee94..e347dd6 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 @@ -584,6 +584,9 @@ class MemSegBuffer extends ResourceSupport implements Buff throw new IllegalArgumentException("The split offset cannot be greater than the buffer capacity, " + "but the split offset was " + splitOffset + ", and capacity is " + capacity() + '.'); } + if (!isAccessible()) { + throw attachTrace(bufferIsClosed(this)); + } if (!isOwned()) { throw attachTrace(new IllegalStateException("Cannot split a buffer that is not owned.")); } 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 7161a2c..21ecb0c 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 @@ -17,6 +17,7 @@ package io.netty.buffer.api.tests; import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; +import io.netty.buffer.api.BufferClosedException; import io.netty.buffer.api.BufferRef; import io.netty.buffer.api.Send; import io.netty.buffer.api.internal.ResourceSupport; @@ -110,9 +111,9 @@ public class BufferSendTest extends BufferTestSupport { try (BufferAllocator allocator = fixture.createAllocator(); Buffer buf = allocator.allocate(8)) { var send = buf.send(); - var exc = assertThrows(IllegalStateException.class, () -> buf.send()); + var exc = assertThrows(BufferClosedException.class, () -> buf.send()); send.receive().close(); - assertThat(exc).hasMessageContaining("Cannot send()"); + assertThat(exc).hasMessageContaining("closed"); } } From 7f74dbc7aaedb337e39fd63ca94f062a3c235f8e Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Tue, 1 Jun 2021 10:34:41 +0200 Subject: [PATCH 3/6] Fix exceptions in ByteBufAdaptor --- .../buffer/api/adaptor/ByteBufAdaptor.java | 101 +++++++++--------- 1 file changed, 51 insertions(+), 50 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 7e79039..3ada76d 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 @@ -23,6 +23,7 @@ import io.netty.buffer.DuplicatedByteBuf; import io.netty.buffer.SlicedByteBuf; import io.netty.buffer.Unpooled; import io.netty.buffer.api.BufferAllocator; +import io.netty.buffer.api.BufferClosedException; import io.netty.buffer.api.BufferReadOnlyException; import io.netty.buffer.api.internal.Statics; import io.netty.buffer.api.Buffer; @@ -254,7 +255,7 @@ public final class ByteBufAdaptor extends ByteBuf { public byte getByte(int index) { try { return buffer.getByte(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -263,7 +264,7 @@ public final class ByteBufAdaptor extends ByteBuf { public short getUnsignedByte(int index) { try { return (short) buffer.getUnsignedByte(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -272,7 +273,7 @@ public final class ByteBufAdaptor extends ByteBuf { public short getShort(int index) { try { return buffer.getShort(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -282,7 +283,7 @@ public final class ByteBufAdaptor extends ByteBuf { ByteOrder originalOrder = buffer.order(); try { return buffer.order(ByteOrder.LITTLE_ENDIAN).getShort(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -293,7 +294,7 @@ public final class ByteBufAdaptor extends ByteBuf { public int getUnsignedShort(int index) { try { return buffer.getUnsignedShort(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -303,7 +304,7 @@ public final class ByteBufAdaptor extends ByteBuf { ByteOrder originalOrder = buffer.order(); try { return buffer.order(ByteOrder.LITTLE_ENDIAN).getUnsignedShort(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -314,7 +315,7 @@ public final class ByteBufAdaptor extends ByteBuf { public int getMedium(int index) { try { return buffer.getMedium(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -324,7 +325,7 @@ public final class ByteBufAdaptor extends ByteBuf { ByteOrder originalOrder = buffer.order(); try { return buffer.order(ByteOrder.LITTLE_ENDIAN).getMedium(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -335,7 +336,7 @@ public final class ByteBufAdaptor extends ByteBuf { public int getUnsignedMedium(int index) { try { return buffer.getUnsignedMedium(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -345,7 +346,7 @@ public final class ByteBufAdaptor extends ByteBuf { ByteOrder originalOrder = buffer.order(); try { return buffer.order(ByteOrder.LITTLE_ENDIAN).getUnsignedMedium(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -356,7 +357,7 @@ public final class ByteBufAdaptor extends ByteBuf { public int getInt(int index) { try { return buffer.getInt(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -366,7 +367,7 @@ public final class ByteBufAdaptor extends ByteBuf { ByteOrder originalOrder = buffer.order(); try { return buffer.order(ByteOrder.LITTLE_ENDIAN).getInt(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -377,7 +378,7 @@ public final class ByteBufAdaptor extends ByteBuf { public long getUnsignedInt(int index) { try { return buffer.getUnsignedInt(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -387,7 +388,7 @@ public final class ByteBufAdaptor extends ByteBuf { ByteOrder originalOrder = buffer.order(); try { return buffer.order(ByteOrder.LITTLE_ENDIAN).getUnsignedInt(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -398,7 +399,7 @@ public final class ByteBufAdaptor extends ByteBuf { public long getLong(int index) { try { return buffer.getLong(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -408,7 +409,7 @@ public final class ByteBufAdaptor extends ByteBuf { ByteOrder originalOrder = buffer.order(); try { return buffer.order(ByteOrder.LITTLE_ENDIAN).getLong(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -419,7 +420,7 @@ public final class ByteBufAdaptor extends ByteBuf { public char getChar(int index) { try { return buffer.getChar(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -428,7 +429,7 @@ public final class ByteBufAdaptor extends ByteBuf { public float getFloat(int index) { try { return buffer.getFloat(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -437,7 +438,7 @@ public final class ByteBufAdaptor extends ByteBuf { public double getDouble(int index) { try { return buffer.getDouble(index); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -536,7 +537,7 @@ public final class ByteBufAdaptor extends ByteBuf { public ByteBuf setByte(int index, int value) { try { buffer.setByte(index, (byte) value); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } return this; @@ -547,7 +548,7 @@ public final class ByteBufAdaptor extends ByteBuf { try { buffer.setShort(index, (short) value); return this; - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -558,7 +559,7 @@ public final class ByteBufAdaptor extends ByteBuf { try { buffer.order(ByteOrder.LITTLE_ENDIAN).setShort(index, (short) value); return this; - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -570,7 +571,7 @@ public final class ByteBufAdaptor extends ByteBuf { try { buffer.setMedium(index, value); return this; - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -581,7 +582,7 @@ public final class ByteBufAdaptor extends ByteBuf { try { buffer.order(ByteOrder.LITTLE_ENDIAN).setMedium(index, value); return this; - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -593,7 +594,7 @@ public final class ByteBufAdaptor extends ByteBuf { try { buffer.setInt(index, value); return this; - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -604,7 +605,7 @@ public final class ByteBufAdaptor extends ByteBuf { try { buffer.order(ByteOrder.LITTLE_ENDIAN).setInt(index, value); return this; - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -616,7 +617,7 @@ public final class ByteBufAdaptor extends ByteBuf { try { buffer.setLong(index, value); return this; - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -627,7 +628,7 @@ public final class ByteBufAdaptor extends ByteBuf { try { buffer.order(ByteOrder.LITTLE_ENDIAN).setLong(index, value); return this; - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -639,7 +640,7 @@ public final class ByteBufAdaptor extends ByteBuf { try { buffer.setChar(index, (char) value); return this; - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -649,7 +650,7 @@ public final class ByteBufAdaptor extends ByteBuf { try { buffer.setFloat(index, value); return this; - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -659,7 +660,7 @@ public final class ByteBufAdaptor extends ByteBuf { try { buffer.setDouble(index, value); return this; - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -766,7 +767,7 @@ public final class ByteBufAdaptor extends ByteBuf { public byte readByte() { try { return buffer.readByte(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -775,7 +776,7 @@ public final class ByteBufAdaptor extends ByteBuf { public short readUnsignedByte() { try { return (short) buffer.readUnsignedByte(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -784,7 +785,7 @@ public final class ByteBufAdaptor extends ByteBuf { public short readShort() { try { return buffer.readShort(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -794,7 +795,7 @@ public final class ByteBufAdaptor extends ByteBuf { ByteOrder originalOrder = buffer.order(); try { return buffer.order(ByteOrder.LITTLE_ENDIAN).readShort(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -805,7 +806,7 @@ public final class ByteBufAdaptor extends ByteBuf { public int readUnsignedShort() { try { return buffer.readUnsignedShort(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -815,7 +816,7 @@ public final class ByteBufAdaptor extends ByteBuf { ByteOrder originalOrder = buffer.order(); try { return buffer.order(ByteOrder.LITTLE_ENDIAN).readUnsignedShort(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -826,7 +827,7 @@ public final class ByteBufAdaptor extends ByteBuf { public int readMedium() { try { return buffer.readMedium(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -836,7 +837,7 @@ public final class ByteBufAdaptor extends ByteBuf { ByteOrder originalOrder = buffer.order(); try { return buffer.order(ByteOrder.LITTLE_ENDIAN).readMedium(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -847,7 +848,7 @@ public final class ByteBufAdaptor extends ByteBuf { public int readUnsignedMedium() { try { return buffer.readUnsignedMedium(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -857,7 +858,7 @@ public final class ByteBufAdaptor extends ByteBuf { ByteOrder originalOrder = buffer.order(); try { return buffer.order(ByteOrder.LITTLE_ENDIAN).readUnsignedMedium(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -868,7 +869,7 @@ public final class ByteBufAdaptor extends ByteBuf { public int readInt() { try { return buffer.readInt(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -878,7 +879,7 @@ public final class ByteBufAdaptor extends ByteBuf { ByteOrder originalOrder = buffer.order(); try { return buffer.order(ByteOrder.LITTLE_ENDIAN).readInt(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -889,7 +890,7 @@ public final class ByteBufAdaptor extends ByteBuf { public long readUnsignedInt() { try { return buffer.readUnsignedInt(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -899,7 +900,7 @@ public final class ByteBufAdaptor extends ByteBuf { ByteOrder originalOrder = buffer.order(); try { return buffer.order(ByteOrder.LITTLE_ENDIAN).readUnsignedInt(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -910,7 +911,7 @@ public final class ByteBufAdaptor extends ByteBuf { public long readLong() { try { return buffer.readLong(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -920,7 +921,7 @@ public final class ByteBufAdaptor extends ByteBuf { ByteOrder originalOrder = buffer.order(); try { return buffer.order(ByteOrder.LITTLE_ENDIAN).readLong(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } finally { buffer.order(originalOrder); @@ -931,7 +932,7 @@ public final class ByteBufAdaptor extends ByteBuf { public char readChar() { try { return buffer.readChar(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -940,7 +941,7 @@ public final class ByteBufAdaptor extends ByteBuf { public float readFloat() { try { return buffer.readFloat(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } @@ -949,7 +950,7 @@ public final class ByteBufAdaptor extends ByteBuf { public double readDouble() { try { return buffer.readDouble(); - } catch (BufferReadOnlyException e) { + } catch (IllegalStateException | BufferClosedException e) { throw new IllegalReferenceCountException(e); } } From eaee4350b7f56c9d8ca3a802083baab910b0b1b8 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Tue, 1 Jun 2021 11:20:58 +0200 Subject: [PATCH 4/6] Make MemSegBuffer.compact throw correct exception when read-only --- .../src/main/java/io/netty/buffer/api/memseg/MemSegBuffer.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 e347dd6..c181e48 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 @@ -17,6 +17,7 @@ package io.netty.buffer.api.memseg; import io.netty.buffer.ByteBuf; import io.netty.buffer.api.BufferAllocator; +import io.netty.buffer.api.BufferReadOnlyException; import io.netty.buffer.api.adaptor.BufferIntegratable; import io.netty.buffer.api.adaptor.ByteBufAdaptor; import io.netty.buffer.api.adaptor.ByteBufAllocatorAdaptor; @@ -618,7 +619,7 @@ class MemSegBuffer extends ResourceSupport implements Buff throw attachTrace(new IllegalStateException("Buffer must be owned in order to compact.")); } if (readOnly()) { - throw new IllegalStateException("Buffer must be writable in order to compact, but was read-only."); + throw new BufferReadOnlyException("Buffer must be writable in order to compact, but was read-only."); } int distance = roff; if (distance == 0) { From 289c9ebba186e5ec70e8124794fe4c68f670fc8a Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Tue, 1 Jun 2021 11:35:01 +0200 Subject: [PATCH 5/6] Make Buffer.ensureWritable throw exceptions consistent with the rest of the API --- .../main/java/io/netty/buffer/api/CompositeBuffer.java | 3 +++ .../java/io/netty/buffer/api/bytebuffer/NioBuffer.java | 3 +++ .../java/io/netty/buffer/api/unsafe/UnsafeBuffer.java | 3 +++ .../java/io/netty/buffer/api/memseg/MemSegBuffer.java | 3 +++ .../io/netty/buffer/api/tests/BufferCompositionTest.java | 2 +- .../io/netty/buffer/api/tests/BufferReadOnlyTest.java | 8 ++++---- .../java/io/netty/buffer/api/tests/BufferTestSupport.java | 7 +++---- 7 files changed, 20 insertions(+), 9 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 cf10acd..8d9a097 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 @@ -731,6 +731,9 @@ public final class CompositeBuffer extends ResourceSupport implements Buffer, Re @Override public void ensureWritable(int size, int minimumGrowth, boolean allowCompaction) { + if (!isAccessible()) { + throw bufferIsClosed(this); + } if (!isOwned()) { throw attachTrace(new IllegalStateException( "Buffer is not owned. Only owned buffers can call ensureWritable.")); 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 115ecd2..d0920e9 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 @@ -431,6 +431,9 @@ class UnsafeBuffer extends ResourceSupport implements Buff @Override public void ensureWritable(int size, int minimumGrowth, boolean allowCompaction) { + if (!isAccessible()) { + throw bufferIsClosed(this); + } if (!isOwned()) { throw attachTrace(new IllegalStateException( "Buffer is not owned. Only owned buffers can call ensureWritable.")); 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 c181e48..472d3bf 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 @@ -518,6 +518,9 @@ class MemSegBuffer extends ResourceSupport implements Buff @Override public void ensureWritable(int size, int minimumGrowth, boolean allowCompaction) { + if (!isAccessible()) { + throw bufferIsClosed(this); + } if (!isOwned()) { throw attachTrace(new IllegalStateException( "Buffer is not owned. Only owned buffers can call ensureWritable.")); 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 bc37a46..a01ed25 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 @@ -392,7 +392,7 @@ public class BufferCompositionTest extends BufferTestSupport { Buffer b = allocator.allocate(4).makeReadOnly(); Buffer composite = CompositeBuffer.compose(allocator, a.send(), b.send())) { assertTrue(composite.readOnly()); - verifyWriteInaccessible(composite, BufferReadOnlyException.class, BufferReadOnlyException.class); + verifyWriteInaccessible(composite, BufferReadOnlyException.class); } } 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 78961fe..edffd70 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 @@ -44,7 +44,7 @@ public class BufferReadOnlyTest extends BufferTestSupport { Buffer buf = allocator.allocate(8)) { var b = buf.makeReadOnly(); assertThat(b).isSameAs(buf); - verifyWriteInaccessible(buf, BufferReadOnlyException.class, BufferReadOnlyException.class); + verifyWriteInaccessible(buf, BufferReadOnlyException.class); } } @@ -67,12 +67,12 @@ public class BufferReadOnlyTest extends BufferTestSupport { assertFalse(buf.readOnly()); buf.makeReadOnly(); assertTrue(buf.readOnly()); - verifyWriteInaccessible(buf, BufferReadOnlyException.class, BufferReadOnlyException.class); + verifyWriteInaccessible(buf, BufferReadOnlyException.class); buf.makeReadOnly(); assertTrue(buf.readOnly()); - verifyWriteInaccessible(buf, BufferReadOnlyException.class, BufferReadOnlyException.class); + verifyWriteInaccessible(buf, BufferReadOnlyException.class); } } @@ -85,7 +85,7 @@ public class BufferReadOnlyTest extends BufferTestSupport { var send = buf.send(); try (Buffer receive = send.receive()) { assertTrue(receive.readOnly()); - verifyWriteInaccessible(receive, BufferReadOnlyException.class, BufferReadOnlyException.class); + verifyWriteInaccessible(receive, BufferReadOnlyException.class); } } } 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 7535481..4b5e23f 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 @@ -324,7 +324,7 @@ public abstract class BufferTestSupport { public static void verifyInaccessible(Buffer buf) { verifyReadInaccessible(buf); - verifyWriteInaccessible(buf, BufferClosedException.class, IllegalStateException.class); + verifyWriteInaccessible(buf, BufferClosedException.class); try (BufferAllocator allocator = BufferAllocator.heap(); Buffer target = allocator.allocate(24)) { @@ -375,8 +375,7 @@ public abstract class BufferTestSupport { assertThrows(UnsupportedOperationException.class, () -> buf.getDouble(0)); } - public static void verifyWriteInaccessible(Buffer buf, Class expected, - Class expectedEnsureWritable) { + public static void verifyWriteInaccessible(Buffer buf, Class expected) { assertThrows(expected, () -> buf.writeByte((byte) 32)); assertThrows(expected, () -> buf.writeUnsignedByte(32)); assertThrows(expected, () -> buf.writeChar('3')); @@ -403,7 +402,7 @@ public abstract class BufferTestSupport { assertThrows(expected, () -> buf.setLong(0, 32)); assertThrows(expected, () -> buf.setDouble(0, 32)); - assertThrows(expectedEnsureWritable, () -> buf.ensureWritable(1)); + assertThrows(expected, () -> buf.ensureWritable(1)); final RuntimeException f1e = assertThrows(RuntimeException.class, () -> buf.fill((byte) 0)); assertTrue(f1e instanceof IllegalStateException || f1e instanceof UnsupportedOperationException); try (BufferAllocator allocator = BufferAllocator.heap(); From d0d929bc55f84151f0950dbd510120c804a1da68 Mon Sep 17 00:00:00 2001 From: Chris Vest Date: Tue, 1 Jun 2021 12:04:57 +0200 Subject: [PATCH 6/6] Align the remaining exception expectations and fix MemSegBuffer and CompositeBuffer to throw the correct exceptions --- .../io/netty/buffer/api/CompositeBuffer.java | 5 +- .../netty/buffer/api/memseg/MemSegBuffer.java | 6 ++ .../api/tests/BufferCompositionTest.java | 7 +-- .../buffer/api/tests/BufferTestSupport.java | 58 +++++++++---------- 4 files changed, 41 insertions(+), 35 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 8d9a097..c5c56e2 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 @@ -808,8 +808,11 @@ public final class CompositeBuffer extends ResourceSupport extension) { Objects.requireNonNull(extension, "Extension buffer cannot be null."); Buffer buffer = extension.receive(); - if (!isOwned()) { + if (!isAccessible() || !isOwned()) { buffer.close(); + if (!isAccessible()) { + throw bufferIsClosed(this); + } throw new IllegalStateException("This buffer cannot be extended because it is not in an owned state."); } if (bufs.length > 0 && buffer.order() != order()) { 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 472d3bf..140969d 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 @@ -214,6 +214,9 @@ class MemSegBuffer extends ResourceSupport implements Buff @Override public Buffer fill(byte value) { + if (!isAccessible()) { + throw bufferIsClosed(this); + } checkSet(0, capacity()); seg.fill(value); return this; @@ -288,6 +291,9 @@ class MemSegBuffer extends ResourceSupport implements Buff @Override public long nativeAddress() { + if (!isAccessible()) { + throw bufferIsClosed(this); + } if (seg.isNative()) { return seg.address().toRawLongValue(); } 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 a01ed25..2814b77 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 @@ -17,6 +17,7 @@ package io.netty.buffer.api.tests; import io.netty.buffer.api.Buffer; import io.netty.buffer.api.BufferAllocator; +import io.netty.buffer.api.BufferClosedException; import io.netty.buffer.api.BufferReadOnlyException; import io.netty.buffer.api.CompositeBuffer; import io.netty.buffer.api.Send; @@ -69,7 +70,7 @@ public class BufferCompositionTest extends BufferTestSupport { CompositeBuffer bufA = CompositeBuffer.compose(allocator, allocator.allocate(4).send()); Send sendA = bufA.send(); try { - assertThrows(IllegalStateException.class, () -> bufA.extendWith(sendA)); + assertThrows(BufferClosedException.class, () -> bufA.extendWith(sendA)); } finally { sendA.discard(); } @@ -151,9 +152,7 @@ public class BufferCompositionTest extends BufferTestSupport { composite = CompositeBuffer.compose(allocator, a.send()); } try (composite) { - var exc = assertThrows(IllegalStateException.class, - () -> composite.extendWith(composite.send())); - assertThat(exc).hasMessageContaining("cannot be extended"); + assertThrows(BufferClosedException.class, () -> composite.extendWith(composite.send())); } } } 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 4b5e23f..9a5ccff 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 @@ -332,15 +332,14 @@ public abstract class BufferTestSupport { assertThrows(BufferClosedException.class, () -> buf.copyInto(0, new byte[1], 0, 1)); assertThrows(BufferClosedException.class, () -> buf.copyInto(0, ByteBuffer.allocate(1), 0, 1)); if (CompositeBuffer.isComposite(buf)) { - assertThrows(IllegalStateException.class, () -> ((CompositeBuffer) buf).extendWith(target.send())); + assertThrows(BufferClosedException.class, () -> ((CompositeBuffer) buf).extendWith(target.send())); } } assertThrows(BufferClosedException.class, () -> buf.split()); assertThrows(BufferClosedException.class, () -> buf.send()); - assertThrows(UnsupportedOperationException.class, () -> acquire((ResourceSupport) buf)); - final RuntimeException c1e = assertThrows(RuntimeException.class, () -> buf.copy()); - assertTrue(c1e instanceof UnsupportedOperationException || c1e instanceof IllegalStateException); + assertThrows(BufferClosedException.class, () -> acquire((ResourceSupport) buf)); + assertThrows(BufferClosedException.class, () -> buf.copy()); assertThrows(BufferClosedException.class, () -> buf.openCursor()); assertThrows(BufferClosedException.class, () -> buf.openCursor(0, 0)); assertThrows(BufferClosedException.class, () -> buf.openReverseCursor()); @@ -348,31 +347,31 @@ public abstract class BufferTestSupport { } public static void verifyReadInaccessible(Buffer buf) { - assertThrows(UnsupportedOperationException.class, () -> buf.readByte()); - assertThrows(UnsupportedOperationException.class, () -> buf.readUnsignedByte()); - assertThrows(UnsupportedOperationException.class, () -> buf.readChar()); - assertThrows(UnsupportedOperationException.class, () -> buf.readShort()); - assertThrows(UnsupportedOperationException.class, () -> buf.readUnsignedShort()); - assertThrows(UnsupportedOperationException.class, () -> buf.readMedium()); - assertThrows(UnsupportedOperationException.class, () -> buf.readUnsignedMedium()); - assertThrows(UnsupportedOperationException.class, () -> buf.readInt()); - assertThrows(UnsupportedOperationException.class, () -> buf.readUnsignedInt()); - assertThrows(UnsupportedOperationException.class, () -> buf.readFloat()); - assertThrows(UnsupportedOperationException.class, () -> buf.readLong()); - assertThrows(UnsupportedOperationException.class, () -> buf.readDouble()); + assertThrows(BufferClosedException.class, () -> buf.readByte()); + assertThrows(BufferClosedException.class, () -> buf.readUnsignedByte()); + assertThrows(BufferClosedException.class, () -> buf.readChar()); + assertThrows(BufferClosedException.class, () -> buf.readShort()); + assertThrows(BufferClosedException.class, () -> buf.readUnsignedShort()); + assertThrows(BufferClosedException.class, () -> buf.readMedium()); + assertThrows(BufferClosedException.class, () -> buf.readUnsignedMedium()); + assertThrows(BufferClosedException.class, () -> buf.readInt()); + assertThrows(BufferClosedException.class, () -> buf.readUnsignedInt()); + assertThrows(BufferClosedException.class, () -> buf.readFloat()); + assertThrows(BufferClosedException.class, () -> buf.readLong()); + assertThrows(BufferClosedException.class, () -> buf.readDouble()); - assertThrows(UnsupportedOperationException.class, () -> buf.getByte(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getUnsignedByte(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getChar(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getShort(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getUnsignedShort(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getMedium(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getUnsignedMedium(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getInt(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getUnsignedInt(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getFloat(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getLong(0)); - assertThrows(UnsupportedOperationException.class, () -> buf.getDouble(0)); + assertThrows(BufferClosedException.class, () -> buf.getByte(0)); + assertThrows(BufferClosedException.class, () -> buf.getUnsignedByte(0)); + assertThrows(BufferClosedException.class, () -> buf.getChar(0)); + assertThrows(BufferClosedException.class, () -> buf.getShort(0)); + assertThrows(BufferClosedException.class, () -> buf.getUnsignedShort(0)); + assertThrows(BufferClosedException.class, () -> buf.getMedium(0)); + assertThrows(BufferClosedException.class, () -> buf.getUnsignedMedium(0)); + assertThrows(BufferClosedException.class, () -> buf.getInt(0)); + assertThrows(BufferClosedException.class, () -> buf.getUnsignedInt(0)); + assertThrows(BufferClosedException.class, () -> buf.getFloat(0)); + assertThrows(BufferClosedException.class, () -> buf.getLong(0)); + assertThrows(BufferClosedException.class, () -> buf.getDouble(0)); } public static void verifyWriteInaccessible(Buffer buf, Class expected) { @@ -403,8 +402,7 @@ public abstract class BufferTestSupport { assertThrows(expected, () -> buf.setDouble(0, 32)); assertThrows(expected, () -> buf.ensureWritable(1)); - final RuntimeException f1e = assertThrows(RuntimeException.class, () -> buf.fill((byte) 0)); - assertTrue(f1e instanceof IllegalStateException || f1e instanceof UnsupportedOperationException); + assertThrows(expected, () -> buf.fill((byte) 0)); try (BufferAllocator allocator = BufferAllocator.heap(); Buffer source = allocator.allocate(8)) { assertThrows(expected, () -> source.copyInto(0, buf, 0, 1));