Strange error handling in Grizzly 2.3.4 when connection refused?

Strange error handling in Grizzly 2.3.4 when connection refused?

Hi there,

While trying to debug one of our unit tests which is failing due to a client connection attempt being refused I came across a surprising looking piece of code. The stack trace is (latest 2.3.4 nightly):

at org.glassfish.grizzly.nio.transport.TCPNIOConnectorHandler.abortConnection(TCPNIOConnectorHandler.java:262)

at org.glassfish.grizzly.nio.transport.TCPNIOConnectorHandler.onConnectedAsync(TCPNIOConnectorHandler.java:215)

at org.glassfish.grizzly.nio.transport.TCPNIOConnectorHandler$1.completed(TCPNIOConnectorHandler.java:154)

at org.glassfish.grizzly.nio.transport.TCPNIOConnectorHandler$1.completed(TCPNIOConnectorHandler.java:150)

at org.glassfish.grizzly.nio.transport.TCPNIOConnection.onConnect(TCPNIOConnection.java:258)

at org.glassfish.grizzly.nio.transport.TCPNIOTransport.fireIOEvent(TCPNIOTransport.java:826)

at org.glassfish.grizzly.strategies.AbstractIOStrategy.fireIOEvent(AbstractIOStrategy.java:113)

at org.glassfish.grizzly.strategies.WorkerThreadIOStrategy.run0(WorkerThreadIOStrategy.java:115)

at org.glassfish.grizzly.strategies.WorkerThreadIOStrategy.executeIoEvent(WorkerThreadIOStrategy.java:101)

at org.glassfish.grizzly.strategies.AbstractIOStrategy.executeIoEvent(AbstractIOStrategy.java:89)

at org.glassfish.grizzly.nio.SelectorRunner.iterateKeyEvents(SelectorRunner.java:406)

at org.glassfish.grizzly.nio.SelectorRunner.iterateKeys(SelectorRunner.java:375)

at org.glassfish.grizzly.nio.SelectorRunner.doSelect(SelectorRunner.java:339)

at org.glassfish.grizzly.nio.SelectorRunner.run(SelectorRunner.java:271)

at org.glassfish.grizzly.threadpool.AbstractThreadPool$Worker.doWork(AbstractThreadPool.java:565)

at org.glassfish.grizzly.threadpool.AbstractThreadPool$Worker.run(AbstractThreadPool.java:545)

at java.lang.Thread.run(Thread.java:662)

Caused by: java.net.ConnectException: Connection refused

at sun.nio.ch.SocketChannelImpl.checkConnect(Native Method)

at sun.nio.ch.SocketChannelImpl.finishConnect(SocketChannelImpl.java:599)

at org.glassfish.grizzly.nio.transport.TCPNIOConnectorHandler.onConnectedAsync(TCPNIOConnectorHandler.java:205)

... 15 more

On closer examination of the method TCPNIOConnectorHandler.onConnectedAsync(...) I see this:

The call to finishConnect is failing with a ConnectException, yet the exception handling code is treating it as if it is unexpected by throwing an IllegalStateException. The current behavior is benign I think since the runtime exception is trapped further up the stack in TCPNIOConnection.onConnect(). However, it's pretty fragile IMO since callers may not be aware that this method may throw a runtime exception for an expected error condition. Once the connection is aborted, does an exception need to be re-thrown at all?

While trying to debug one of our unit tests which is
failing due to a client connection attempt being refused I
came across a surprising looking piece of code. The stack
trace is (latest 2.3.4 nightly):

at
org.glassfish.grizzly.nio.transport.TCPNIOConnectorHandler.abortConnection(TCPNIOConnectorHandler.java:262)

at
org.glassfish.grizzly.nio.transport.TCPNIOConnectorHandler.onConnectedAsync(TCPNIOConnectorHandler.java:215)

at
org.glassfish.grizzly.nio.transport.TCPNIOConnectorHandler$1.completed(TCPNIOConnectorHandler.java:154)

at
org.glassfish.grizzly.nio.transport.TCPNIOConnectorHandler$1.completed(TCPNIOConnectorHandler.java:150)

at
org.glassfish.grizzly.nio.transport.TCPNIOConnection.onConnect(TCPNIOConnection.java:258)

at
org.glassfish.grizzly.nio.transport.TCPNIOTransport.fireIOEvent(TCPNIOTransport.java:826)

at
org.glassfish.grizzly.strategies.AbstractIOStrategy.fireIOEvent(AbstractIOStrategy.java:113)

at
org.glassfish.grizzly.strategies.WorkerThreadIOStrategy.run0(WorkerThreadIOStrategy.java:115)

at
org.glassfish.grizzly.strategies.WorkerThreadIOStrategy.executeIoEvent(WorkerThreadIOStrategy.java:101)

at
org.glassfish.grizzly.strategies.AbstractIOStrategy.executeIoEvent(AbstractIOStrategy.java:89)

at
org.glassfish.grizzly.nio.SelectorRunner.iterateKeyEvents(SelectorRunner.java:406)

at
org.glassfish.grizzly.nio.SelectorRunner.iterateKeys(SelectorRunner.java:375)

at
org.glassfish.grizzly.nio.SelectorRunner.doSelect(SelectorRunner.java:339)

at
org.glassfish.grizzly.nio.SelectorRunner.run(SelectorRunner.java:271)

at
org.glassfish.grizzly.threadpool.AbstractThreadPool$Worker.doWork(AbstractThreadPool.java:565)

at
org.glassfish.grizzly.threadpool.AbstractThreadPool$Worker.run(AbstractThreadPool.java:545)

at
java.lang.Thread.run(Thread.java:662)

Caused by:
java.net.ConnectException: Connection refused

at
sun.nio.ch.SocketChannelImpl.checkConnect(Native Method)

at
sun.nio.ch.SocketChannelImpl.finishConnect(SocketChannelImpl.java:599)

at
org.glassfish.grizzly.nio.transport.TCPNIOConnectorHandler.onConnectedAsync(TCPNIOConnectorHandler.java:205)

... 15 more

On closer examination of the method
TCPNIOConnectorHandler.onConnectedAsync(...) I see this:

The call to finishConnect is failing with a
ConnectException, yet the exception handling code is treating
it as if it is unexpected by throwing an
IllegalStateException. The current behavior is benign I think
since the runtime exception is trapped further up the stack in
TCPNIOConnection.onConnect(). However, it's pretty fragile IMO
since callers may not be aware that this method may throw a
runtime exception for an expected error condition. Once the
connection is aborted, does an exception need to be re-thrown
at all?