ChangeLogTag:Fri Nov 5 16:14:52 2004 Balachandran Natarajan <bala@dre.vanderbilt.edu>

4 files changed, 96 insertions, 44 deletions

diff --git a/TAO/ChangeLog b/TAO/ChangeLog
index 6000d5c8836..e07f2172bd9 100644
--- a/TAO/ChangeLog
+++ b/TAO/ChangeLog
@@ -1,3 +1,20 @@
+Fri Nov  5 16:14:52 2004  Balachandran Natarajan  <bala@dre.vanderbilt.edu>
+
+	* tao/DynamicInterface/DII_Arguments.cpp (interceptor_paramlist):
+
+	  Do not insert the argument into any Any. Just call replace () to
+	  make a logical copy of the TAO::Any_impl. 
+
+	* tao/PortableServer/Object_Adapter.cpp:
+	* tao/PortableServer/POA.cpp:
+
+	  Reorganized the lock strategy which prevents deadlocks with
+	  ImR. Please see
+
+	  http://groups.yahoo.com/group/tao-users/message/18317
+
+	  for the original bug report and the suggested solution.
+
 Fri Nov  5 07:02:01 2004  Chad Elliott  <elliott_c@ociweb.com>
 
         * orbsvcs/tests/Security/Big_Request/client.cpp:
diff --git a/TAO/tao/DynamicInterface/DII_Arguments.cpp b/TAO/tao/DynamicInterface/DII_Arguments.cpp
index 210626bc3a2..f0a10a0dc39 100644
--- a/TAO/tao/DynamicInterface/DII_Arguments.cpp
+++ b/TAO/tao/DynamicInterface/DII_Arguments.cpp
@@ -36,6 +36,7 @@ namespace TAO
         return 0;
       }
     ACE_ENDTRY;
+    ACE_CHECK_RETURN (false);
 
     this->byte_order_ = cdr.byte_order ();
 
@@ -65,6 +66,7 @@ namespace TAO
         return 0;
       }
     ACE_ENDTRY;
+    ACE_CHECK_RETURN (false);
 
     return 1;
   }
@@ -94,35 +96,40 @@ namespace TAO
         return 0;
       }
     ACE_ENDTRY;
+    ACE_CHECK_RETURN (false);
 
     return 1;
   }
 
   void
