ChangeLogTag: Fri Apr 12 15:39:50 2002 Balachandran Natarajan <bala@cs.wustl.edu>connection_timeout_0

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  <bala@cs.wustl.edu>
+
+	* 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  <bala@cs.wustl.edu>
 
 	* 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<SVC_HANDLER>::
     activate_svc_handler (SVC_HANDLER *sh,
                           void *arg)
 {
-  if (ACE_Concurrency_Strategy<SVC_HANDLER>::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<SVC_HANDLER>::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;