andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Not attempt to read from fd when channel is closed during read loop. Related to [#5031]

Browse Source 
Motivation:

We need to break out of the read loop for two reasons:

- If the input was shutdown in between (which may be the case when the user did it in the
  fireChannelRead(...) method we should not try to read again to not produce any
  miss-leading exceptions.

- If the user closes the channel we need to ensure we not try to read from it again as
  the filedescriptor may be re-used already by the OS if the system is handling a lot of
  concurrent connections and so needs a lot of filedescriptors. If not do this we risk
  reading data from a filedescriptor that belongs to another socket then the socket that
  was "wrapped" by this Channel implementation.

Modification:

Break the reading loop if the input was shutdown from within the channelRead(...) method.

Result:

No more meaningless exceptions and no risk to read data from wrong socket after the original was closed.
This commit is contained in:
Norman Maurer 2016-03-24 19:37:43 +01:00
parent 84bbbf7e09
commit 4950a523a7
2 changed files with 18 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollStreamChannel.java
View File

@ -876,6 +876,21 @@ public abstract class AbstractEpollStreamChannel extends AbstractEpollChannel im
allocHandle.incMessagesRead(1);
pipeline.fireChannelRead(byteBuf);
byteBuf = null;
if (fd().isInputShutdown()) {
// We need to do this for two reasons:
//
// - If the input was shutdown in between (which may be the case when the user did it in the
// fireChannelRead(...) method we should not try to read again to not produce any
// miss-leading exceptions.
//
// - If the user closes the channel we need to ensure we not try to read from it again as
// the filedescriptor may be re-used already by the OS if the system is handling a lot of
// concurrent connections and so needs a lot of filedescriptors. If not do this we risk
// reading data from a filedescriptor that belongs to another socket then the socket that
// was "wrapped" by this Channel implementation.
break;
}
} while (allocHandle.continueReading());
allocHandle.readComplete();

7
transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketChannelTest.java
View File

@ -367,10 +367,9 @@ public class EpollSocketChannelTest {
private static class ExceptionHandler extends ChannelInboundHandlerAdapter {
final AtomicLong count = new AtomicLong();
/**
* We expect to get 2 calls to {@link #exceptionCaught(ChannelHandlerContext, Throwable)}.
* 1 call from BuggyChannelHandler and 1 from closing the channel in this class.
* We expect to get 1 call to {@link #exceptionCaught(ChannelHandlerContext, Throwable)}.
*/
final CountDownLatch latch1 = new CountDownLatch(2);
final CountDownLatch latch1 = new CountDownLatch(1);
final CountDownLatch latch2 = new CountDownLatch(1);
@Override
@ -380,7 +379,7 @@ public class EpollSocketChannelTest {
} else {
latch2.countDown();
}
// This is expected to throw an exception!
// This should not throw any exception.
ctx.close();
}
}