Hi Bongjae,
thank you for the patch.
I've reworked it a little bit and commited to trunk.
Can you pls. check, if it works for you?
Thank you.
WBR,
Alexey.
On Apr 30, 2009, at 9:25 , Bongjae Chang wrote:
> Hi,
>
> I have another problem when I use the connection management cache.
>
> Sometimes, I couldn't receive a response packet when I tried to test
> TCP and UDP connector with CacheableConnectorHandlerPool.
>
> I tried to send two packets to the echo server which had an
> EchoFilter and supported TCP and UDP, then the former packet's
> response was lost.
>
> This is simple test code.
>
> -----
> * main part
> public static void main( String[] args ) throws IOException {
> Controller controller = new Controller();
> ConnectorHandlerPool cacheableHandlerPool = new
> CacheableConnectorHandlerPool( controller, 1000, 10, 1 );
> controller.setConnectorHandlerPool( cacheableHandlerPool );
>
> TCPSelectorHandler tcpSelectorHandler = new TCPSelectorHandler();
> tcpSelectorHandler.setPort( myPort );
> controller.addSelectorHandler( tcpSelectorHandler );
>
> UDPSelectorHandler udpSelectorHandler = new UDPSelectorHandler();
> udpSelectorHandler.setPort( myPort );
> controller.addSelectorHandler( udpSelectorHandler );
> ...
> tcpSendTest();
> udpSendTest();
> ...
> }
>
> * send part
> private void tcpSendTest() {
> ConnectorHandler connectorHandler = null;
> try {
> ...
> connectorHandler =
> controller.acquireConnectorHandler( Controller.Protocol.TCP );
> connectorHandler.connect( remoteAddress, localAddress );
>
> OutputWrite.flushChannel( connectorHandler.getUnderlyingChannel(),
> appMessage, timeout );
> ...
> } finally {
> if( connectorHandler != null ) {
> try {
> connectorHandler.close();
> } catch( IOException e ) {}
> controller.releaseConnectorHandler( connectorHandler );
> }
> }
> }
>
> private void udpSendTest() {
> ConnectorHandler connectorHandler = null;
> try {
> ...
> connectorHandler =
> controller.acquireConnectorHandler( Controller.Protocol.UDP );
> connectorHandler.connect( remoteAddress, localAddress );
>
> OutputWrite
> .flushChannel
> ( (DatagramChannel)connectorHandler.getUnderlyingChannel(),
> remoteAddress, appMessage, timeout );
> ...
> } finally {
> if( connectorHandler != null ) {
> try {
> connectorHandler.close();
> } catch( IOException e ) {}
> controller.releaseConnectorHandler( connectorHandler );
> }
> }
> }
>
> -----
>
> I also reviewed why this error had been occurred for a while.
>
> When I inserted some test code in CacheableConnectorHandler.java for
> debugging because any exception or log didn't be thrown, the
> following error was occurred.
>
> -----
> java.lang.NullPointerException
> at
> com
> .sun
> .grizzly
> .connectioncache
> .client
> .CacheableConnectorHandler.onRead(CacheableConnectorHandler.java:221)
> at
> com
> .sun
> .grizzly
> .CallbackHandlerContextTask.doCall(CallbackHandlerContextTask.java:64)
> 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 error line.
> -----
> In CacheableConnectorHandler.java
>
> public void onRead( IOEvent ioEvent ) {
> try {
> underlyingCallbackHandler.onRead( ioEvent );
> } catch( Throwable t ) {
> t.printStackTrace();
> }
> }
> -----
>
> NPE was thrown because underlyingCallbackHandler is null.
>
> First, I didn't understand why the underlyingCallbackHandler was
> null because the onConnect() method in
> CacheableConnectorHandler.java assigned DefaultCallbackHandler to
> the underlyingCallbackHandler if it was null.
>
> But when I debugged CacheableConnectorHandler, I found that some
> racing condition could be occurred.
>
> If the connector handler will be reused after the connector handler
> is released, the underlyingCallbackHandler is not thread-safe
> because CacheableConnectorHandler implements CallbackHandler and
> CacheableConnectorHandler's onConnect() and onRead() can be called
> concurrently.
>
> So, I tried to edit the original code in order to prevent sharing
> underlyingCalbackHandler experimentally.
>
> Here is experimental code.
>
> -----
> In CacheableConnectorHandler.java
>
> ...
> private class ConnectExecutor {
> ...
> public void invokeProtocolConnect() throws IOException {
> wasCalled = true;
> // This is like onConnect().
> CallbackHandler newCallbackHandler = this.callbackHandler;
> if( newCallbackHandler == null )
> newCallbackHandler = new
> DefaultCallbackHandler( CacheableConnectorHandler.this );
> switch(methodNumber) {
> // Preventing sharing CacheableConnectorHandler.this
> which is CallbackHandler's impl.
> //case 1:
> underlyingConnectorHandler.connect(remoteAddress,
> CacheableConnectorHandler.this, selectorHandler );
> case 1:
> underlyingConnectorHandler.connect(remoteAddress,
> newCallbackHandler , selectorHandler );
> ...
> }
> }
> }
> ...
> -----
> Then, I think that underlyingConnectorHandler doesn't use
> CacheableConnectorHandler's callback impl any more.
>
> Because this is just my survey, any other problems can exist or my
> review can be wrong. But I hope that my preview helps you to confirm
> this error.
>
> And if my scenario is not correct, please let me know.
>
> Thanks.
>
> --
> Bongjae Chang