ChangeLogTag: Sun May 7 21:03:30 UTC 2006 Iliyan Jeliazkov <iliyan@ociweb.com>

8 files changed, 130 insertions, 126 deletions

diff --git a/ChangeLog b/ChangeLog
index 4dabe45c49f..d8488be78a4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,16 +1,34 @@
+Sun May  7 21:03:30 UTC 2006  Iliyan Jeliazkov  <iliyan@ociweb.com>
+
+        * ace/Service_Config.{h,cpp}:
+
+          Weeding out the remaining issues with single threaded
+          builds. The implementation now relies on the ACE_TSS_* macros to
+          abstract from the differences among build styles and platform
+          support for TSS.
+
+        * ace/Service_Gestalt.{h,cpp}:
+
+          Eliminated commented out code. Minor reformatting in places.
+
+        * ace/TSS_T.{h,inl,cpp}:
+
+          Added some comments to help others avoid a few pitfalls. Made
+          ts_init() a non-const, which eliminates a few const_cast's.
+
 Sun May  7 07:02:51 UTC 2006  Iliyan Jeliazkov  <iliyan@ociweb.com>
 
         * ace/Service_Config.h:
         * ace/Service_Config.cpp:
 
           Refactored the TSS usage (again) to fix a nasty
- 	  order-of-initialization problem. The gist of which is that, if
- 	  the ptr (to a template class instance) is a static member, in
- 	  static builds, it will be initialized by the process prologue
- 	  code *after* another static initializer has had the chance to
- 	  use and assign it a value. The solution was to use a method
- 	  scope static instance, which C++ guarantees to be initialized by
- 	  the (first) exit from that method.
+          order-of-initialization problem. The gist of which is that, if
+          the ptr (to a template class instance) is a static member, in
+          static builds, it will be initialized by the process prologue
+          code *after* another static initializer has had the chance to
+          use and assign it a value. The solution was to use a method
+          scope static instance, which C++ guarantees to be initialized by
+          the (first) exit from that method.
 
         * ace/Service_Gestalt.h:
         * ace/Service_Gestalt.cpp:
diff --git a/ace/Service_Config.cpp b/ace/Service_Config.cpp
index 3ca3f021fec..a613a5081e9 100644
--- a/ace/Service_Config.cpp
+++ b/ace/Service_Config.cpp
@@ -49,12 +49,6 @@ ACE_Service_Config_Guard::~ACE_Service_Config_Guard (void)
 
 ACE_ALLOC_HOOK_DEFINE (ACE_Service_Config)
 
-// A thread-specific storage to keep a pointer to the (current) global
-// configuration. Using a pointer to avoid the order of initialization
-// debacle possible when using static class instances. The memory is
-// dynamicaly allocated and leaked from current()
-
-
 // Set the signal handler to point to the handle_signal() function.
 ACE_Sig_Adapter *ACE_Service_Config::signal_handler_ = 0;
 
@@ -282,10 +276,15 @@ ACE_Service_Config::instance (void)
 }
 
 
+// A thread-specific storage to keep a pointer to the (current) global
+// configuration. Using a pointer to avoid the order of initialization
+// debacle possible when using static class instances. The memory is
+// dynamicaly allocated and leaked from current()
+
 /// Provides access to the static ptr, containing the TSS
 /// accessor. Ensures the desired order of initialization, even when
 /// other static initializers need the value.
-ACE_TSS< ACE_Service_Config::TSS_Resources > *& ACE_Service_Config::impl_ (void)
+ACE_Service_Config::TSS_Service_Gestalt_Ptr *& ACE_Service_Config::impl_ (void)
 {
   /// A "straight" static ptr does not work in static builds, because
   /// some static initializer may call current() method and assign
@@ -295,7 +294,7 @@ ACE_TSS< ACE_Service_Config::TSS_Resources > *& ACE_Service_Config::impl_ (void)
   /// static guarantees that the first time the method is invoked, the
   /// instance_ will be initialized before returning.
 
-  static ACE_TSS< ACE_Service_Config::TSS_Resources > *instance_ = 0;
+  static TSS_Service_Gestalt_Ptr *instance_ = 0;
 
   return instance_;
 }
