reaktivity / nukleus-tcp.java

TCP Nukleus Implementation
Apache License 2.0
1 stars 8 forks source link

Must catch IOException from channel.read and channel.write to avoid repeated exception reporting from the reaktor #9

Closed cmebarrow closed 7 years ago

cmebarrow commented 7 years ago

I accidentally hit this while working on a test. The console output was:

java.io.IOException: An established connection was aborted by the software in your host machine
    at sun.nio.ch.SocketDispatcher.read0(Native Method)
    at sun.nio.ch.SocketDispatcher.read(SocketDispatcher.java:43)
    at sun.nio.ch.IOUtil.readIntoNativeBuffer(IOUtil.java:223)
    at sun.nio.ch.IOUtil.read(IOUtil.java:197)
    at sun.nio.ch.SocketChannelImpl.read(SocketChannelImpl.java:380)
    at org.reaktivity.nukleus.tcp.internal.reader.stream.StreamFactory$Stream.handleRead(StreamFactory.java:113)
    at org.reaktivity.nukleus.tcp.internal.reader.stream.StreamFactory$Stream.handleStream(StreamFactory.java:89)
    at org.reaktivity.nukleus.tcp.internal.reader.stream.StreamFactory$Stream.access$2(StreamFactory.java:85)
    at org.reaktivity.nukleus.tcp.internal.reader.stream.StreamFactory$$Lambda$52/2030708124.getAsInt(Unknown Source)
    at org.reaktivity.nukleus.tcp.internal.reader.Source.processKey(Source.java:105)
    at org.reaktivity.nukleus.tcp.internal.reader.Source$$Lambda$36/749126367.applyAsInt(Unknown Source)
    at org.agrona.nio.NioSelectedKeySet.forEach(NioSelectedKeySet.java:128)
    at org.reaktivity.nukleus.tcp.internal.reader.Source.process(Source.java:65)
    at org.reaktivity.nukleus.Nukleus$Composite.process(Nukleus.java:52)
    at org.reaktivity.nukleus.Nukleus$Composite.process(Nukleus.java:52)
    at org.reaktivity.nukleus.Nukleus$Composite.process(Nukleus.java:52)
    at org.reaktivity.reaktor.test.NukleusRule$1.lambda$0(NukleusRule.java:166)
    at org.reaktivity.reaktor.test.NukleusRule$1$$Lambda$12/1627960023.run(Unknown Source)
    at java.lang.Thread.run(Thread.java:745)
java.io.IOException: An established connection was aborted by the software in your host machine
    at sun.nio.ch.SocketDispatcher.read0(Native Method)
    at sun.nio.ch.SocketDispatcher.read(SocketDispatcher.java:43)
    at sun.nio.ch.IOUtil.readIntoNativeBuffer(IOUtil.java:223)
    at sun.nio.ch.IOUtil.read(IOUtil.java:197)
etc (repeated 510 times)

This can be reproduced by adding the following test case to ServerIT and running it several times (the problem does not always occur, it obviously depends on timing):

    @Test
    @Specification({
        "${route}/input/new/controller",
        "${streams}/server.sent.data/server/target"
    })
    public void shouldNotGetRepeatedIOExceptionsFromReaderStreamRead() throws Exception
    {
        k3po.start();
        k3po.awaitBarrier("ROUTED_INPUT");

        try(Socket socket = new Socket("127.0.0.1", 0x1f90))
        {
            socket.shutdownInput();
            Thread.sleep(500);
        }

        k3po.finish();
    }
cmebarrow commented 7 years ago

We should not report this condition like an unexpected error (as we do other exceptions caught in the reaktor) because it is a frequent condition that clients abruptly close a connection (e.g. mobile phones). We should report it in a separate counter. The same applies for subsequent attempts to write to the channel on which the IOException (RST) occurred.

cmebarrow commented 7 years ago

Looking at the jdk source code and the Microsoft Windows Socket API documentation there is no reliable way to distinguish an IOException from SocketChannel.read due to client sending a RST from any other one. Proposed solution from discussion with John is:

Testing shows any subsequent attempt to write to the socket channel will result in an exception (which will be reported as a RESET frame to the writer). It doesn't appear there is any value in immediately reporting a RESET to the writer when we catch the exception from the read.

cmebarrow commented 7 years ago

Orderly Versus Abortive Connection Release in Java is a very useful link.

jfallows commented 7 years ago

Fix merged, closing.