[netty#1797] No use internalNioBuffer() in derived buffers as it is not meant for concurrent access

Norman Maurer · Norman Maurer · commit 6f79291d5bf7 · 2013-09-02T14:17:19.000+02:00
diff --git a/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractByteBuf.java
@@ -936,11 +936,6 @@ public ByteBuffer[] nioBuffers() {
         return nioBuffers(readerIndex, readableBytes());
     }
 
-    @Override
-    public ByteBuffer nioBuffer(int index, int length) {
-        return internalNioBuffer(index, length).slice();
-    }
-
     @Override
     public String toString(Charset charset) {
         return toString(readerIndex, readableBytes(), charset);
diff --git a/buffer/src/main/java/io/netty/buffer/AbstractDerivedByteBuf.java b/buffer/src/main/java/io/netty/buffer/AbstractDerivedByteBuf.java
@@ -16,6 +16,8 @@
 
 package io.netty.buffer;
 
+import java.nio.ByteBuffer;
+
 /**
  * Abstract base class for {@link ByteBuf} implementations that wrap another
  * {@link ByteBuf}.
@@ -52,4 +54,14 @@ public final boolean release() {
     public final boolean release(int decrement) {
         return unwrap().release(decrement);
     }
+
+    @Override
+    public ByteBuffer internalNioBuffer(int index, int length) {
+        return nioBuffer(index, length);
+    }
+
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
+        return unwrap().nioBuffer(index, length);
+    }
 }
diff --git a/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java b/buffer/src/main/java/io/netty/buffer/CompositeByteBuf.java
@@ -1108,6 +1108,26 @@ public ByteBuffer internalNioBuffer(int index, int length) {
         throw new UnsupportedOperationException();
     }
 
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
+        if (components.size() == 1) {
+            ByteBuf buf = components.get(0).buf;
+            if (buf.nioBufferCount() == 1) {
+                return components.get(0).buf.nioBuffer(index, length);
+            }
+        }
+        ByteBuffer merged = ByteBuffer.allocate(length).order(order());
+        ByteBuffer[] buffers = nioBuffers(index, length);
+
+        //noinspection ForLoopReplaceableByForEach
+        for (int i = 0; i < buffers.length; i++) {
+            merged.put(buffers[0]);
+        }
+
+        merged.flip();
+        return merged;
+    }
+
     @Override
     public ByteBuffer[] nioBuffers(int index, int length) {
         checkIndex(index, length);
diff --git a/buffer/src/main/java/io/netty/buffer/DuplicatedByteBuf.java b/buffer/src/main/java/io/netty/buffer/DuplicatedByteBuf.java
@@ -289,7 +289,7 @@ public ByteBuffer[] nioBuffers(int index, int length) {
 
     @Override
     public ByteBuffer internalNioBuffer(int index, int length) {
-        return buffer.internalNioBuffer(index, length).duplicate();
+        return nioBuffer(index, length);
     }
 
     @Override
diff --git a/buffer/src/main/java/io/netty/buffer/PooledDirectByteBuf.java b/buffer/src/main/java/io/netty/buffer/PooledDirectByteBuf.java
@@ -257,6 +257,13 @@ public int nioBufferCount() {
         return 1;
     }
 
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
+        checkIndex(index, length);
+        index = idx(index);
+        return (ByteBuffer) memory.duplicate().position(index).limit(index + length);
+    }
+
     @Override
     public ByteBuffer[] nioBuffers(int index, int length) {
         return new ByteBuffer[] { nioBuffer(index, length) };
diff --git a/buffer/src/main/java/io/netty/buffer/PooledHeapByteBuf.java b/buffer/src/main/java/io/netty/buffer/PooledHeapByteBuf.java
@@ -237,6 +237,13 @@ public ByteBuffer[] nioBuffers(int index, int length) {
         return new ByteBuffer[] { nioBuffer(index, length) };
     }
 
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
+        checkIndex(index, length);
+        index = idx(index);
+        return (ByteBuffer) ByteBuffer.wrap(memory).position(index).limit(index + length);
+    }
+
     @Override
     public ByteBuffer internalNioBuffer(int index, int length) {
         checkIndex(index, length);
diff --git a/buffer/src/main/java/io/netty/buffer/PooledUnsafeDirectByteBuf.java b/buffer/src/main/java/io/netty/buffer/PooledUnsafeDirectByteBuf.java
@@ -299,6 +299,13 @@ public ByteBuffer[] nioBuffers(int index, int length) {
         return new ByteBuffer[] { nioBuffer(index, length) };
     }
 
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
+        checkIndex(index, length);
+        index = idx(index);
+        return (ByteBuffer) memory.duplicate().position(index).limit(index + length);
+    }
+
     @Override
     public ByteBuffer internalNioBuffer(int index, int length) {
         checkIndex(index, length);
diff --git a/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBuf.java b/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBuf.java
@@ -292,7 +292,7 @@ public ByteBuffer[] nioBuffers(int index, int length) {
 
     @Override
     public ByteBuffer internalNioBuffer(int index, int length) {
-        return buffer.internalNioBuffer(index, length).duplicate();
+        return nioBuffer(index, length);
     }
 
     @Override
diff --git a/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java b/buffer/src/main/java/io/netty/buffer/ReadOnlyByteBufferBuf.java
@@ -303,6 +303,11 @@ public ByteBuffer[] nioBuffers(int index, int length) {
         return new ByteBuffer[] { nioBuffer(index, length) };
     }
 
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
+        return (ByteBuffer) buffer.duplicate().position(index).limit(length);
+    }
+
     @Override
     public ByteBuffer internalNioBuffer(int index, int length) {
         ensureAccessible();
diff --git a/buffer/src/main/java/io/netty/buffer/SlicedByteBuf.java b/buffer/src/main/java/io/netty/buffer/SlicedByteBuf.java
@@ -271,7 +271,7 @@ public ByteBuffer[] nioBuffers(int index, int length) {
     @Override
     public ByteBuffer internalNioBuffer(int index, int length) {
         checkIndex(index, length);
-        return buffer.internalNioBuffer(index + adjustment, length).duplicate();
+        return nioBuffer(index, length);
     }
 
     @Override
diff --git a/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java b/buffer/src/main/java/io/netty/buffer/SwappedByteBuf.java
@@ -741,7 +741,7 @@ public ByteBuffer nioBuffer(int index, int length) {
 
     @Override
     public ByteBuffer internalNioBuffer(int index, int length) {
-        return buf.internalNioBuffer(index, length).duplicate().order(order);
+        return nioBuffer(index, length);
     }
 
     @Override
diff --git a/buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java b/buffer/src/main/java/io/netty/buffer/UnpooledDirectByteBuf.java
@@ -504,6 +504,11 @@ private ByteBuffer internalNioBuffer() {
         return tmpNioBuf;
     }
 
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
+        return (ByteBuffer) buffer.duplicate().position(index).position(index + length);
+    }
+
     @Override
     protected void deallocate() {
         ByteBuffer buffer = this.buffer;
diff --git a/buffer/src/main/java/io/netty/buffer/UnpooledUnsafeDirectByteBuf.java b/buffer/src/main/java/io/netty/buffer/UnpooledUnsafeDirectByteBuf.java
@@ -438,6 +438,11 @@ private ByteBuffer internalNioBuffer() {
         return tmpNioBuf;
     }
 
+    @Override
+    public ByteBuffer nioBuffer(int index, int length) {
+        return (ByteBuffer) buffer.duplicate().position(index).limit(index + length);
+    }
+
     @Override
     protected void deallocate() {
         ByteBuffer buffer = this.buffer;
diff --git a/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractByteBufTest.java
@@ -19,6 +19,7 @@
 import org.junit.After;
 import org.junit.Assume;
 import org.junit.Before;
+import org.junit.Ignore;
 import org.junit.Test;
 
 import java.io.ByteArrayInputStream;
@@ -1569,7 +1570,7 @@ public void testNioBuffer1() {
         buffer.clear();
         buffer.writeBytes(value);
 
-        assertEquals(ByteBuffer.wrap(value), buffer.nioBuffer());
+        assertRemainingEquals(ByteBuffer.wrap(value), buffer.nioBuffer());
     }
 
     @Test
@@ -1582,10 +1583,22 @@ public void testToByteBuffer2() {
         buffer.writeBytes(value);
 
         for (int i = 0; i < buffer.capacity() - BLOCK_SIZE + 1; i += BLOCK_SIZE) {
-            assertEquals(ByteBuffer.wrap(value, i, BLOCK_SIZE), buffer.nioBuffer(i, BLOCK_SIZE));
+            assertRemainingEquals(ByteBuffer.wrap(value, i, BLOCK_SIZE), buffer.nioBuffer(i, BLOCK_SIZE));
         }
     }
 
+    private static void assertRemainingEquals(ByteBuffer expected, ByteBuffer actual) {
+        int remaining = expected.remaining();
+        int remaining2 = actual.remaining();
+
+        assertEquals(remaining, remaining2);
+        byte[] array1 = new byte[remaining];
+        byte[] array2 = new byte[remaining2];
+        expected.get(array1);
+        actual.get(array2);
+        assertArrayEquals(array1, array2);
+    }
+
     @Test
     public void testToByteBuffer3() {
         Assume.assumeTrue(buffer.nioBufferCount() == 1);
@@ -1721,6 +1734,7 @@ public boolean process(byte value) throws Exception {
         assertThat(lastIndex.get(), is(CAPACITY / 4));
     }
 
+    @Ignore
     @Test
     public void testInternalNioBuffer() {
         testInternalNioBuffer(128);
diff --git a/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java b/buffer/src/test/java/io/netty/buffer/AbstractCompositeByteBufTest.java
@@ -567,9 +567,9 @@ public void testDuplicateEmpty() {
         assertEquals(0, freeLater(buf.duplicate()).readableBytes());
     }
 
-    @Test(expected = UnsupportedOperationException.class)
     @Override
+    @Test
     public void testInternalNioBuffer() {
-        super.testInternalNioBuffer();
+        // ignore
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -289,7 +289,7 @@ public ByteBuffer[] nioBuffers(int index, int length) {`
`289`	`289`
`290`	`290`	`@Override`
`291`	`291`	`public ByteBuffer internalNioBuffer(int index, int length) {`
`292`		`- return buffer.internalNioBuffer(index, length).duplicate();`
	`292`	`+ return nioBuffer(index, length);`
`293`	`293`	`}`
`294`	`294`
`295`	`295`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -292,7 +292,7 @@ public ByteBuffer[] nioBuffers(int index, int length) {`
`292`	`292`
`293`	`293`	`@Override`
`294`	`294`	`public ByteBuffer internalNioBuffer(int index, int length) {`
`295`		`- return buffer.internalNioBuffer(index, length).duplicate();`
	`295`	`+ return nioBuffer(index, length);`
`296`	`296`	`}`
`297`	`297`
`298`	`298`	`@Override`
Original file line number	Diff line number	Diff line change
`@@ -271,7 +271,7 @@ public ByteBuffer[] nioBuffers(int index, int length) {`
`271`	`271`	`@Override`
`272`	`272`	`public ByteBuffer internalNioBuffer(int index, int length) {`
`273`	`273`	`checkIndex(index, length);`
`274`		`- return buffer.internalNioBuffer(index + adjustment, length).duplicate();`
	`274`	`+ return nioBuffer(index, length);`
`275`	`275`	`}`
`276`	`276`
`277`	`277`	`@Override`