Skip to content

Commit

Permalink
AbstractByteBuf.ensureWritable(...) should check if buffer was released
Browse files Browse the repository at this point in the history 
Motivation:

AbstractByteBuf.ensureWritable(...) should check if buffer was released and if so throw an IllegalReferenceCountException

Modifications:

Ensure we throw in all cases.

Result:

More consistent and correct behaviour
  • Loading branch information
@normanmaurer
normanmaurer committed Jul 19, 2017
1 parent 96e06aa commit d125ade
Show file tree
Hide file tree
Showing 10 changed files with 26 additions and 70 deletions.
29 changes: 8 additions & 21 deletions buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
View file
Expand Up @@ -266,7 +266,8 @@ public ByteBuf ensureWritable(int minWritableBytes) {
return this;
}

private void ensureWritable0(int minWritableBytes) {
final void ensureWritable0(int minWritableBytes) {
ensureAccessible();
if (minWritableBytes <= writableBytes()) {
return;
}
Expand All @@ -286,6 +287,7 @@ private void ensureWritable0(int minWritableBytes) {

@Override
public int ensureWritable(int minWritableBytes, boolean force) {
ensureAccessible();
if (minWritableBytes < 0) {
throw new IllegalArgumentException(String.format(
"minWritableBytes: %d (expected: >= 0)", minWritableBytes));
Expand Down Expand Up @@ -668,16 +670,16 @@ public ByteBuf setZero(int index, int length) {
@Override
public int setCharSequence(int index, CharSequence sequence, Charset charset) {
if (charset.equals(CharsetUtil.UTF_8)) {
ensureWritable(ByteBufUtil.utf8MaxBytes(sequence));
ensureWritable0(ByteBufUtil.utf8MaxBytes(sequence));
return ByteBufUtil.writeUtf8(this, index, sequence, sequence.length());
}
if (charset.equals(CharsetUtil.US_ASCII) || charset.equals(CharsetUtil.ISO_8859_1)) {
int len = sequence.length();
ensureWritable(len);
ensureWritable0(len);
return ByteBufUtil.writeAscii(this, index, sequence, len);
}
byte[] bytes = sequence.toString().getBytes(charset);
ensureWritable(bytes.length);
ensureWritable0(bytes.length);
setBytes(index, bytes);
return bytes.length;
}
Expand Down Expand Up @@ -934,15 +936,13 @@ public ByteBuf writeBoolean(boolean value) {

@Override
public ByteBuf writeByte(int value) {
ensureAccessible();
ensureWritable0(1);
_setByte(writerIndex++, value);
return this;
}

@Override
public ByteBuf writeShort(int value) {
ensureAccessible();
ensureWritable0(2);
_setShort(writerIndex, value);
writerIndex += 2;
Expand All @@ -951,7 +951,6 @@ public ByteBuf writeShort(int value) {

@Override
public ByteBuf writeShortLE(int value) {
ensureAccessible();
ensureWritable0(2);
_setShortLE(writerIndex, value);
writerIndex += 2;
Expand All @@ -960,7 +959,6 @@ public ByteBuf writeShortLE(int value) {

@Override
public ByteBuf writeMedium(int value) {
ensureAccessible();
ensureWritable0(3);
_setMedium(writerIndex, value);
writerIndex += 3;
Expand All @@ -969,7 +967,6 @@ public ByteBuf writeMedium(int value) {

@Override
public ByteBuf writeMediumLE(int value) {
ensureAccessible();
ensureWritable0(3);
_setMediumLE(writerIndex, value);
writerIndex += 3;
Expand All @@ -978,7 +975,6 @@ public ByteBuf writeMediumLE(int value) {

@Override
public ByteBuf writeInt(int value) {
ensureAccessible();
ensureWritable0(4);
_setInt(writerIndex, value);
writerIndex += 4;
Expand All @@ -987,7 +983,6 @@ public ByteBuf writeInt(int value) {

@Override
public ByteBuf writeIntLE(int value) {
ensureAccessible();
ensureWritable0(4);
_setIntLE(writerIndex, value);
writerIndex += 4;
Expand All @@ -996,7 +991,6 @@ public ByteBuf writeIntLE(int value) {

@Override
public ByteBuf writeLong(long value) {
ensureAccessible();
ensureWritable0(8);
_setLong(writerIndex, value);
writerIndex += 8;
Expand All @@ -1005,7 +999,6 @@ public ByteBuf writeLong(long value) {

@Override
public ByteBuf writeLongLE(long value) {
ensureAccessible();
ensureWritable0(8);
_setLongLE(writerIndex, value);
writerIndex += 8;
Expand All @@ -1032,7 +1025,6 @@ public ByteBuf writeDouble(double value) {

@Override
public ByteBuf writeBytes(byte[] src, int srcIndex, int length) {
ensureAccessible();
ensureWritable(length);
setBytes(writerIndex, src, srcIndex, length);
writerIndex += length;
Expand Down Expand Up @@ -1064,7 +1056,6 @@ public ByteBuf writeBytes(ByteBuf src, int length) {

@Override
public ByteBuf writeBytes(ByteBuf src, int srcIndex, int length) {
ensureAccessible();
ensureWritable(length);
setBytes(writerIndex, src, srcIndex, length);
writerIndex += length;
Expand All @@ -1073,9 +1064,8 @@ public ByteBuf writeBytes(ByteBuf src, int srcIndex, int length) {

@Override
public ByteBuf writeBytes(ByteBuffer src) {
ensureAccessible();
int length = src.remaining();
ensureWritable(length);
ensureWritable0(length);
setBytes(writerIndex, src);
writerIndex += length;
return this;
Expand All @@ -1084,7 +1074,6 @@ public ByteBuf writeBytes(ByteBuffer src) {
@Override
public int writeBytes(InputStream in, int length)
throws IOException {
ensureAccessible();
ensureWritable(length);
int writtenBytes = setBytes(writerIndex, in, length);
if (writtenBytes > 0) {
Expand All @@ -1095,7 +1084,6 @@ public int writeBytes(InputStream in, int length)

@Override
public int writeBytes(ScatteringByteChannel in, int length) throws IOException {
ensureAccessible();
ensureWritable(length);
int writtenBytes = setBytes(writerIndex, in, length);
if (writtenBytes > 0) {
Expand All @@ -1106,7 +1094,6 @@ public int writeBytes(ScatteringByteChannel in, int length) throws IOException {

@Override
public int writeBytes(FileChannel in, long position, int length) throws IOException {
ensureAccessible();
ensureWritable(length);
int writtenBytes = setBytes(writerIndex, in, position, length);
if (writtenBytes > 0) {
Expand All @@ -1123,7 +1110,7 @@ public ByteBuf writeZero(int length) {

ensureWritable(length);
int wIndex = writerIndex;
checkIndex(wIndex, length);
checkIndex0(wIndex, length);

int nLong = length >>> 3;
int nBytes = length & 7;
Expand Down
6 changes: 3 additions & 3 deletions buffer/src/main/java/io/netty/buffer/AbstractUnsafeSwappedByteBuf.java
View file
Expand Up @@ -122,23 +122,23 @@ public final ByteBuf setDouble(int index, double value) {

@Override
public final ByteBuf writeShort(int value) {
wrapped.ensureWritable(2);
wrapped.ensureWritable0(2);
_setShort(wrapped, wrapped.writerIndex, nativeByteOrder ? (short) value : Short.reverseBytes((short) value));
wrapped.writerIndex += 2;
return this;
}

@Override
public final ByteBuf writeInt(int value) {
wrapped.ensureWritable(4);
wrapped.ensureWritable0(4);
_setInt(wrapped, wrapped.writerIndex, nativeByteOrder ? value : Integer.reverseBytes(value));
wrapped.writerIndex += 4;
return this;
}

@Override
public final ByteBuf writeLong(long value) {
wrapped.ensureWritable(8);
wrapped.ensureWritable0(8);
_setLong(wrapped, wrapped.writerIndex, nativeByteOrder ? value : Long.reverseBytes(value));
wrapped.writerIndex += 8;
return this;
Expand Down
4 changes: 2 additions & 2 deletions buffer/src/main/java/io/netty/buffer/ByteBufUtil.java
View file
Expand Up @@ -482,7 +482,7 @@ public static int writeUtf8(ByteBuf buf, CharSequence seq) {
for (;;) {
if (buf instanceof AbstractByteBuf) {
AbstractByteBuf byteBuf = (AbstractByteBuf) buf;
byteBuf.ensureWritable(utf8MaxBytes(seq));
byteBuf.ensureWritable0(utf8MaxBytes(seq));
int written = writeUtf8(byteBuf, byteBuf.writerIndex, seq, seq.length());
byteBuf.writerIndex += written;
return written;
Expand Down Expand Up @@ -583,7 +583,7 @@ public static int writeAscii(ByteBuf buf, CharSequence seq) {
for (;;) {
if (buf instanceof AbstractByteBuf) {
AbstractByteBuf byteBuf = (AbstractByteBuf) buf;
byteBuf.ensureWritable(len);
byteBuf.ensureWritable0(len);
int written = writeAscii(byteBuf, byteBuf.writerIndex, seq, len);
byteBuf.writerIndex += written;
return written;
Expand Down
5 changes: 3 additions & 2 deletions buffer/src/main/java/io/netty/buffer/PooledUnsafeDirectByteBuf.java
View file
Expand Up @@ -374,15 +374,16 @@ protected SwappedByteBuf newSwappedByteBuf() {

@Override
public ByteBuf setZero(int index, int length) {
UnsafeByteBufUtil.setZero(this, addr(index), index, length);
checkIndex(index, length);
UnsafeByteBufUtil.setZero(addr(index), length);
return this;
}

@Override
public ByteBuf writeZero(int length) {
ensureWritable(length);
int wIndex = writerIndex;
setZero(wIndex, length);
UnsafeByteBufUtil.setZero(addr(wIndex), length);
writerIndex = wIndex + length;
return this;
}
Expand Down
10 changes: 3 additions & 7 deletions buffer/src/main/java/io/netty/buffer/PooledUnsafeHeapByteBuf.java
View file
Expand Up @@ -131,8 +131,9 @@ protected void _setLongLE(int index, long value) {
@Override
public ByteBuf setZero(int index, int length) {
if (PlatformDependent.javaVersion() >= 7) {
checkIndex(index, length);
// Only do on java7+ as the needed Unsafe call was only added there.
_setZero(index, length);
UnsafeByteBufUtil.setZero(memory, idx(index), length);
return this;
}
return super.setZero(index, length);
Expand All @@ -144,18 +145,13 @@ public ByteBuf writeZero(int length) {
// Only do on java7+ as the needed Unsafe call was only added there.
ensureWritable(length);
int wIndex = writerIndex;
_setZero(wIndex, length);
UnsafeByteBufUtil.setZero(memory, idx(wIndex), length);
writerIndex = wIndex + length;
return this;
}
return super.writeZero(length);
}

private void _setZero(int index, int length) {
checkIndex(index, length);
UnsafeByteBufUtil.setZero(memory, idx(index), length);
}

@Override
@Deprecated
protected SwappedByteBuf newSwappedByteBuf() {
Expand Down
5 changes: 3 additions & 2 deletions buffer/src/main/java/io/netty/buffer/UnpooledUnsafeDirectByteBuf.java
View file
Expand Up @@ -517,15 +517,16 @@ protected SwappedByteBuf newSwappedByteBuf() {

@Override
public ByteBuf setZero(int index, int length) {
UnsafeByteBufUtil.setZero(this, addr(index), index, length);
checkIndex(index, length);
UnsafeByteBufUtil.setZero(addr(index), length);
return this;
}

@Override
public ByteBuf writeZero(int length) {
ensureWritable(length);
int wIndex = writerIndex;
setZero(wIndex, length);
UnsafeByteBufUtil.setZero(addr(wIndex), length);
writerIndex = wIndex + length;
return this;
}
Expand Down
10 changes: 3 additions & 7 deletions buffer/src/main/java/io/netty/buffer/UnpooledUnsafeHeapByteBuf.java
View file
Expand Up @@ -245,7 +245,8 @@ protected void _setLongLE(int index, long value) {
public ByteBuf setZero(int index, int length) {
if (PlatformDependent.javaVersion() >= 7) {
// Only do on java7+ as the needed Unsafe call was only added there.
_setZero(index, length);
checkIndex(index, length);
UnsafeByteBufUtil.setZero(array, index, length);
return this;
}
return super.setZero(index, length);
Expand All @@ -257,18 +258,13 @@ public ByteBuf writeZero(int length) {
// Only do on java7+ as the needed Unsafe call was only added there.
ensureWritable(length);
int wIndex = writerIndex;
_setZero(wIndex, length);
UnsafeByteBufUtil.setZero(array, wIndex, length);
writerIndex = wIndex + length;
return this;
}
return super.writeZero(length);
}

private void _setZero(int index, int length) {
checkIndex(index, length);
UnsafeByteBufUtil.setZero(array, index, length);
}

@Override
@Deprecated
protected SwappedByteBuf newSwappedByteBuf() {
Expand Down
3 changes: 1 addition & 2 deletions buffer/src/main/java/io/netty/buffer/UnsafeByteBufUtil.java
View file
Expand Up @@ -581,12 +581,11 @@ static void getBytes(AbstractByteBuf buf, long addr, int index, OutputStream out
}
}

static void setZero(AbstractByteBuf buf, long addr, int index, int length) {
static void setZero(long addr, int length) {
if (length == 0) {
return;
}

buf.checkIndex(index, length);
PlatformDependent.setMemory(addr, length, ZERO);
}

Expand Down
12 changes: 0 additions & 12 deletions buffer/src/test/java/io/netty/buffer/SlicedByteBufTest.java
View file
Expand Up @@ -94,18 +94,6 @@ public void testNioBufferExposeOnlyRegion() {
super.testNioBufferExposeOnlyRegion();
}

@Test(expected = IndexOutOfBoundsException.class)
@Override
public void testEnsureWritableAfterRelease() {
super.testEnsureWritableAfterRelease();
}

@Test(expected = IndexOutOfBoundsException.class)
@Override
public void testWriteZeroAfterRelease() throws IOException {
super.testWriteZeroAfterRelease();
}

@Test(expected = IndexOutOfBoundsException.class)
@Override
public void testGetReadOnlyDirectDst() {
Expand Down
12 changes: 0 additions & 12 deletions buffer/src/test/java/io/netty/buffer/WrappedUnpooledUnsafeByteBufTest.java
View file
Expand Up @@ -88,18 +88,6 @@ public void testNioBufferExposeOnlyRegion() {
super.testNioBufferExposeOnlyRegion();
}

@Test(expected = IndexOutOfBoundsException.class)
@Override
public void testEnsureWritableAfterRelease() {
super.testEnsureWritableAfterRelease();
}

@Test(expected = IndexOutOfBoundsException.class)
@Override
public void testWriteZeroAfterRelease() throws IOException {
super.testWriteZeroAfterRelease();
}

@Test(expected = IndexOutOfBoundsException.class)
@Override
public void testGetReadOnlyDirectDst() {
Expand Down

0 comments on commit d125ade

Please sign in to comment.