@@ -313,10 +312,10 @@ ACE_Service_Config::current (void)
     {
       // TSS already initialized, but a new thread may need its own
       // ptr to the process-wide gestalt.
-      if ((*ACE_Service_Config::impl_ ())->ptr_ == 0)
+      if (ACE_TSS_GET (ACE_Service_Config::impl_ (), TSS_Resources)->ptr_ == 0)
         return current_i (global ());
 
-      return (*ACE_Service_Config::impl_ ())->ptr_;
+      return ACE_TSS_GET (ACE_Service_Config::impl_ (), TSS_Resources)->ptr_;
     }
   else
     {
@@ -329,10 +328,10 @@ ACE_Service_Config::current (void)
         {
           // Another thread snuck in and initialized the TSS, but we
           // still need ow own ptr to the process-wide gestalt.
-          if ((*ACE_Service_Config::impl_ ())->ptr_ == 0)
+          if (ACE_TSS_GET (ACE_Service_Config::impl_ (), TSS_Resources)->ptr_ == 0)
             return current_i (global ());
 
-          return (*ACE_Service_Config::impl_ ())->ptr_;
+          return ACE_TSS_GET (ACE_Service_Config::impl_ (), TSS_Resources)->ptr_;
         }
 
       return current_i (global ());
@@ -356,10 +355,10 @@ ACE_Service_Config::current_i (ACE_Service_Gestalt *newcurrent)
 {
   if (ACE_Service_Config::impl_ () == 0)
     {
-      ACE_NEW_RETURN (ACE_Service_Config::impl_ (), ACE_TSS < TSS_Resources >, 0);
+      ACE_NEW_RETURN (ACE_Service_Config::impl_ (), TSS_Service_Gestalt_Ptr, 0);
     }
 
-  (*ACE_Service_Config::impl_ ())->ptr_ = newcurrent;
+  ACE_TSS_GET (ACE_Service_Config::impl_ (), TSS_Resources)->ptr_ = newcurrent;
   return newcurrent;
 }
 
diff --git a/ace/Service_Config.h b/ace/Service_Config.h
index 0d5769627f8..a6607f4e1e4 100644
--- a/ace/Service_Config.h
+++ b/ace/Service_Config.h
@@ -227,10 +227,15 @@ private:
     ACE_Service_Gestalt *ptr_;
   };
 
+  /// A type for the TSS-stored resources. The typedef helps to
+  /// abstract from the particularities of single-threaded vs
+  /// multi-threaded environments.
+  typedef ACE_TSS_TYPE (ACE_Service_Config::TSS_Resources) TSS_Service_Gestalt_Ptr;
+
   /// Provides access to the static ptr, containing the TSS
   /// accessor. Ensures the desired order of initialization, even when
   /// other static initializers need the value.
-  static ACE_TSS< ACE_Service_Config::TSS_Resources > * & impl_ (void);
+  static TSS_Service_Gestalt_Ptr * & impl_ (void);
 
 protected:
 
diff --git a/ace/Service_Gestalt.cpp b/ace/Service_Gestalt.cpp
index b166dd37766..8396ddcfb11 100644
--- a/ace/Service_Gestalt.cpp
+++ b/ace/Service_Gestalt.cpp
@@ -59,21 +59,21 @@ private:
 };
 
 ACE_Service_Type_Forward_Declaration_Guard::ACE_Service_Type_Forward_Declaration_Guard
