> Bongjae wrote:
> I think that ConnectorHandler's socketChannel can be null if
> unexpected exceptions are occurred in CallbackHandler. ex) if an
> error was occurred before calling
> ConnectorHandler#finishConnect(SelectionKey key)
> So I think that graceful handling of NPE and more check logic are
> needed about underlyingConnectorHandler.getUnderlyingChannel().
>
>
> In CacheableConnectorHandler.java
> -----
> private voide notifyCallbackHandlerPseudoConnect() throws
> ClosedChannelException {
> ...
> SelectionKey key =
> underlyingConnectorHandler
> .getUnderlyingChannel().keyFor( protocolSelector );
> ...
> }
>
>
> At above the bold line, don't we need to check null channel? Is it
> redundant?
I think not. Pseudo connect is called, when we use already connected
connection, so underlying channel should be there. So it's really non-
normal (exceptional) situation, if underlying channel will be null
there.
Thank you!
WBR,
Alexey.
>
> If user makes a wrong CallbackHandler impl and
> AbstractConnectorHandler#setUnderlyingChannel doesn't be called,
> "underlyingConnectorHandler#getUnderlyingChannel()" can return
> null's channel adequately.
>
> So I think that graceful exception or at least warning log should be
> printed rather than NPE like this.
>
> -----
> private voide notifyCallbackHandlerPseudoConnect() throws
> IOException {
> SelectableChannel underlyingChannel =
> underlyingConnectorHandler.getUnderlyingChannel();
> if( underlyingChannel == null ) {
> throw new IOException(); // warning log or exception
> }
> SelectionKey key = underlyingChannel.keyFor( protocolSelector );
> ...
> }
> -----
>
> Thanks.
>
> --
> Bongjae Chang
>
>
> ----- Original Message -----
> From: Oleksiy Stashok
> To: dev_at_grizzly.dev.java.net
> Sent: Tuesday, April 28, 2009 10:15 PM
> Subject: Re: Connection management cache's problems
>
> Hi Bongjae,
>
> Thank you for finding this out!
>
> I've commited the fix, please let me know if it works for you.
>
> WBR,
> Alexey.
>
> On Apr 28, 2009, at 6:38 , Bongjae Chang wrote:
>
>> In addition, default CallbackHandler in CacheableConnectorHandler
>> didn't implement onRead(), onWrite().
>>
>> Here is default CallbackHandler in CacheableConnectorHandler.java
>>
>> -----
>> public void onConnect(IOEvent ioEvent) {
>> if (underlyingCallbackHandler == null) {
>> underlyingCallbackHandler = new
>> CallbackHandler<Context>(){
>> public void onConnect(IOEvent<Context> ioEvent) {
>> SelectionKey key =
>> ioEvent.attachment().getSelectionKey();
>> finishConnect(key);
>>
>> ioEvent
>> .attachment
>> ().getSelectorHandler().register(key,SelectionKey.OP_READ);
>> }
>> public void onRead(IOEvent<Context> ioEvent) {}
>> public void onWrite(IOEvent<Context> ioEvent) {}
>> };
>> }
>>
>> underlyingCallbackHandler.onConnect(ioEvent);
>> }
>> -----
>>
>> So I couldn't test EchoFilter with CacheableConnectorHandlerPool.
>> Are there any restrictions?
>>
>> If not, I think that the following code is better(Of course, if
>> DefaultCallbackHandler doesn't have ClassCastException's error).
>>
>> -----
>> public void onConnect(IOEvent ioEvent) {
>> if (underlyingCallbackHandler == null) {
>> underlyingCallbackHandler = new
>> DefaultCallbackHandler( this );
>> }
>> underlyingCallbackHandler.onConnect(ioEvent);
>> }
>> -----
>>
>> Thanks.
>>
>> --
>> Bongjae Chang
>>
>>
>> ----- Original Message -----
>> From: Bongjae Chang
>> To: dev_at_grizzly.dev.java.net
>> Sent: Tuesday, April 28, 2009 11:31 AM
>> Subject: Connection management cache's problems
>>
>> Hi.
>>
>> I tried to test
>> com
>> .sun.grizzly.connectioncache.client.CacheableConnectorHandlerPool
>> with com.sun.grizzly.DefaultCallbackHandler, but I had some problems.
>>
>> Here is simple test code.
>>
>> -----
>> [init part]
>> Controller controller = new Controller();
>> ConnectorHandlerPool cacheableHandlerPool = new
>> CacheableConnectorHandlerPool( controller, 100, 10, 1 );
>> controller.setConnectorHandlerPool( cacheableHandlerPool );
>>
>> TCPSelectorHandler tcpSelectorHandler = new TCPSelectorHandler();
>> tcpSelectorHandler.setPort( myPort );
>> tcpSelectorHandler.setSelectionKeyHandler( new
>> DefaultSelectionKeyHandler() );
>> controller.addSelectorHandler( tcpSelectorHandler );
>> ...
>>
>> [send part]
>> ConnectorHandler connectorHandler;
>> connectorHandler =
>> controller.acquireConnectorHandler( Controller.Protocol.TCP );
>> ..
>> connectorHandler.connect( remoteAddress, localAddress, new
>> DefaultCallbackHandler( connectorHandler ) );
>> OutputWrite.flushChannel( connectorHandler.getUnderlyingChannel(),
>> appMessage, timeout );
>> ...
>> -----
>>
>>
>> Here is result.
>> -----
>> java.lang.IllegalStateException: Invalid Response State.
>> SocketChannel cannot be null.
>> at
>> com.sun.grizzly.util.OutputWriter.flushChannel(OutputWriter.java:94)
>> at SimpleTestServer.sendTest(SimpleTestServer.java:134)
>> at SimpleTestServer.main(SimpleTestServer.java:44)
>> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>> at
>> sun
>> .reflect
>> .NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
>> at
>> sun
>> .reflect
>> .DelegatingMethodAccessorImpl
>> .invoke(DelegatingMethodAccessorImpl.java:43)
>> at java.lang.reflect.Method.invoke(Method.java:623)
>> at com.intellij.rt.execution.application.AppMain.main(AppMain.java:
>> 90)
>> java.lang.NullPointerException
>> at
>> com
>> .sun
>> .grizzly
>> .connectioncache
>> .client
>> .CacheableConnectorHandler
>> .notifyCallbackHandlerPseudoConnect(CacheableConnectorHandler.java:
>> 222)
>> at
>> com
>> .sun
>> .grizzly
>> .connectioncache
>> .client
>> .CacheableConnectorHandler.doConnect(CacheableConnectorHandler.java:
>> 164)
>> at
>> com
>> .sun
>> .grizzly
>> .connectioncache
>> .client
>> .CacheableConnectorHandler.connect(CacheableConnectorHandler.java:
>> 117)
>> at SimpleTestServer.sendTest(SimpleTestServer.java:132)
>> at SimpleTestServer.main(SimpleTestServer.java:45)
>> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
>> at
>> sun
>> .reflect
>> .NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
>> at
>> sun
>> .reflect
>> .DelegatingMethodAccessorImpl
>> .invoke(DelegatingMethodAccessorImpl.java:43)
>> at java.lang.reflect.Method.invoke(Method.java:623)
>> at com.intellij.rt.execution.application.AppMain.main(AppMain.java:
>> 90)
>> -----
>>
>>
>> Here is the NPE's line.
>>
>> In CacheableConnectorHandler.java
>> -----
>> private voide notifyCallbackHandlerPseudoConnect() throws
>> ClosedChannelException {
>> ...
>> SelectionKey key =
>> underlyingConnectorHandler
>> .getUnderlyingChannel().keyFor( protocolSelector );
>> ...
>> }
>>
>> I think that ConnectorHandler's socketChannel can be null if
>> unexpected exceptions are occurred in CallbackHandler. ex) if an
>> error was occurred before calling
>> ConnectorHandler#finishConnect(SelectionKey key)
>>
>> So I think that graceful handling of NPE and more check logic are
>> needed about underlyingConnectorHandler.getUnderlyingChannel().
>>
>> And I reviewed why this error had been occurred for a while.
>>
>> When I inserted some test code for debugging, the following error
>> was occurred.
>> -----
>> java.lang.ClassCastException:
>> com.sun.grizzly.connectioncache.client.CacheableConnectorHandler
>> cannot be cast to com.sun.grizzly.TCPConnectorHandler
>> at
>> com
>> .sun
>> .grizzly
>> .DefaultCallbackHandler.onConnect(DefaultCallbackHandler.java:128)
>> at
>> com
>> .sun
>> .grizzly
>> .connectioncache
>> .client
>> .CacheableConnectorHandler.onConnect(CacheableConnectorHandler.java:
>> 250)
>> at
>> com
>> .sun
>> .grizzly
>> .CallbackHandlerContextTask.doCall(CallbackHandlerContextTask.java:
>> 68)
>> at
>> com
>> .sun
>> .grizzly.SelectionKeyContextTask.call(SelectionKeyContextTask.java:
>> 57)
>> at com.sun.grizzly.ContextTask.run(ContextTask.java:69)
>> at
>> java
>> .util
>> .concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:
>> 1110)
>> at java.util.concurrent.ThreadPoolExecutor
>> $Worker.run(ThreadPoolExecutor.java:603)
>> at java.lang.Thread.run(Thread.java:717)
>> -----
>>
>> Here is DefaultCallbackHandler#onConnect(IOEvent<Context> ioEvent);
>>
>> In DefaultCallbackHandler.java
>> -----
>> public void onConnect( IOEvent<Context> ioEvent ) {
>> SelectionKey key = ioEvent.attachment().getSelectionKey();
>> if( ioEvent.attachment().getProtocol() ==
>> Controller.Protocol.TCP ) {
>>
>> ((TCPConnectorHandler
>> )connectorHandler
>> ).setUnderlyingChannel((SocketChannel)key.channel());
>> } else if( ioEvent.attachment().getProtocol() ==
>> Controller.Protocol.TLS) {
>>
>> ((SSLConnectorHandler
>> )connectorHandler
>> ).setUnderlyingChannel((SocketChannel)key.channel());
>> }
>> try {
>> connectorHandler.finishConnect(key);
>> ...
>> }
>> ...
>>
>> }
>> -----
>>
>> I think that above calling setUnderlyingChannel() is redundant
>> because both TCPConnectorHandler#finishConnect() and
>> SSLConnectorHandler#finishConnect() already assign socketChannel
>> key's channel.
>>
>> Of course, it maybe has no problem if I don't use
>> DefaultCallbackHandler.
>>
>> But I hope that above reduntant code could be removed and I am
>> curious to know if my use and point have any faults.
>>
>> Please give me advice.
>>
>> Thanks.
>>
>> --
>> Bongjae Chang
>
>