ChangeLogTag: Tue Aug 27 12:40:12 2002 Balachandran Natarajan <bala@isis-server.vuse.vanderbilt.edu>

7 files changed, 185 insertions, 82 deletions

diff --git a/TAO/ChangeLog b/TAO/ChangeLog
index 923d901eef3..08fd5678fef 100644
--- a/TAO/ChangeLog
+++ b/TAO/ChangeLog
@@ -1,3 +1,27 @@
+Tue Aug 27 12:40:12 2002  Balachandran Natarajan  <bala@isis-server.vuse.vanderbilt.edu>
+
+	Fix for [BUG 1265]. 
+	
+	* tao/Adapter.cpp:
+	* tao/Adapter.h: Methods check_close () and close () have an empty
+	  ACE_THROW_SPEC (()) to indicate that no exceptions pass through
+	  to the higher layer. 
+	  
+	* tao/ORB_Core.h:
+	* tao/ORB_Core.cpp (shutdown): Applied the patch in BUG 1265 which
+	  basically releases the mutex before calling actual close () on
+	  the adapter objects. Further, made sure that
+	  destroy_interceptors () and the shutdown () calls dont propagate
+	  exceptions to the caller. 
+
+  	  Please see the discussions in BUG 1265 and chapter 2 of
+	  "Exceptional C++" by Herb Sutter for details. 
+
+	* tao/ORB.cpp (shutdown): Added a comment as to why we cannot lock
+	  exceptions within this method. Doing so would render us
+	  non-compliant with the CORBA spec. 
+
+	 
 Tue Aug 27 10:22:50 2002  Balachandran Natarajan  <bala@isis-server.vuse.vanderbilt.edu>
 
 	* tests/ORB_shutdown/server.cpp: Fixed a compile error in builds
diff --git a/TAO/tao/Adapter.cpp b/TAO/tao/Adapter.cpp
index 833c3696a60..19eacac66c0 100644
--- a/TAO/tao/Adapter.cpp
+++ b/TAO/tao/Adapter.cpp
@@ -45,25 +45,54 @@ TAO_Adapter_Registry::~TAO_Adapter_Registry (void)
 void
 TAO_Adapter_Registry::close (int wait_for_completion
                              ACE_ENV_ARG_DECL)
+  ACE_THROW_SPEC (())
 {
-  for (size_t i = 0; i != this->adapters_count_; ++i)
+  ACE_TRY
     {
-      this->adapters_[i]->close (wait_for_completion ACE_ENV_ARG_PARAMETER);
-      ACE_CHECK;
+      for (size_t i = 0; i != this->adapters_count_; ++i)
+        {
+          this->adapters_[i]->close (wait_for_completion
+                                     ACE_ENV_ARG_PARAMETER);
+          ACE_TRY_CHECK;
+        }
     }
+  ACE_CATCHANY
+    {
+      if (TAO_debug_level > 3)
+        ACE_PRINT_EXCEPTION (ACE_ANY_EXCEPTION,
+                             "Exception in TAO_Adapter_Registry::close () \n");
+      return;
+    }
+  ACE_ENDTRY;
+
+  return;
 }
 
 void
 TAO_Adapter_Registry::check_close (int wait_for_completion
                                    ACE_ENV_ARG_DECL)
+  ACE_THROW_SPEC (())
 {
-  for (size_t i = 0; i != this->adapters_count_; ++i)
+  ACE_TRY
     {
-      this->adapters_[i]->check_close (wait_for_completion ACE_ENV_ARG_PARAMETER);
-      ACE_CHECK;
+      for (size_t i = 0; i != this->adapters_count_; ++i)
+        {
+          this->adapters_[i]->check_close (wait_for_completion
+                                           ACE_ENV_ARG_PARAMETER);
+          ACE_TRY_CHECK;
+        }
     }
-}
+  ACE_CATCHALL
+    {
+      if (TAO_debug_level > 3)
+        ACE_DEBUG ((LM_DEBUG,
+                    ACE_LIB_TEXT ("(%P|%t) Exception in TAO_Adapter_Registry::check_close () \n")));
+      return;
+    }
+  ACE_ENDTRY;
 
+  return;
+}
 void
 TAO_Adapter_Registry::insert (TAO_Adapter *adapter
                               ACE_ENV_ARG_DECL)