-(ACE_Service_Repository *r, const ACE_TCHAR *name)
-  : repo_ (r)
-  , name_ (name)
+  (ACE_Service_Repository *r, const ACE_TCHAR *name)
+    : repo_ (r)
+    , name_ (name)
 {
   ACE_ASSERT (this->repo_ != 0); // No repository specified?
   ACE_ASSERT (this->name_ != 0); // No name?
-
+  
   ACE_NEW_NORETURN (this->dummy_, // Allocate the forward declaration ...
                     ACE_Service_Type (this->name_,  // ... use the same name
                                       0,            // ... inactive
                                       this->dummy_dll_, // ... bogus ACE_DLL
                                       0));              // ... no type_impl
-
+  
   ACE_ASSERT (this->dummy_ != 0); // No memory?
-
+  
   if(ACE::debug ())
     ACE_DEBUG ((LM_DEBUG,
                 ACE_LIB_TEXT ("(%P|%t) FWDCL::start, repo=%@, \'%s\' ")
@@ -81,7 +81,7 @@ ACE_Service_Type_Forward_Declaration_Guard::ACE_Service_Type_Forward_Declaration
                 this->repo_,
                 this->name_,
                 this->dummy_));
-
+  
   // Note that the dummy_'s memory can disaper between invoking
   // the ctor and dtor, if the expected "real" dynamic service is
   // inserted in the repository.
@@ -100,12 +100,13 @@ ACE_Service_Type_Forward_Declaration_Guard::~ACE_Service_Type_Forward_Declaratio
   // We inserted it (as inactive), so we expect to find it, right?
   if (ret < 0 && ret != -2)
     {
-      ACE_ERROR ((LM_WARNING,
-      ACE_LIB_TEXT ("(%P|%t) FWDCL::end - Failed (%d) to find %s\n"),
-      ret, this->name_));
-      ACE_ASSERT (ret == -2 || ret >= 0);
+      if(ACE::debug ())
+        ACE_ERROR ((LM_WARNING,
+                    ACE_LIB_TEXT ("(%P|%t) FWDCL::end - Failed (%d) to find %s\n"),
+                    ret, this->name_));
+      return;
     }
-
+  
   if (tmp != 0 && tmp->type () != 0)
     {
       // Something has registered a proper (non-forward-decl) service with
@@ -115,51 +116,53 @@ ACE_Service_Type_Forward_Declaration_Guard::~ACE_Service_Type_Forward_Declaratio
       // actual implementation, so nothing is left to do. We are hereby giving
       // up any ownership claims.
       this->dummy_ = 0;
-
+      
       if(ACE::debug ())
-  {
-    ACE_DEBUG ((LM_DEBUG,
-          ACE_LIB_TEXT ("(%P|%t) FWDCL::end, repo=%@ - ")
-          ACE_LIB_TEXT ("Found different decl - "),
-          this->repo_,
-          this->name_));
-    tmp->dump ();
-  }
-
+        {
+          ACE_DEBUG ((LM_DEBUG,
+                      ACE_LIB_TEXT ("(%P|%t) FWDCL::end, repo=%@ - ")
+                      ACE_LIB_TEXT ("Found different decl - "),
+                      this->repo_,
+                      this->name_));
+          tmp->dump ();
+        }
+      
     }
   else
     {
       if(ACE::debug ())
-  {
-    ACE_DEBUG ((LM_DEBUG,
-          ACE_LIB_TEXT ("(%P|%t) FWDCL::end, repo=%@ - ")
-          ACE_LIB_TEXT ("Removing incomplete decl - "),
-          this->repo_,
-          this->name_));
-    this->dummy_->dump ();
-  }
-
+        {
+          ACE_DEBUG ((LM_DEBUG,
+                      ACE_LIB_TEXT ("(%P|%t) FWDCL::end, repo=%@ - ")
+                      ACE_LIB_TEXT ("Removing incomplete decl - "),
+                      this->repo_,
+                      this->name_));
+          this->dummy_->dump ();
+        }
+      
       // The (dummy) forward declaration is still there and is
       // the same, which means that no actual declaration was
       // provided inside the guarded scope. Therefore, the forward
       // declaration is no longer necessary.
       if (this->repo_->remove (this->name_,
-             const_cast< ACE_Service_Type**> (&this->dummy_)) == 0)
-  {
-    delete this->dummy_;
-  }
+                               const_cast< ACE_Service_Type**> (&this->dummy_)) == 0)
+        {
+          delete this->dummy_;
+        }
       else
-  {
-    ACE_ERROR ((LM_ERROR,
-          ACE_LIB_TEXT ("(%P|%t) FWDCL::end, repo=%@ - ")
-          ACE_LIB_TEXT ("Failed to remove incomplete decl"),
-          this->repo_,
-          this->name_));
-    this->dummy_->dump ();
-  }
+        {
+          if(ACE::debug ())
+            {
+              ACE_ERROR ((LM_ERROR,
+                          ACE_LIB_TEXT ("(%P|%t) FWDCL::end, repo=%@ - ")
+                          ACE_LIB_TEXT ("Failed to remove incomplete decl"),
+                          this->repo_,
+                          this->name_));
+              this->dummy_->dump ();
+            }
+        }
     }
-
-
+  
   // Clean up
   this->dummy_ = 0;
   this->repo_ = 0;
@@ -198,35 +201,6 @@ ACE_Service_Gestalt::ACE_Service_Gestalt (size_t size, bool svc_repo_is_owned, b
                     ACE_STATIC_SVCS);
 }
 