-  NVList_Argument::interceptor_paramlist (Dynamic::ParameterList *list)
+  NVList_Argument::interceptor_paramlist (Dynamic::ParameterList *lst)
   {
     const CORBA::ULong len = this->x_->count ();
-    list->length (len);
+    lst->length (len);
 
     for (CORBA::ULong i = 0; i < len; ++i)
       {
-        (*list)[i].argument <<= *this->x_->item (i)->value ();
+        if (!this->x_->item (i)->value ())
+          return;
+
+        (*lst)[i].argument.replace (
+          this->x_->item (i)->value ()->impl ());
 
         switch (this->x_->item (i)->flags ())
           {
           case CORBA::ARG_IN:
             {
-              (*list)[i].mode = CORBA::PARAM_IN;
+              (*lst)[i].mode = CORBA::PARAM_IN;
               break;
             }
           case CORBA::ARG_INOUT:
             {
-              (*list)[i].mode = CORBA::PARAM_INOUT;
+              (*lst)[i].mode = CORBA::PARAM_INOUT;
               break;
             }
           case CORBA::ARG_OUT:
             {
-              (*list)[i].mode = CORBA::PARAM_OUT;
+              (*lst)[i].mode = CORBA::PARAM_OUT;
               break;
             }
           default:
diff --git a/TAO/tao/PortableServer/Object_Adapter.cpp b/TAO/tao/PortableServer/Object_Adapter.cpp
index 6515171f1cc..ced76a36912 100644
--- a/TAO/tao/PortableServer/Object_Adapter.cpp
+++ b/TAO/tao/PortableServer/Object_Adapter.cpp
@@ -409,16 +409,6 @@ TAO_Object_Adapter::activate_poa (const poa_name &folded_name,
   else
     ++iterator;
 
-  // A recursive thread lock without using a recursive thread lock.
-  // Non_Servant_Upcall has a magic constructor and destructor.  We
-  // unlock the Object_Adapter lock for the duration of the adapter
-  // activator(s) upcalls; reacquiring once the upcalls complete.
-  // Even though we are releasing the lock, other threads will not be
-  // able to make progress since
-  // <Object_Adapter::non_servant_upcall_in_progress_> has been set.
-  Non_Servant_Upcall non_servant_upcall (*parent);
-  ACE_UNUSED_ARG (non_servant_upcall);
-
   for (;
        iterator != end;
        ++iterator)
@@ -1251,7 +1241,7 @@ TAO_Object_Adapter::Non_Servant_Upcall::Non_Servant_Upcall (TAO_POA &poa)
   // Adjust the nesting level.
   this->object_adapter_.non_servant_upcall_nesting_level_++;
 
-  // Release the Object Adapter lock.
+  // We always release
   this->object_adapter_.lock ().release ();
 }
 
@@ -1260,12 +1250,11 @@ TAO_Object_Adapter::Non_Servant_Upcall::~Non_Servant_Upcall (void)
   // Reacquire the Object Adapter lock.
   this->object_adapter_.lock ().acquire ();
 
+  this->object_adapter_.non_servant_upcall_nesting_level_--;
+
   // We are done with this nested upcall.
   this->object_adapter_.non_servant_upcall_in_progress_ = this->previous_;
 
-  // Adjust the nesting level.
-  this->object_adapter_.non_servant_upcall_nesting_level_--;
-
   // If we are at the outer nested upcall.
   if (this->object_adapter_.non_servant_upcall_nesting_level_ == 0)
     {
diff --git a/TAO/tao/PortableServer/POA.cpp b/TAO/tao/PortableServer/POA.cpp
index ebed46d63f7..cb4beec958c 100644
--- a/TAO/tao/PortableServer/POA.cpp
+++ b/TAO/tao/PortableServer/POA.cpp
@@ -385,14 +385,6 @@ TAO_POA::complete_destruction_i (ACE_ENV_SINGLE_ARG_DECL)
   // lead to reference deadlock, i.e., POA holds object A, but POA
   // cannot die because object A hold POA.
   {
-    // A recursive thread lock without using a recursive thread lock.
-    // Non_Servant_Upcall has a magic constructor and destructor.  We
-    // unlock the Object_Adapter lock for the duration of the servant
-    // activator upcalls; reacquiring once the upcalls complete.  Even
-    // though we are releasing the lock, other threads will not be
-    // able to make progress since
-    // <Object_Adapter::non_servant_upcall_in_progress_> has been set.
-
     //
     // If new things are added to this cleanup code, make sure to move
     // the minimum CORBA #define after the declaration of
@@ -401,6 +393,14 @@ TAO_POA::complete_destruction_i (ACE_ENV_SINGLE_ARG_DECL)
 
 #if (TAO_HAS_MINIMUM_POA == 0)
 
+    // A recursive thread lock without using a recursive thread lock.
+    // Non_Servant_Upcall has a magic constructor and destructor.  We
+    // unlock the Object_Adapter lock for the duration of the servant
+    // activator upcalls; reacquiring once the upcalls complete.  Even
+    // though we are releasing the lock, other threads will not be
+    // able to make progress since
+    // <Object_Adapter::non_servant_upcall_in_progress_> has been set.
+
     TAO_Object_Adapter::Non_Servant_Upcall non_servant_upcall (*this);
     ACE_UNUSED_ARG (non_servant_upcall);
 
@@ -616,16 +616,6 @@ TAO_POA::find_POA (const char *adapter_name,
   // Lock access for the duration of this transaction.
   TAO_POA_GUARD_RETURN (0);
 
-  // A recursive thread lock without using a recursive thread lock.
-  // Non_Servant_Upcall has a magic constructor and destructor.  We
-  // unlock the Object_Adapter lock for the duration of the servant
-  // activator upcalls; reacquiring once the upcalls complete.  Even
-  // though we are releasing the lock, other threads will not be able
-  // to make progress since
-  // <Object_Adapter::non_servant_upcall_in_progress_> has been set.
-  TAO_Object_Adapter::Non_Servant_Upcall non_servant_upcall (*this);
-  ACE_UNUSED_ARG (non_servant_upcall);
-
   TAO_POA *poa = this->find_POA_i (adapter_name,
                                    activate_it
                                    ACE_ENV_ARG_PARAMETER);
@@ -657,6 +647,18 @@ TAO_POA::find_POA_i (const ACE_CString &child_name,
               this->check_poa_manager_state (ACE_ENV_SINGLE_ARG_PARAMETER);
               ACE_CHECK_RETURN (0);
 
+              // A recursive thread lock without using a recursive
+              // thread lock. Non_Servant_Upcall has a magic
+              // constructor and destructor.  We unlock the
+              // Object_Adapter lock for the duration of the servant
+              // activator upcalls; reacquiring once the upcalls
+              // complete.  Even though we are releasing the lock,
+              // other threads will not be able to make progress since
+              // <Object_Adapter::non_servant_upcall_in_progress_> has
+              // been set.
+              TAO_Object_Adapter::Non_Servant_Upcall non_servant_upcall (*this);
+              ACE_UNUSED_ARG (non_servant_upcall);
+
               CORBA::Boolean success =
                 this->adapter_activator_->unknown_adapter (this,
                                                            child_name.c_str ()
@@ -3944,9 +3946,21 @@ TAO_POA::imr_notify_startup (ACE_ENV_SINGLE_ARG_DECL)
   if (CORBA::is_nil (imr.in ()))
       return;
 
-  ImplementationRepository::Administration_var imr_locator =
-    ImplementationRepository::Administration::_narrow (imr.in () ACE_ENV_ARG_PARAMETER);
-  ACE_CHECK;
+  ImplementationRepository::Administration_var imr_locator;
+  {
+        // A recursive thread lock without using a recursive thread lock.
+        // Non_Servant_Upcall has a magic constructor and destructor.  We
+        // unlock the Object_Adapter lock for the duration of the servant
+        // activator upcalls; reacquiring once the upcalls complete.  Even
+        // though we are releasing the lock, other threads will not be able
+        // to make progress since
+        // <Object_Adapter::non_servant_upcall_in_progress_> has been set.
+        TAO_Object_Adapter::Non_Servant_Upcall non_servant_upcall (*this);
+        ACE_UNUSED_ARG (non_servant_upcall);
+
+        imr_locator = ImplementationRepository::Administration::_narrow (imr.in () ACE_ENV_ARG_PARAMETER);
+        ACE_CHECK;
+  }
 
   if (CORBA::is_nil(imr_locator.in ()))
       return;
@@ -4011,11 +4025,22 @@ TAO_POA::imr_notify_startup (ACE_ENV_SINGLE_ARG_DECL)
 
   ACE_TRY
     {
+      // A recursive thread lock without using a recursive thread lock.
+      // Non_Servant_Upcall has a magic constructor and destructor.  We
+      // unlock the Object_Adapter lock for the duration of the servant
+      // activator upcalls; reacquiring once the upcalls complete.  Even
+      // though we are releasing the lock, other threads will not be able
+      // to make progress since
+      // <Object_Adapter::non_servant_upcall_in_progress_> has been set.
+      TAO_Object_Adapter::Non_Servant_Upcall non_servant_upcall (*this);
+
       imr_locator->server_is_running (this->name().c_str (),
                                       partial_ior.c_str(),
                                       svr.in()
                                       ACE_ENV_ARG_PARAMETER);
       ACE_TRY_CHECK;
+
+      ACE_UNUSED_ARG (non_servant_upcall);
     }
   ACE_CATCH (CORBA::SystemException, sysex)
     {
@@ -4040,9 +4065,11 @@ void
 TAO_POA::imr_notify_shutdown (void)
 {
   // Notify the Implementation Repository about shutting down.
+
   CORBA::Object_var imr = this->orb_core ().implrepo_service ();
 
-  // Check to see if there was an imr returned.  If none, return ourselves.
+  // Check to see if there was an imr returned.  If none, return
+  // ourselves.
   if (CORBA::is_nil (imr.in ()))
     return;
 
@@ -4051,12 +4078,24 @@ TAO_POA::imr_notify_shutdown (void)
       if (TAO_debug_level > 0)
         ACE_DEBUG ((LM_DEBUG, "Notifing IMR of Shutdown server:%s\n", this->the_name()));
 
+      // A recursive thread lock without using a recursive thread lock.
+      // Non_Servant_Upcall has a magic constructor and destructor.  We
+      // unlock the Object_Adapter lock for the duration of the servant
+      // activator upcalls; reacquiring once the upcalls complete.  Even
+      // though we are releasing the lock, other threads will not be able
+      // to make progress since
+      // <Object_Adapter::non_servant_upcall_in_progress_> has been set.
+      TAO_Object_Adapter::Non_Servant_Upcall non_servant_upcall (*this);
+      ACE_UNUSED_ARG (non_servant_upcall);
+
       // Get the IMR's administrative object and call shutting_down on it
       ImplementationRepository::Administration_var imr_locator =
-        ImplementationRepository::Administration::_narrow (imr.in () ACE_ENV_ARG_PARAMETER);
+        ImplementationRepository::Administration::_narrow (imr.in ()
+                                                           ACE_ENV_ARG_PARAMETER);
       ACE_TRY_CHECK;
 
-      imr_locator->server_is_shutting_down (this->the_name () ACE_ENV_ARG_PARAMETER);
+      imr_locator->server_is_shutting_down (this->the_name ()
+                                            ACE_ENV_ARG_PARAMETER);
       ACE_TRY_CHECK;
     }
   ACE_CATCHANY