From 556a3d59800ea7c5afee42fa4a56adccbb2789cf Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Fri, 20 Feb 2015 14:20:33 +0100 Subject: [PATCH] Correctly handle autoRead == false when epoll LT is used Motivation: When epoll LT is used and autoRead == false when entering epollIn() we need to return without reading any data. Modifications: Correctly respect autoRead == false if using epoll LT. Result: Consistent and correct behaviour. --- .../epoll/AbstractEpollServerChannel.java | 17 +++++++++++++---- .../epoll/AbstractEpollStreamChannel.java | 15 +++++++++++---- .../channel/epoll/EpollDatagramChannel.java | 19 +++++++++++++------ .../epoll/EpollDomainSocketChannel.java | 14 ++++++++++---- 4 files changed, 47 insertions(+), 18 deletions(-) diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollServerChannel.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollServerChannel.java index 96b18b3504..8339824de1 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollServerChannel.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollServerChannel.java @@ -16,6 +16,7 @@ package io.netty.channel.epoll; import io.netty.channel.Channel; +import io.netty.channel.ChannelConfig; import io.netty.channel.ChannelOutboundBuffer; import io.netty.channel.ChannelPipeline; import io.netty.channel.ChannelPromise; @@ -75,14 +76,22 @@ public abstract class AbstractEpollServerChannel extends AbstractEpollChannel im @Override void epollInReady() { assert eventLoop().inEventLoop(); + boolean edgeTriggered = isFlagSet(Native.EPOLLET); + + final ChannelConfig config = config(); + if (!readPending && !edgeTriggered && !config.isAutoRead()) { + // ChannelConfig.setAutoRead(false) was called in the meantime + clearEpollIn0(); + return; + } + final ChannelPipeline pipeline = pipeline(); Throwable exception = null; try { try { - boolean edgeTriggered = isFlagSet(Native.EPOLLET); // if edgeTriggered is used we need to read all messages as we are not notified again otherwise. final int maxMessagesPerRead = edgeTriggered - ? Integer.MAX_VALUE : config().getMaxMessagesPerRead(); + ? Integer.MAX_VALUE : config.getMaxMessagesPerRead(); int messages = 0; do { int socketFd = Native.accept(fd().intValue()); @@ -99,7 +108,7 @@ public abstract class AbstractEpollServerChannel extends AbstractEpollChannel im pipeline.fireChannelReadComplete(); pipeline.fireExceptionCaught(t); } finally { - if (!edgeTriggered && !config().isAutoRead()) { + if (!edgeTriggered && !config.isAutoRead()) { // This is not using EPOLLET so we can stop reading // ASAP as we will get notified again later with // pending data @@ -122,7 +131,7 @@ public abstract class AbstractEpollServerChannel extends AbstractEpollChannel im // * The user called Channel.read() or ChannelHandlerContext.read() in channelReadComplete(...) method // // See https://github.com/netty/netty/issues/2254 - if (!config().isAutoRead() && !readPending) { + if (!readPending && !config.isAutoRead()) { clearEpollIn0(); } } diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollStreamChannel.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollStreamChannel.java index 1dc606b909..95d952212e 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollStreamChannel.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollStreamChannel.java @@ -579,6 +579,14 @@ public abstract class AbstractEpollStreamChannel extends AbstractEpollChannel { @Override void epollInReady() { final ChannelConfig config = config(); + boolean edgeTriggered = isFlagSet(Native.EPOLLET); + + if (!readPending && !edgeTriggered && !config.isAutoRead()) { + // ChannelConfig.setAutoRead(false) was called in the meantime + clearEpollIn0(); + return; + } + final ChannelPipeline pipeline = pipeline(); final ByteBufAllocator allocator = config.getAllocator(); RecvByteBufAllocator.Handle allocHandle = this.allocHandle; @@ -589,10 +597,9 @@ public abstract class AbstractEpollStreamChannel extends AbstractEpollChannel { ByteBuf byteBuf = null; boolean close = false; try { - boolean edgeTriggered = isFlagSet(Native.EPOLLET); // if edgeTriggered is used we need to read all messages as we are not notified again otherwise. final int maxMessagesPerRead = edgeTriggered - ? Integer.MAX_VALUE : config().getMaxMessagesPerRead(); + ? Integer.MAX_VALUE : config.getMaxMessagesPerRead(); int messages = 0; int totalReadAmount = 0; do { @@ -625,7 +632,7 @@ public abstract class AbstractEpollStreamChannel extends AbstractEpollChannel { // which might mean we drained the recv buffer completely. break; } - if (!edgeTriggered && !config().isAutoRead()) { + if (!edgeTriggered && !config.isAutoRead()) { // This is not using EPOLLET so we can stop reading // ASAP as we will get notified again later with // pending data @@ -659,7 +666,7 @@ public abstract class AbstractEpollStreamChannel extends AbstractEpollChannel { // * The user called Channel.read() or ChannelHandlerContext.read() in channelReadComplete(...) method // // See https://github.com/netty/netty/issues/2254 - if (!config.isAutoRead() && !readPending) { + if (!readPending && !config.isAutoRead()) { clearEpollIn0(); } } diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollDatagramChannel.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollDatagramChannel.java index d148af46ef..f3809865c0 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollDatagramChannel.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollDatagramChannel.java @@ -18,6 +18,7 @@ package io.netty.channel.epoll; import io.netty.buffer.ByteBuf; import io.netty.buffer.CompositeByteBuf; import io.netty.channel.AddressedEnvelope; +import io.netty.channel.ChannelConfig; import io.netty.channel.ChannelFuture; import io.netty.channel.ChannelMetadata; import io.netty.channel.ChannelOption; @@ -507,21 +508,27 @@ public final class EpollDatagramChannel extends AbstractEpollChannel implements @Override void epollInReady() { + assert eventLoop().inEventLoop(); DatagramChannelConfig config = config(); + boolean edgeTriggered = isFlagSet(Native.EPOLLET); + + if (!readPending && !edgeTriggered && !config.isAutoRead()) { + // ChannelConfig.setAutoRead(false) was called in the meantime + clearEpollIn0(); + return; + } + RecvByteBufAllocator.Handle allocHandle = this.allocHandle; if (allocHandle == null) { this.allocHandle = allocHandle = config.getRecvByteBufAllocator().newHandle(); } - assert eventLoop().inEventLoop(); - final ChannelPipeline pipeline = pipeline(); Throwable exception = null; try { - boolean edgeTriggered = isFlagSet(Native.EPOLLET); // if edgeTriggered is used we need to read all messages as we are not notified again otherwise. final int maxMessagesPerRead = edgeTriggered - ? Integer.MAX_VALUE : config().getMaxMessagesPerRead(); + ? Integer.MAX_VALUE : config.getMaxMessagesPerRead(); int messages = 0; do { ByteBuf data = null; @@ -558,7 +565,7 @@ public final class EpollDatagramChannel extends AbstractEpollChannel implements if (data != null) { data.release(); } - if (!edgeTriggered && !config().isAutoRead()) { + if (!edgeTriggered && !config.isAutoRead()) { // This is not using EPOLLET so we can stop reading // ASAP as we will get notified again later with // pending data @@ -585,7 +592,7 @@ public final class EpollDatagramChannel extends AbstractEpollChannel implements // * The user called Channel.read() or ChannelHandlerContext.read() in channelReadComplete(...) method // // See https://github.com/netty/netty/issues/2254 - if (!config().isAutoRead() && !readPending) { + if (!readPending && !config.isAutoRead()) { clearEpollIn(); } } diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollDomainSocketChannel.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollDomainSocketChannel.java index f36074d221..76917d19cb 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollDomainSocketChannel.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollDomainSocketChannel.java @@ -131,14 +131,20 @@ public final class EpollDomainSocketChannel extends AbstractEpollStreamChannel i } private void epollInReadFd() { + boolean edgeTriggered = isFlagSet(Native.EPOLLET); final ChannelConfig config = config(); + if (!readPending && !edgeTriggered && !config.isAutoRead()) { + // ChannelConfig.setAutoRead(false) was called in the meantime + clearEpollIn0(); + return; + } + final ChannelPipeline pipeline = pipeline(); try { - boolean edgeTriggered = isFlagSet(Native.EPOLLET); // if edgeTriggered is used we need to read all messages as we are not notified again otherwise. final int maxMessagesPerRead = edgeTriggered - ? Integer.MAX_VALUE : config().getMaxMessagesPerRead(); + ? Integer.MAX_VALUE : config.getMaxMessagesPerRead(); int messages = 0; do { int socketFd = Native.recvFd(fd().intValue()); @@ -158,7 +164,7 @@ public final class EpollDomainSocketChannel extends AbstractEpollStreamChannel i pipeline.fireChannelReadComplete(); pipeline.fireExceptionCaught(t); } finally { - if (!edgeTriggered && !config().isAutoRead()) { + if (!edgeTriggered && !config.isAutoRead()) { // This is not using EPOLLET so we can stop reading // ASAP as we will get notified again later with // pending data @@ -187,7 +193,7 @@ public final class EpollDomainSocketChannel extends AbstractEpollStreamChannel i // * The user called Channel.read() or ChannelHandlerContext.read() in channelReadComplete(...) method // // See https://github.com/netty/netty/issues/2254 - if (!config.isAutoRead() && !readPending) { + if (!readPending && !config.isAutoRead()) { clearEpollIn0(); } }