-// ACE_Service_Gestalt::ACE_Service_Gestalt (size_t size)
-//   : svc_repo_is_owned_ (true)
-//   , is_opened_ (0)
-//   , logger_key_ (ACE_DEFAULT_LOGGER_KEY)
-//   , no_static_svcs_ (1)
-//   , svc_queue_ (0)
-//   , svc_conf_file_queue_ (0)
-// {
-//   ACE_NEW_NORETURN (this->repo_,
-//                     ACE_Service_Repository (size));
-
-//   ACE_NEW_NORETURN (this->static_svcs_,
-//                     ACE_STATIC_SVCS);
-// }
-
-// ACE_Service_Gestalt::ACE_Service_Gestalt (void)
-//   : svc_repo_is_owned_ (false)
-//   , is_opened_ (0)
-//   , logger_key_ (ACE_DEFAULT_LOGGER_KEY)
-//   , no_static_svcs_ (1)
-//   , svc_queue_ (0)
-//   , svc_conf_file_queue_ (0)
-// {
-//   this->repo_ = ACE_Service_Repository::instance ();
-
-//   ACE_NEW_NORETURN (this->static_svcs_,
-//                     ACE_STATIC_SVCS);
-// }
-
 // Add the default statically-linked services to the Service
 // Repository.
 
@@ -261,25 +235,25 @@ ACE_Service_Gestalt::find_static_svc_descriptor (const ACE_TCHAR* name,
                                                  ACE_Static_Svc_Descriptor **ssd) const
 {
   ACE_TRACE ("ACE_Service_Gestalt::find_static_svc_descriptor");
-
+  
   if (this->static_svcs_ == 0)
     return -1;
-
+  
   ACE_Static_Svc_Descriptor **ssdp = 0;
   for (ACE_STATIC_SVCS_ITERATOR iter ( *this->static_svcs_);
        iter.next (ssdp) != 0;
        iter.advance ())
     {
       if (ACE_OS::strcmp ((*ssdp)->name_, name) == 0)
-  {
-    if (ssd != 0)
-      *ssd = *ssdp;
-
-    return 0;
-  }
+        {
+          if (ssd != 0)
+            *ssd = *ssdp;
+          
+          return 0;
+        }
     }
   return -1;
-
+  
 } /* find_static_svc_descriptor () */
 
 
diff --git a/ace/Service_Gestalt.h b/ace/Service_Gestalt.h
index b192adccff6..378dcc5e73a 100644
--- a/ace/Service_Gestalt.h
+++ b/ace/Service_Gestalt.h
@@ -58,14 +58,12 @@ public:
     MAX_SERVICES = ACE_DEFAULT_SERVICE_REPOSITORY_SIZE
   };
 
-//   /// Default constructor - associates the instance with the process-wide
-//   /// singleton instance of ACE_Service_Repository.
-//   ACE_Service_Gestalt (void);
-
-//   /// Creates an instance with a specified repository size. Takes ownership
-//   /// of the repository.
-//   ACE_Service_Gestalt (size_t size);
-  ACE_Service_Gestalt (size_t size, bool svc_repo_is_owned = true, bool no_static_svcs = true);
+  /// Constructor either associates the instance with the process-wide
+  /// singleton instance of ACE_Service_Repository, or creates and
+  /// manages its own instance of the specified size.
+  ACE_Service_Gestalt (size_t size, 
+                       bool svc_repo_is_owned = true, 
+                       bool no_static_svcs = true);
 
   /// Perform user-specified close activities and remove dynamic
   /// memory.
diff --git a/ace/TSS_T.cpp b/ace/TSS_T.cpp
index d6dc2599e8d..aa2160c28c9 100644
--- a/ace/TSS_T.cpp
+++ b/ace/TSS_T.cpp
@@ -92,16 +92,16 @@ ACE_TSS<TYPE>::cleanup (void *ptr)
 }
 
 template <class TYPE> int
