Hi Matt,
agree it's a design issue.
CompletionHandler doesn't fit well, we need something that allows
IOException to be thrown. I'll rework that.
Thank you for bringing this up.
WBR,
Alexey.
On 02.07.13 06:54, Matthew Swift wrote:
> 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:
>
> protected static void onConnectedAsync(final TCPNIOConnection
> connection,
> final CompletionHandler<Connection> completionHandler) {
>
> final TCPNIOTransport tcpTransport =
> (TCPNIOTransport) connection.getTransport();
> final SocketChannel channel = (SocketChannel)
> connection.getChannel();
>
> try {
> if (!channel.isConnected()) {
> *channel.finishConnect();*
> }
>
> connection.resetProperties();
>
> // Deregister OP_CONNECT interest
> connection.disableIOEvent(IOEvent.CLIENT_CONNECTED);
>
> tcpTransport.configureChannel(channel);
> } catch (Exception e) {
> abortConnection(connection, completionHandler, e);
> *throw new IllegalStateException(e);*
> }
>
> if (connection.notifyReady()) {
> tcpTransport.fireIOEvent(IOEvent.CONNECTED, connection,
> new EnableReadHandler(completionHandler));
> }
> }
>
> 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?
>
> Cheers,
>
> Matt
>