From a2bd9a4399929f862ae5c6d773b72e4915b1c682 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Wed, 7 Jun 2017 17:12:19 +0200 Subject: [PATCH] Correctly handle ByteBuf implementations which have no memoryAddress when writing to native transport Motivation: Commit 3c4dfed08ae2ce300f6ac1d0794d42a14c344b79 introduced a regression in handling buffers that have no memoryAddress. Modifications: Fix regression and also add unit tests. Result: It's possible again to write buffers without memory address. --- .../transport/socket/DatagramUnicastTest.java | 25 ++++++++++++------- .../netty/channel/epoll/UnixChannelUtil.java | 2 +- .../channel/epoll/UnixChannelUtilTest.java | 3 +++ 3 files changed, 20 insertions(+), 10 deletions(-) diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/DatagramUnicastTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/DatagramUnicastTest.java index c5701e8360..b234bf5f81 100644 --- a/testsuite/src/main/java/io/netty/testsuite/transport/socket/DatagramUnicastTest.java +++ b/testsuite/src/main/java/io/netty/testsuite/transport/socket/DatagramUnicastTest.java @@ -37,7 +37,7 @@ public class DatagramUnicastTest extends AbstractDatagramTest { private static final byte[] BYTES = {0, 1, 2, 3}; private enum WrapType { - NONE, DUP, SLICE, + NONE, DUP, SLICE, READ_ONLY } @Test @@ -189,14 +189,21 @@ public class DatagramUnicastTest extends AbstractDatagramTest { } for (int i = 0; i < count; i++) { - if (wrapType == WrapType.DUP) { - cc.write(new DatagramPacket(buf.retain().duplicate(), addr)); - } else if (wrapType == WrapType.SLICE) { - cc.write(new DatagramPacket(buf.retain().slice(), addr)); - } else if (wrapType == WrapType.NONE) { - cc.write(new DatagramPacket(buf.retain(), addr)); - } else { - throw new Exception("unknown wrap type: " + wrapType); + switch (wrapType) { + case DUP: + cc.write(new DatagramPacket(buf.retain().duplicate(), addr)); + break; + case SLICE: + cc.write(new DatagramPacket(buf.retain().slice(), addr)); + break; + case READ_ONLY: + cc.write(new DatagramPacket(Unpooled.unmodifiableBuffer(buf.retain()), addr)); + break; + case NONE: + cc.write(new DatagramPacket(buf.retain(), addr)); + break; + default: + throw new Error("unknown wrap type: " + wrapType); } } // release as we used buf.retain() before diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/UnixChannelUtil.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/UnixChannelUtil.java index 713b85d7c4..25fc4e376c 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/UnixChannelUtil.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/UnixChannelUtil.java @@ -27,6 +27,6 @@ public final class UnixChannelUtil { * (We check this because otherwise we need to make it a new direct buffer.) */ public static boolean isBufferCopyNeededForWrite(ByteBuf byteBuf) { - return !(byteBuf.hasMemoryAddress() || byteBuf.isDirect() && byteBuf.nioBufferCount() <= Native.IOV_MAX); + return !byteBuf.hasMemoryAddress() || !byteBuf.isDirect() || byteBuf.nioBufferCount() > Native.IOV_MAX; } } diff --git a/transport-native-epoll/src/test/java/io/netty/channel/epoll/UnixChannelUtilTest.java b/transport-native-epoll/src/test/java/io/netty/channel/epoll/UnixChannelUtilTest.java index ba5a098fa5..b3edc61d0e 100644 --- a/transport-native-epoll/src/test/java/io/netty/channel/epoll/UnixChannelUtilTest.java +++ b/transport-native-epoll/src/test/java/io/netty/channel/epoll/UnixChannelUtilTest.java @@ -47,10 +47,13 @@ public class UnixChannelUtilTest { private static void testIsBufferCopyNeededForWrite(ByteBufAllocator alloc) { ByteBuf byteBuf = alloc.directBuffer(); assertFalse(isBufferCopyNeededForWrite(byteBuf)); + assertTrue(isBufferCopyNeededForWrite(byteBuf.asReadOnly())); + assertTrue(byteBuf.release()); byteBuf = alloc.heapBuffer(); assertTrue(isBufferCopyNeededForWrite(byteBuf)); + assertTrue(isBufferCopyNeededForWrite(byteBuf.asReadOnly())); assertTrue(byteBuf.release()); assertCompositeByteBufIsBufferCopyNeededForWrite(alloc, 2, 0, false);