Salut,
Oleksiy Stashok wrote:
> Hi Bongjae,
>
> again, thank you for observations.
> I think the main issue here, is that we suppose to have connect
> non-blocking, but really have it blocking.
> That's why we have all that API inconsistency.
>
> What I could propose as general fix here - is to make connect really
> non-blocking and change API a little bit by making connect to return Future.
>
> So the result will be following:
>
> public Future connect(....) {
> ...... non-blocking connect .......
> }
>
> What do you think?
I agree....meanwhile, I think Bongjae's proposed patch would improve the
current situation until we implement your solution. Since we should try
to reduce API incompatibility, would it makes sense to apply the patch?
Thanks!
-- Jeanfrancois
>
> Thanks.
>
> WBR,
> Alexey.
>
> On May 9, 2009, at 9:53 , Bongjae Chang wrote:
>
>> Hi,
>>
>> When I tried to connect the invalid remote with using the
>> ConnectorHandler's connect(), I couldn't receive any exception though
>> ConnectorHandler's connect() API already had IOException.
>>
>> So, I should do a redundant check for graceful handling of errors.
>>
>> Here is a sample.
>> -----
>> ConnectorHandler connectorHandler = null;
>> try {
>> ...
>> connectorHandler = controller.acquireConnectorHandler(
>> Controller.Protocol.TCP );
>> ...
>> connectorHandler.connect( invalidRemoteAddress, localAddress );
>> if( connectorHandler instanceof TCPConnectorHandler ) {
>> *if( ((TCPConnectorHandler)connectorHandler).isConnected() )* {
>> OutputWrite.flushChannel(
>> connectorHandler.getUnderlyingChannel(), message, timeout );
>> }
>> }
>> OutputWrite.flushChannel( connectorHandler.getUnderlyingChannel(),
>> message, timeout );
>> } catch( Throwable t ) {
>> ...
>> } finally {
>> ...
>> }
>>
>> So,
>> * Suggestion #1
>> - I think that it is better that ConnectorHandlers which include TCP,
>> UDP, SSL and Cacheable type try to check the connection error itself.
>>
>> * Suggestion #2
>> - I think that it is better that AbstractConnectorHandler has
>> isConnected member and isConnected() API.
>>
>> * Suggestion #3
>> - I think that ConnectorHandler has isConnectedLatch for verifying the
>> connection, then it is better that ConnectorHandlers try to check the
>> isConnectedLatch's timeout.
>>
>> Then, patch is like the following.
>>
>> * In TCPConnectorHandler, UDPConnectorHandler and SSLConnectorHandler.
>> -----
>> public void connect(SocketAddress remoteAddress, SocketAddress
>> localAddress, P callbackHandler, E selectorHandler ) throws IOException {
>> ...
>> boolean finishConnect = false;
>> try {
>> finishConnect = isConnectedLatch.await(connectionTimeout,
>> TimeUnit.MILLISECONDS);
>> } catch(InterruptedException ex) {
>> throw new IOException(ex.getMessage());
>> }
>> if( !finishConnect )
>> throw new IOException( "failed to connect " + remoteAddress +
>> " because of connection timeout" );
>> if( !isConnected() )
>> throw new IOException( "failed to connect " + remoteAddress );
>> }
>> -----
>>
>> Could any side-effects be occurred if connect() method throws IOException?
>>
>> Thanks.
>>
>> --
>> Bongjae Chang
>