From fba3a1170f62ef467346871f9053892beea51eb4 Mon Sep 17 00:00:00 2001 From: Nick Cross Date: Sun, 16 Jun 2013 18:46:14 +0100 Subject: [PATCH] BZ947 Add fix for invalid disconnection logic --- doc/REL_NOTES | 1 + src/org/jacorb/orb/Delegate.java | 2 +- .../orb/giop/ClientConnectionManager.java | 13 +- .../org/jacorb/test/orb/DisconnectTest.java | 250 ++++++++++++++++++ 4 files changed, 259 insertions(+), 7 deletions(-) create mode 100644 test/regression/src/org/jacorb/test/orb/DisconnectTest.java diff --git a/doc/REL_NOTES b/doc/REL_NOTES index f54062202..99aca01a9 100644 --- a/doc/REL_NOTES +++ b/doc/REL_NOTES @@ -51,6 +51,7 @@ This Release - Add support for GIOP 1.1 Fragmented Messages (BZ950). - Add public API for Client/ServerRequestInfo. - Add fix for NIO with large requests (BZ939). + - Add fix for disconnect logic on bad connection (BZ947). - TAO ImR/NS Compatibility - Implement the new property "jacorb.use_tao_imr". diff --git a/src/org/jacorb/orb/Delegate.java b/src/org/jacorb/orb/Delegate.java index e3aeaa4fd..010802af1 100644 --- a/src/org/jacorb/orb/Delegate.java +++ b/src/org/jacorb/orb/Delegate.java @@ -1481,7 +1481,7 @@ private void disconnect(ClientConnection connectionInUse) return; } - if (disconnectAfterNonRecoverableSystemException) + if (!disconnectAfterNonRecoverableSystemException) { return; } diff --git a/src/org/jacorb/orb/giop/ClientConnectionManager.java b/src/org/jacorb/orb/giop/ClientConnectionManager.java index 24d05eb42..66a8a0ac5 100644 --- a/src/org/jacorb/orb/giop/ClientConnectionManager.java +++ b/src/org/jacorb/orb/giop/ClientConnectionManager.java @@ -31,6 +31,7 @@ import org.jacorb.orb.iiop.IIOPProfile; import org.omg.CORBA.BAD_PARAM; import org.omg.ETF.Factories; +import org.omg.ETF.Profile; import org.slf4j.Logger; /** @@ -45,7 +46,7 @@ public class ClientConnectionManager private final org.jacorb.orb.ORB orb; /** connection mgmt. */ - private final Map connections = new HashMap(); + private final Map connections = new HashMap(); private RequestListener request_listener; @@ -76,7 +77,7 @@ public void configure(Configuration myConfiguration) // Moved from the constructor to facilitate logging. receptor_pool = new MessageReceptorPool("client", "ClientMessageReceptor", myConfiguration); - org.jacorb.config.Configuration configuration = (org.jacorb.config.Configuration)myConfiguration; + org.jacorb.config.Configuration configuration = myConfiguration; logger = configuration.getLogger("jacorb.orb.giop"); request_listener = new NoBiDirClientRequestListener(orb, logger); @@ -93,7 +94,7 @@ public synchronized ClientConnection getConnection(org.omg.ETF.Profile profile) /* look for an existing connection */ ClientConnection clientConnection = - (ClientConnection)connections.get( profile ); + connections.get( profile ); if (clientConnection == null && profile instanceof IIOPProfile) { @@ -103,7 +104,7 @@ public synchronized ClientConnection getConnection(org.omg.ETF.Profile profile) { final IIOPProfile sslProfile = iiopProfile.toNonSSL(); - clientConnection = (ClientConnection) connections.get(sslProfile); + clientConnection = connections.get(sslProfile); } } @@ -214,9 +215,9 @@ public synchronized void shutdown() { /* release all open connections */ - for( Iterator i = new HashSet(connections.values()).iterator(); i.hasNext(); ) + for( Iterator i = new HashSet(connections.values()).iterator(); i.hasNext(); ) { - ((ClientConnection) i.next()).close(); + i.next().close(); } if( logger.isDebugEnabled()) diff --git a/test/regression/src/org/jacorb/test/orb/DisconnectTest.java b/test/regression/src/org/jacorb/test/orb/DisconnectTest.java new file mode 100644 index 000000000..7ec37137f --- /dev/null +++ b/test/regression/src/org/jacorb/test/orb/DisconnectTest.java @@ -0,0 +1,250 @@ +package org.jacorb.test.orb; + +/* + * JacORB - a free Java ORB + * + * Copyright (C) 1997-2013 Gerald Brose / The JacORB Team. + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Library General Public + * License as published by the Free Software Foundation; either + * version 2 of the License, or (at your option) any later version. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Library General Public License for more details. + * + * You should have received a copy of the GNU Library General Public + * License along with this library; if not, write to the Free + * Software Foundation, 51 Franklin Street, Fifth Floor, Boston, + * MA 02110-1301, USA. + */ + +import java.lang.reflect.Field; +import java.util.HashMap; +import java.util.Properties; +import junit.framework.Test; +import junit.framework.TestSuite; +import org.jacorb.orb.Delegate; +import org.jacorb.orb.giop.ClientConnection; +import org.jacorb.orb.giop.ClientConnectionManager; +import org.jacorb.test.BasicServer; +import org.jacorb.test.BasicServerHelper; +import org.jacorb.test.common.ClientServerSetup; +import org.jacorb.test.common.ClientServerTestCase; +import org.jacorb.test.common.JacORBTestSuite; +import org.omg.CORBA.TIMEOUT; +import org.omg.ETF.Profile; + +/** + * @author Nick Cross + * + * Verifies that the connection map is cleared after a timeout and disconnect. + */ +public class DisconnectTest extends ClientServerTestCase +{ + protected BasicServer server = null; + + + public DisconnectTest(String name, ClientServerSetup setup) + { + super(name, setup); + } + + protected void setUp() throws Exception + { + server = BasicServerHelper.narrow( setup.getServerObject() ); + } + + protected void tearDown() throws Exception + { + server = null; + } + + public static Test suite() + { + TestSuite suite = new JacORBTestSuite("Disconnect client test", DisconnectTest.class); + + Properties client_props = new Properties(); + client_props.setProperty ("jacorb.retries", "0"); + client_props.setProperty ("jacorb.retry_interval", "50"); + + client_props.setProperty + ("jacorb.connection.client.pending_reply_timeout", "5000"); + + Properties server_props = new Properties(); + server_props.setProperty( "org.omg.PortableInterceptor.ORBInitializerClass." + + DisconnectInitializer.class.getName(), "" ); + + + + ClientServerSetup setup = new ClientServerSetup( suite, + "org.jacorb.test.orb.BasicServerImpl", + client_props, + server_props); + + suite.addTest( new DisconnectTest( "test_disconnect", setup )); + + return setup; + } + + public void test_disconnect() + { + boolean timeout = false; + try + { + server.ping(); + } + catch (TIMEOUT e) + { + timeout = true; + + Field fconnmgr; + Field connections; + try + { + fconnmgr = Delegate.class.getDeclaredField("conn_mg"); + fconnmgr.setAccessible(true); + Delegate d = (Delegate) ((org.omg.CORBA.portable.ObjectImpl)server)._get_delegate(); + ClientConnectionManager ccm = (ClientConnectionManager) fconnmgr.get(d); + connections = ClientConnectionManager.class.getDeclaredField("connections"); + connections.setAccessible(true); + @SuppressWarnings("unchecked") + HashMap c = (HashMap) connections.get(ccm); + + assertTrue (c.size() == 0); + } + catch (SecurityException e1) + { + // TODO Auto-generated catch block + e1.printStackTrace(); + } + catch (NoSuchFieldException e1) + { + // TODO Auto-generated catch block + e1.printStackTrace(); + } + catch (IllegalArgumentException e1) + { + // TODO Auto-generated catch block + e1.printStackTrace(); + } + catch (IllegalAccessException e1) + { + // TODO Auto-generated catch block + e1.printStackTrace(); + } + } + if ( ! timeout ) + { + fail ("Did not catch a timeout"); + } + } + + public static class DisconnectInitializer + extends org.omg.CORBA.LocalObject + implements org.omg.PortableInterceptor.ORBInitializer + { + /** + * Called before init of the actual ORB. + * + * @param info The ORB init info. + */ + public void pre_init (org.omg.PortableInterceptor.ORBInitInfo info) + { + try + { + info.add_server_request_interceptor (new ServerInterceptorA()); + } + catch ( org.omg.PortableInterceptor.ORBInitInfoPackage.DuplicateName ex ) + { + fail ("unexpected exception received: " + ex); + } + } + + /** + * Called after init of the actual ORB. + * + * @param info The ORB init info. + */ + public void post_init (org.omg.PortableInterceptor.ORBInitInfo info) + { + } + } + + static class ServerInterceptorA + extends org.omg.CORBA.LocalObject + implements org.omg.PortableInterceptor.ServerRequestInterceptor + { + static boolean localDebugOn = false; + + public java.lang.String name() + { + return ""; + } + + public void destroy() + { + } + + public void receive_request_service_contexts( + org.omg.PortableInterceptor.ServerRequestInfo ri ) + throws org.omg.PortableInterceptor.ForwardRequest + { + if (localDebugOn) + { + System.out.println ("LocalServerInterceptorA - receive_request_service_contexts"); + } + } + + public void receive_request( org.omg.PortableInterceptor.ServerRequestInfo ri ) + throws org.omg.PortableInterceptor.ForwardRequest + { + if (localDebugOn) + { + System.out.println ("LocalServerInterceptorA - receive_request"); + } + try + { + Thread.sleep (10000); + } + catch (InterruptedException e) + { + // TODO Auto-generated catch block + e.printStackTrace(); + } + } + + public void send_reply( org.omg.PortableInterceptor.ServerRequestInfo ri ) + { + if (localDebugOn) + { + System.out.println ("LocalServerInterceptorA - send_reply"); + } + } + + public void send_exception( org.omg.PortableInterceptor.ServerRequestInfo ri ) + throws org.omg.PortableInterceptor.ForwardRequest + { + if (localDebugOn) + { + System.out.println ("LocalServerInterceptorA - send_exception"); + } + } + + public void send_other( org.omg.PortableInterceptor.ServerRequestInfo ri ) + throws org.omg.PortableInterceptor.ForwardRequest + { + if (localDebugOn) + { + System.out.println ("LocalServerInterceptorA - send_other"); + } + } + } + + public static void main(String args[]) + { + junit.textui.TestRunner.run(suite()); + } +}