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