diff --git a/TAO/tao/Adapter.h b/TAO/tao/Adapter.h
index bf6c43a308e..b25c45d95aa 100644
--- a/TAO/tao/Adapter.h
+++ b/TAO/tao/Adapter.h
@@ -119,7 +119,8 @@ public:
    * POA while performing an upcall).
    */
   void close (int wait_for_completion
-              ACE_ENV_ARG_DECL);
+              ACE_ENV_ARG_DECL)
+    ACE_THROW_SPEC (());
 
   /**
    * Verify if the close() call can be invoked in the current
@@ -127,7 +128,8 @@ public:
    * Raise the right exception if not.
    */
   void check_close (int wait_for_completion
-                    ACE_ENV_ARG_DECL);
+                    ACE_ENV_ARG_DECL)
+    ACE_THROW_SPEC (());
 
   /// Insert a new adapter into the registry.
   void insert (TAO_Adapter *adapter
diff --git a/TAO/tao/ORB.cpp b/TAO/tao/ORB.cpp
index 79b3c941c03..67b90321570 100644
--- a/TAO/tao/ORB.cpp
+++ b/TAO/tao/ORB.cpp
@@ -174,12 +174,14 @@ void
 CORBA_ORB::shutdown (CORBA::Boolean wait_for_completion
                      ACE_ENV_ARG_DECL)
 {
+  // We cannot lock the exceptions here. We need to propogate
+  // BAD_INV_ORDER  exceptions if needed to the caller. Locking
+  // exceptions down would render us non-compliant with the spec.
   this->check_shutdown (ACE_ENV_SINGLE_ARG_PARAMETER);
   ACE_CHECK;
 
   this->orb_core ()->shutdown (wait_for_completion
                                ACE_ENV_ARG_PARAMETER);
-  ACE_CHECK;
 }
 
 void
@@ -201,8 +203,8 @@ CORBA_ORB::destroy (ACE_ENV_SINGLE_ARG_DECL)
   if (TAO_debug_level >= 3)
     {
       ACE_DEBUG ((LM_DEBUG,
-                  ACE_LIB_TEXT("CORBA::ORB::destroy() has been called on ORB <%s>.\n"),
-                  ACE_TEXT_CHAR_TO_TCHAR(this->orb_core ()->orbid ())));
+                  ACE_LIB_TEXT ("CORBA::ORB::destroy() has been called on ORB <%s>.\n"),
+                  ACE_TEXT_CHAR_TO_TCHAR (this->orb_core ()->orbid ())));
     }
 
   this->orb_core ()->destroy (ACE_ENV_SINGLE_ARG_PARAMETER);
