From dcc4aae2a8000d65757fac4203232893a1dfbdcb Mon Sep 17 00:00:00 2001 From: bala Date: Fri, 12 Apr 2002 21:09:28 +0000 Subject: ChangeLogTag: Fri Apr 12 15:39:50 2002 Balachandran Natarajan --- TAO/tao/ChangeLog | 28 ++++++++++++++++++++++++++++ TAO/tao/Connector_Impl.cpp | 9 ++------- TAO/tao/IIOP_Connector.cpp | 19 ++++++++++++++++--- TAO/tao/Strategies/SHMIOP_Connector.cpp | 24 ++++++++++++++++++------ TAO/tao/Strategies/UIOP_Connector.cpp | 21 ++++++++++++++++----- 5 files changed, 80 insertions(+), 21 deletions(-) diff --git a/TAO/tao/ChangeLog b/TAO/tao/ChangeLog index d50a9cb9616..c6d30eedf09 100644 --- a/TAO/tao/ChangeLog +++ b/TAO/tao/ChangeLog @@ -1,3 +1,31 @@ +Fri Apr 12 15:39:50 2002 Balachandran Natarajan + + * tao/Connector_Impl.cpp: Dont add the handler to the reactor. + + * tao/IIOP_Connector.cpp: + * tao/Strategies/SHMIOP_Connector.cpp: + * tao/Strategies/UIOP_Connector.cpp: Add the handler to the + reactor after the transport has been cached. + + The above changes helps to works around a race condition. The + race occurs in the following scenario: + + - the handler is added to the reactor as soon as the connection + is established + - the server crashes and a thread waiting on the reactor closes + down the connection. As the connection handler and transports + have not been ref counted (yet) they could be completely + destroyed. + - the thread that initiated the connection would try to + duplicate a null transport and try adding that to cache and + all hell would break loose. + + The methodology adopted is to delay adding the handler to the + reactor till the handler is added to cache. This would make sure + that the transport is refcounted and hence the thread making the + connection will have a valid pointer to take action and do the + cleanup when it finds that the connection is closed. + Fri Apr 12 14:14:01 2002 Balachandran Natarajan * tao/Connector_Registry.cpp: diff --git a/TAO/tao/Connector_Impl.cpp b/TAO/tao/Connector_Impl.cpp index 54f60860d4a..6c4023da250 100644 --- a/TAO/tao/Connector_Impl.cpp +++ b/TAO/tao/Connector_Impl.cpp @@ -56,13 +56,8 @@ TAO_Connect_Concurrency_Strategy:: activate_svc_handler (SVC_HANDLER *sh, void *arg) { - if (ACE_Concurrency_Strategy::activate_svc_handler (sh, - arg) == -1) - return -1; - - // If the wait strategy wants us to be registered with the reactor - // then we do so. - return sh->transport ()->wait_strategy ()->register_handler (); + return ACE_Concurrency_Strategy::activate_svc_handler (sh, + arg); } diff --git a/TAO/tao/IIOP_Connector.cpp b/TAO/tao/IIOP_Connector.cpp index ff5a3c56f2e..101660999e8 100644 --- a/TAO/tao/IIOP_Connector.cpp +++ b/TAO/tao/IIOP_Connector.cpp @@ -109,7 +109,6 @@ int TAO_IIOP_Connector::make_connect (TAO_GIOP_Invocation *invocation, TAO_Transport_Descriptor_Interface *desc) { - TAO_Transport *&transport = invocation->transport (); ACE_Time_Value *max_wait_time = invocation->max_wait_time (); TAO_Endpoint *endpoint = desc->endpoint (); @@ -144,7 +143,7 @@ TAO_IIOP_Connector::make_connect (TAO_GIOP_Invocation *invocation, int result = 0; TAO_IIOP_Connection_Handler *svc_handler = 0; - TAO_Transport *base_transport; + if (TAO_debug_level > 2) ACE_DEBUG ((LM_DEBUG, @@ -200,7 +199,8 @@ TAO_IIOP_Connector::make_connect (TAO_GIOP_Invocation *invocation, ACE_LIB_TEXT ("new connection on HANDLE %d\n"), svc_handler->peer ().get_handle ())); - base_transport = TAO_Transport::_duplicate (svc_handler->transport ()); + TAO_Transport *base_transport = + TAO_Transport::_duplicate (svc_handler->transport ()); // Add the handler to Cache int retval = @@ -214,6 +214,19 @@ TAO_IIOP_Connector::make_connect (TAO_GIOP_Invocation *invocation, ACE_LIB_TEXT ("could not add the new connection to Cache \n"))); } + // If the wait strategy wants us to be registered with the reactor + // then we do so. + retval = base_transport->wait_strategy ()->register_handler (); + + if (retval != 0 && TAO_debug_level > 0) + { + ACE_DEBUG ((LM_DEBUG, + ACE_LIB_TEXT ("(%P|%t) IIOP_Connector::connect ") + ACE_LIB_TEXT ("could not add the new connection to reactor \n"))); + } + + // Handover the transport pointer to the Invocation class. + TAO_Transport *&transport = invocation->transport (); transport = base_transport; return 0; diff --git a/TAO/tao/Strategies/SHMIOP_Connector.cpp b/TAO/tao/Strategies/SHMIOP_Connector.cpp index 8b6f21e84c9..440522991d1 100644 --- a/TAO/tao/Strategies/SHMIOP_Connector.cpp +++ b/TAO/tao/Strategies/SHMIOP_Connector.cpp @@ -124,7 +124,6 @@ TAO_SHMIOP_Connector::make_connect (TAO_GIOP_Invocation *invocation, ACE_TEXT ("TAO (%P|%t) Connector::connect - ") ACE_TEXT ("looking for SHMIOP connection.\n"))); - TAO_Transport *&transport = invocation->transport (); ACE_Time_Value *max_wait_time = invocation->max_wait_time (); TAO_Endpoint *endpoint = desc->endpoint (); @@ -160,7 +159,6 @@ TAO_SHMIOP_Connector::make_connect (TAO_GIOP_Invocation *invocation, int result = 0; TAO_SHMIOP_Connection_Handler *svc_handler = 0; - TAO_Transport *base_transport = 0; if (TAO_debug_level > 2) ACE_DEBUG ((LM_DEBUG, @@ -216,11 +214,13 @@ TAO_SHMIOP_Connector::make_connect (TAO_GIOP_Invocation *invocation, return -1; } - base_transport = TAO_Transport::_duplicate (svc_handler->transport ()); + TAO_Transport *base_transport = + TAO_Transport::_duplicate (svc_handler->transport ()); + // Add the handler to Cache int retval = this->orb_core ()->lane_resources ().transport_cache ().cache_transport (desc, - svc_handler->transport ()); + base_transport); if (retval != 0 && TAO_debug_level > 0) { @@ -229,8 +229,20 @@ TAO_SHMIOP_Connector::make_connect (TAO_GIOP_Invocation *invocation, ACE_TEXT ("could not add the new connection to Cache \n"))); } - // No need to _duplicate and release since base_transport - // is going out of scope. transport now has control of base_transport. + + // If the wait strategy wants us to be registered with the reactor + // then we do so. + retval = base_transport->wait_strategy ()->register_handler (); + + if (retval != 0 && TAO_debug_level > 0) + { + ACE_DEBUG ((LM_DEBUG, + ACE_LIB_TEXT ("(%P|%t) IIOP_Connector::connect ") + ACE_LIB_TEXT ("could not add the new connection to reactor \n"))); + } + + // Handover the transport pointer to the Invocation class. + TAO_Transport *&transport = invocation->transport (); transport = base_transport; return 0; diff --git a/TAO/tao/Strategies/UIOP_Connector.cpp b/TAO/tao/Strategies/UIOP_Connector.cpp index 77279113a86..cc594d87c81 100644 --- a/TAO/tao/Strategies/UIOP_Connector.cpp +++ b/TAO/tao/Strategies/UIOP_Connector.cpp @@ -121,7 +121,6 @@ TAO_UIOP_Connector::make_connect (TAO_GIOP_Invocation *invocation, ACE_TEXT ("TAO (%P|%t) Connector::connect - ") ACE_TEXT ("looking for UIOP connection.\n"))); - TAO_Transport *&transport = invocation->transport (); ACE_Time_Value *max_wait_time = invocation->max_wait_time (); TAO_Endpoint *endpoint = desc->endpoint (); @@ -147,7 +146,6 @@ TAO_UIOP_Connector::make_connect (TAO_GIOP_Invocation *invocation, int result = 0; TAO_UIOP_Connection_Handler *svc_handler = 0; - TAO_Transport *base_transport = 0; if (TAO_debug_level > 2) ACE_DEBUG ((LM_DEBUG, @@ -199,7 +197,9 @@ TAO_UIOP_Connector::make_connect (TAO_GIOP_Invocation *invocation, return -1; } - base_transport = TAO_Transport::_duplicate (svc_handler->transport ()); + TAO_Transport *base_transport = + TAO_Transport::_duplicate (svc_handler->transport ()); + // Add the handler to Cache int retval = this->orb_core ()->lane_resources ().transport_cache ().cache_transport (desc, @@ -212,8 +212,19 @@ TAO_UIOP_Connector::make_connect (TAO_GIOP_Invocation *invocation, ACE_TEXT ("could not add the new connection to Cache \n"))); } - // No need to _duplicate and release since base_transport - // is going out of scope. transport now has control of base_transport. + // If the wait strategy wants us to be registered with the reactor + // then we do so. + retval = base_transport->wait_strategy ()->register_handler (); + + if (retval != 0 && TAO_debug_level > 0) + { + ACE_DEBUG ((LM_DEBUG, + ACE_LIB_TEXT ("(%P|%t) IIOP_Connector::connect ") + ACE_LIB_TEXT ("could not add the new connection to reactor \n"))); + } + + // Handover the transport pointer to the Invocation class. + TAO_Transport *&transport = invocation->transport (); transport = base_transport; return 0; -- cgit v1.2.1