-ACE_TSS<TYPE>::ts_init (void) const
+ACE_TSS<TYPE>::ts_init (void) 
 {
   // Ensure that we are serialized!
-  ACE_GUARD_RETURN (ACE_Thread_Mutex, ace_mon, (ACE_Thread_Mutex &) this->keylock_, 0);
+  ACE_GUARD_RETURN (ACE_Thread_Mutex, ace_mon, this->keylock_, 0);
 
   // Use the Double-Check pattern to make sure we only create the key
   // once!
   if (this->once_ == 0)
     {
-      if (ACE_Thread::keycreate (const_cast<ACE_thread_key_t *> (&this->key_),
+      if (ACE_Thread::keycreate (&this->key_,
 #if defined (ACE_HAS_THR_C_DEST)
                                  &ACE_TSS_C_cleanup,
 #else
@@ -111,9 +111,8 @@ ACE_TSS<TYPE>::ts_init (void) const
         return -1; // Major problems, this should *never* happen!
       else
         {
-          // This *must* come last to avoid race conditions!  Note that
-          // we need to "cast away const..."
-          * const_cast<int*> (&this->once_) = 1;
+          // This *must* come last to avoid race conditions!
+          this->once_ = 1;
           return 0;
         }
     }
@@ -184,7 +183,7 @@ ACE_TSS<TYPE>::ts_get (void) const
   if (this->once_ == 0)
     {
       // Create and initialize thread-specific ts_obj.
-      if (this->ts_init () == -1)
+      if (const_cast< ACE_TSS < TYPE > * >(this)->ts_init () == -1)
         // Seriously wrong..
         return 0;
     }
diff --git a/ace/TSS_T.h b/ace/TSS_T.h
index c43f18bf66b..27b8ed852f2 100644
--- a/ace/TSS_T.h
+++ b/ace/TSS_T.h
@@ -22,20 +22,21 @@
 # pragma once
 #endif /* ACE_LACKS_PRAGMA_ONCE */
 
-// This should probably go somewhere else, but it's only used here and in Thread_Manager.
+// This should probably go somewhere else, but it's only used here and
+// in Thread_Manager.
+// Note there is no ACE_TSS_SET because one would typicaly do
+// 'ACE_TSS_GET()->xyz_ = value', so the macro would have been too
+// complicated.
 # if defined (ACE_HAS_THREADS) && (defined (ACE_HAS_THREAD_SPECIFIC_STORAGE) || defined (ACE_HAS_TSS_EMULATION))
 #   define ACE_TSS_TYPE(T) ACE_TSS< T >
 #   if defined (ACE_HAS_BROKEN_CONVERSIONS)
 #     define ACE_TSS_GET(I, T) (*(I))
-#     define ACE_TSS_SET(I, T, V) (*(I) = (V))
 #   else
 #     define ACE_TSS_GET(I, T) ((I)->operator T * ())
-#     define ACE_TSS_SET(I, T, V) ((I)->operator T & () = (V))
 #   endif /* ACE_HAS_BROKEN_CONVERSIONS */
 # else
 #   define ACE_TSS_TYPE(T) T
 #   define ACE_TSS_GET(I, T) (I)
-#   define ACE_TSS_SET(I, T, V) ((I)=(V))
 # endif /* ACE_HAS_THREADS && (ACE_HAS_THREAD_SPECIFIC_STORAGE || ACE_HAS_TSS_EMULATION) */
 
 #include "ace/Thread_Mutex.h"
@@ -59,6 +60,15 @@ ACE_BEGIN_VERSIONED_NAMESPACE_DECL
  * allow a built-in type, others won't).  See template class
  * ACE_TSS_Type_Adapter, below, for adapting built-in types to work
  * with ACE_TSS.
+ *  
+ * @note Beware when creating static instances of this type
+ * (as with any other, btw). The unpredictable order of initialization
+ * across different platforms may cause a situation where you'd use
+ * the instance, before it is fully initialized. That's why typically
+ * instances of this type are dynamicaly allocated. On the stack it is
+ * typically allocated inside the ACE_Thread::svc() method which
+ * limits its lifetime appropriately.
+ *
  */
 template <class TYPE>
 class ACE_TSS
@@ -73,6 +83,7 @@ public:
    * thread will return this value.  This is useful since it enables
    * us to assign objects to thread-specific data that have
    * arbitrarily complex constructors.
+   *
    */
   ACE_TSS (TYPE *ts_obj = 0);
 
@@ -117,7 +128,7 @@ protected:
 
   /// Factors out common code for initializing TSS.  This must NOT be
   /// called with the lock held...
-  int ts_init (void) const;
+  int ts_init (void);
 
 #if !(defined (ACE_HAS_THREADS) && (defined (ACE_HAS_THREAD_SPECIFIC_STORAGE) || defined (ACE_HAS_TSS_EMULATION)))
   /// This implementation only works for non-threading systems...
diff --git a/ace/TSS_T.inl b/ace/TSS_T.inl
index d4378089137..1d74573c658 100644
--- a/ace/TSS_T.inl
+++ b/ace/TSS_T.inl
@@ -13,7 +13,7 @@ ACE_TSS<TYPE>::ACE_TSS (TYPE *type)
 }
 
 template <class TYPE> ACE_INLINE int
-ACE_TSS<TYPE>::ts_init (void) const
+ACE_TSS<TYPE>::ts_init (void) 
 {
   return 0;
 }