diff --git a/TAO/tao/ORB_Core.cpp b/TAO/tao/ORB_Core.cpp
index 92adc2dab42..5f910f6fbf1 100644
--- a/TAO/tao/ORB_Core.cpp
+++ b/TAO/tao/ORB_Core.cpp
@@ -1764,21 +1764,35 @@ TAO_ORB_Core::run (ACE_Time_Value *tv,
 void
 TAO_ORB_Core::shutdown (CORBA::Boolean wait_for_completion
                         ACE_ENV_ARG_DECL)
+  ACE_THROW_SPEC (())
 {
-  ACE_GUARD (TAO_SYNCH_MUTEX, monitor, this->lock_);
-
-  if (this->has_shutdown () == 0)
+  ACE_TRY
     {
-      this->adapter_registry_.check_close (wait_for_completion
-                                           ACE_ENV_ARG_PARAMETER);
-      ACE_CHECK;
+      {
+        ACE_GUARD (TAO_SYNCH_MUTEX, monitor, this->lock_);
+
+        if (this->has_shutdown () != 0)
+          return;
+
+        // Check if we are on the right state, i.e. do not accept
+        // shutdowns with the 'wait_for_completion' flag set in the middle
+        // of an upcall (because those deadlock).
+        this->adapter_registry_.check_close (wait_for_completion
+                                             ACE_ENV_ARG_PARAMETER);
+        ACE_TRY_CHECK;
+
+        // Set the 'has_shutdown' flag, so any further attempt to shutdown
+        // becomes a noop.
+        this->has_shutdown_ = 1;
+
+        // need to release the mutex, because some of the shutdown
+        // operations invoke application code, that could (and in practice
+        // does!) callback into ORB Core code.
+      }
 
       this->adapter_registry_.close (wait_for_completion
                                      ACE_ENV_ARG_PARAMETER);
-      ACE_CHECK;
-
-      // Set the shutdown flag
-      this->has_shutdown_ = 1;
+      ACE_TRY_CHECK;
 
       // Shutdown reactor.
       this->thread_lane_resources_manager ().shutdown_reactor ();
@@ -1795,7 +1809,7 @@ TAO_ORB_Core::shutdown (CORBA::Boolean wait_for_completion
 
       // Invoke Interceptor::destroy() on all registered interceptors.
       this->destroy_interceptors (ACE_ENV_SINGLE_ARG_PARAMETER);
-      ACE_CHECK;
+      ACE_TRY_CHECK;
 
       // Explicitly destroy the object reference table since it
       // contains references to objects, which themselves may contain
@@ -1806,6 +1820,16 @@ TAO_ORB_Core::shutdown (CORBA::Boolean wait_for_completion
       this->pi_current_ = 0;  // For the sake of consistency.
 #endif  /* TAO_HAS_INTERCEPTORS == 1 */
     }
+  ACE_CATCHALL
+    {
+      // Do not allow exceptions to escape.. So catch all the
+      // exceptions.
+      // @@ Not sure what to print here for the users..
+
+    }
+  ACE_ENDTRY;
+
+  return;
 }
 
 void
@@ -1841,81 +1865,95 @@ TAO_ORB_Core::destroy (ACE_ENV_SINGLE_ARG_DECL)
 
 void
 TAO_ORB_Core::destroy_interceptors (ACE_ENV_SINGLE_ARG_DECL)
+  ACE_THROW_SPEC (())
 {
   size_t len = 0;   // The length of the interceptor array.
   size_t ilen = 0;  // The incremental length of the interceptor array.
 
+  ACE_TRY
+    {
 #if TAO_HAS_INTERCEPTORS == 1
-  TAO_ClientRequestInterceptor_List::TYPE &client_interceptors =
-    this->client_request_interceptors_.interceptors ();
+      TAO_ClientRequestInterceptor_List::TYPE &client_interceptors =
+        this->client_request_interceptors_.interceptors ();
 
-  len = client_interceptors.size ();
-  ilen = len;
-  for (size_t i = 0; i < len; ++i)
-    {
-      // Destroy the interceptors in reverse order in case the array
-      // list is only partially destroyed and another invocation
-      // occurs afterwards.
-      --ilen;
+      len = client_interceptors.size ();
+      ilen = len;
+      for (size_t i = 0; i < len; ++i)
+        {
+          // Destroy the interceptors in reverse order in case the array
+          // list is only partially destroyed and another invocation
+          // occurs afterwards.
+          --ilen;
 
-      client_interceptors[ilen]->destroy (ACE_ENV_SINGLE_ARG_PARAMETER);
-      ACE_CHECK;
+          client_interceptors[ilen]->destroy (ACE_ENV_SINGLE_ARG_PARAMETER);
+          ACE_TRY_CHECK;
 
-      // Since Interceptor::destroy() can throw an exception, decrease
-      // the size of the interceptor array incrementally since some
-      // interceptors may not have been destroyed yet.  Note that this
-      // size reduction is fast since no memory is actually
-      // deallocated.
-      client_interceptors.size (ilen);
-    }
+          // Since Interceptor::destroy() can throw an exception, decrease
+          // the size of the interceptor array incrementally since some
+          // interceptors may not have been destroyed yet.  Note that this
+          // size reduction is fast since no memory is actually
+          // deallocated.
+          client_interceptors.size (ilen);
+        }
 
-  TAO_ServerRequestInterceptor_List::TYPE &server_interceptors =
-    this->server_request_interceptors_.interceptors ();
+      TAO_ServerRequestInterceptor_List::TYPE &server_interceptors =
+        this->server_request_interceptors_.interceptors ();
 
-  len = server_interceptors.size ();
-  ilen = len;
-  for (size_t j = 0; j < len; ++j)
-    {
-      // Destroy the interceptors in reverse order in case the array
-      // list is only partially destroyed and another invocation
-      // occurs afterwards.
-      --ilen;
+      len = server_interceptors.size ();
+      ilen = len;
+      for (size_t j = 0; j < len; ++j)
+        {
+          // Destroy the interceptors in reverse order in case the array
+          // list is only partially destroyed and another invocation
+          // occurs afterwards.
+          --ilen;
 
-      server_interceptors[ilen]->destroy (ACE_ENV_SINGLE_ARG_PARAMETER);
-      ACE_CHECK;
+          server_interceptors[ilen]->destroy (ACE_ENV_SINGLE_ARG_PARAMETER);
+          ACE_TRY_CHECK;
 
-      // Since Interceptor::destroy() can throw an exception, decrease
-      // the size of the interceptor array incrementally since some
-      // interceptors may not have been destroyed yet.  Note that this
-      // size reduction is fast since no memory is actually
-      // deallocated.
-      server_interceptors.size (ilen);
-    }
+          // Since Interceptor::destroy() can throw an exception, decrease
+          // the size of the interceptor array incrementally since some
+          // interceptors may not have been destroyed yet.  Note that this
+          // size reduction is fast since no memory is actually
+          // deallocated.
+          server_interceptors.size (ilen);
+        }
 
 #endif  /* TAO_HAS_INTERCEPTORS == 1 */
 
-  TAO_IORInterceptor_List::TYPE &ior_interceptors =
-    this->ior_interceptors_.interceptors ();
+      TAO_IORInterceptor_List::TYPE &ior_interceptors =
+        this->ior_interceptors_.interceptors ();
 
-  len = ior_interceptors.size ();
-  ilen = len;
-  for (size_t k = 0; k < len; ++k)
-    {
-      // Destroy the interceptors in reverse order in case the array
-      // list is only partially destroyed and another invocation
-      // occurs afterwards.
-      --ilen;
+      len = ior_interceptors.size ();
+      ilen = len;
+      for (size_t k = 0; k < len; ++k)
+        {
+          // Destroy the interceptors in reverse order in case the array
+          // list is only partially destroyed and another invocation
+          // occurs afterwards.
+          --ilen;
 
-      ior_interceptors[ilen]->destroy (ACE_ENV_SINGLE_ARG_PARAMETER);
-      ACE_CHECK;
+          ior_interceptors[ilen]->destroy (ACE_ENV_SINGLE_ARG_PARAMETER);
+          ACE_TRY_CHECK;
 
-      // Since Interceptor::destroy() can throw an exception, decrease
-      // the size of the interceptor array incrementally since some
-      // interceptors may not have been destroyed yet.  Note that this
-      // size reduction is fast since no memory is actually
-      // deallocated.
-      ior_interceptors.size (ilen);
+          // Since Interceptor::destroy() can throw an exception, decrease
+          // the size of the interceptor array incrementally since some
+          // interceptors may not have been destroyed yet.  Note that this
+          // size reduction is fast since no memory is actually
+          // deallocated.
+          ior_interceptors.size (ilen);
+        }
     }
+  ACE_CATCHALL
+    {
+      // .. catch all the exceptions..
+      if (TAO_debug_level > 3)
+        ACE_DEBUG ((LM_DEBUG,
+                    ACE_LIB_TEXT ("(%P|%t) Exception in TAO_ORB_Core::destroy_interceptors () \n")));
+    }
+  ACE_ENDTRY;
+
+  return;
 }
 
 TAO_Thread_Lane_Resources &
diff --git a/TAO/tao/ORB_Core.h b/TAO/tao/ORB_Core.h
index 0b19dc8b1b8..2b7ddd16e86 100644
--- a/TAO/tao/ORB_Core.h
+++ b/TAO/tao/ORB_Core.h
@@ -671,7 +671,8 @@ public:
 
   /// End the event loop
   void shutdown (CORBA::Boolean wait_for_completion
-                 ACE_ENV_ARG_DECL);
+                 ACE_ENV_ARG_DECL)
+    ACE_THROW_SPEC (());
 
   /// Get the shutdown flag value
   int has_shutdown (void);
@@ -978,8 +979,10 @@ protected:
   void services_callbacks_init (void);
 
   /// Helper method that invokes Interceptor::destroy() on all
-  /// registered interceptors when ORB::destroy() is called.
-  void destroy_interceptors (ACE_ENV_SINGLE_ARG_DECL);
+  /// registered interceptors when ORB::destroy() is called. Prevents
+  /// exceptions from propagating up the call chain.
+  void destroy_interceptors (ACE_ENV_SINGLE_ARG_DECL)
+    ACE_THROW_SPEC (());
 
   /// Pointer to the list of protocol loaded into this ORB instance.
   /// Helper method to hold the common code part for -ORBEndpoint and
diff --git a/TAO/tests/ORB_shutdown/server.cpp b/TAO/tests/ORB_shutdown/server.cpp
index d110d0ff643..8e71ddc6f8a 100644
--- a/TAO/tests/ORB_shutdown/server.cpp
+++ b/TAO/tests/ORB_shutdown/server.cpp
@@ -75,6 +75,11 @@ main (int argc, char *argv[])
 
       orb->destroy (ACE_ENV_SINGLE_ARG_PARAMETER);
       ACE_TRY_CHECK;
+
+      // Calling again should never send throw an exception.
+      orb->shutdown (1
+                     ACE_ENV_ARG_PARAMETER);
+      ACE_TRY_CHECK;
     }
   ACE_CATCHANY
     {