ChangeLogTag: Sun May 7 07:02:51 UTC 2006 Iliyan Jeliazkov <iliyan@ociweb.com>

5 files changed, 197 insertions, 104 deletions

diff --git a/ChangeLog b/ChangeLog
index 1be877685e6..16f4f98b9ef 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,24 @@
+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 pologue
+          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:
+
+          Refactored the construction to avoid using straight new in
+          constructor initializer lists. Replaced new with ACE_NEW_* where
+          appropriate.
+
 Sat May  6 18:08:13 UTC 2006  J.T. Conklin  <jtc@acorntoolworks.com>
 
         * configure.ac:
@@ -8,9 +29,9 @@ Sat May  6 18:08:13 UTC 2006  J.T. Conklin  <jtc@acorntoolworks.com>
 
 Sat May  6 09:52:06 2006  Douglas C. Schmidt  <schmidt@cse.wustl.edu>
 
-	* examples/DLL/test_dll.cpp: Renamed the DLLs and removed the "./"
-	  from the pathname.  Thanks to Sam and Roopa for reporting this
- 	  and tracking it down, respectively.
+        * examples/DLL/test_dll.cpp: Renamed the DLLs and removed the "./"
+          from the pathname.  Thanks to Sam and Roopa for reporting this
+          and tracking it down, respectively.
 
 Sat May  6 10:23:13 UTC 2006  Steve Huston  <shuston@riverace.com>
 
@@ -20,11 +41,11 @@ Sat May  6 10:23:13 UTC 2006  Steve Huston  <shuston@riverace.com>
 
 Tue Apr 25 10:22:16 2006  Douglas C. Schmidt  <schmidt@cse.wustl.edu>
 
-	* ace/OS_NS_string.cpp (strerror): Do a sanity check to make sure
-	  errnum >= 0 and < _sys_nerr and set errno to EINVAL if things
- 	  are not sane.  Thanks to Aleksandar Vukajlovic <vukajlo at
- 	  finsoft dot co dot yu> for this fix.
-  
+        * ace/OS_NS_string.cpp (strerror): Do a sanity check to make sure
+          errnum >= 0 and < _sys_nerr and set errno to EINVAL if things
+          are not sane.  Thanks to Aleksandar Vukajlovic <vukajlo at
+          finsoft dot co dot yu> for this fix.
+
 Fri May  5 19:37:12 UTC 2006  Johnny Willemsen  <jwillemsen@remedy.nl>
 
         * ace/Default_Constants.h:
diff --git a/ace/Service_Config.cpp b/ace/Service_Config.cpp
index d2b8156524e..3ca3f021fec 100644
--- a/ace/Service_Config.cpp
+++ b/ace/Service_Config.cpp
@@ -30,11 +30,9 @@ ACE_Service_Config_Guard::ACE_Service_Config_Guard (ACE_Service_Gestalt * psg)
                 this->saved_->repo_,
                 psg->repo_));
 
+  // Modify the TSS - no locking needed
   if (saved_ != psg)
-    {
-      // Modify the TSS - no locking needed
       (void)ACE_Service_Config::current (psg);
-    }
 }
 
 ///
@@ -55,7 +53,7 @@ ACE_ALLOC_HOOK_DEFINE (ACE_Service_Config)
 // 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()
-ACE_Service_Config::ACE_Service_Gestalt_TSS_Ptr *ACE_Service_Config::current_ (0);
+
 
 // Set the signal handler to point to the handle_signal() function.
 ACE_Sig_Adapter *ACE_Service_Config::signal_handler_ = 0;
@@ -275,6 +273,7 @@ ACE_Service_Config::global (void)
   return ACE_Singleton<ACE_Service_Config, ACE_SYNCH_MUTEX>::instance ();
 }
 
+
 ///
 ACE_Service_Gestalt *
 ACE_Service_Config::instance (void)
@@ -282,6 +281,25 @@ ACE_Service_Config::instance (void)
   return  ACE_Service_Config::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)
+{
+  /// A "straight" static ptr does not work in static builds, because
+  /// some static initializer may call current() method and assign
+  /// value to instance_ *before* the startup code has had a chance to
+  /// initialize it . This results in instance_ being "zeroed" out
+  /// after it was assigned the correct value. Having a method scoped
+  /// 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;
+
+  return instance_;
+}
+
 /// Return the configuration instance, considered "global" in the
 /// current thread. This may be the same as instance(), but on some
 /// occasions, it may be a different one. For example,
@@ -291,46 +309,58 @@ ACE_Service_Gestalt *
 ACE_Service_Config::current (void)
 {
 
-  // There is always one (ubergestalt) available per-process
-  ACE_Service_Gestalt *tmp = ACE_Service_Config::global ();
-
-  // If the Object_Manager is in transient state, the
-  // ACE_Service_Gestalt::current_ instance may not have been
-  // constructed yet (or may have been already destroyed). Either way
-  // there are no other threads.
-  if (ACE_Object_Manager::starting_up () || ACE_Object_Manager::shutting_down ())
-    return tmp;
-
-  if (ACE_Service_Config::current_ == 0)
+  if (ACE_Service_Config::impl_ () != 0)
     {
-      // Gotta make sure this piece of memory does not get leaked at process exi
-      ACE_Cleanup_Adapter< ACE_Service_Gestalt_TSS_Ptr > *tmp = 0;
-      ACE_NEW_RETURN (tmp, ACE_Cleanup_Adapter< ACE_Service_Gestalt_TSS_Ptr >, 0);
+      // TSS already initialized, but a new thread may need its own
+      // ptr to the process-wide gestalt.
+      if ((*ACE_Service_Config::impl_ ())->ptr_ == 0)
+        return current_i (global ());
 
-      // Register the instance for destruction at program termination.
-      ACE_Object_Manager::at_exit (tmp);
-
-      // Now just use the object, wrapped inside the cleanup adapter
-      ACE_Service_Config::current_ = &tmp->object ();
+      return (*ACE_Service_Config::impl_ ())->ptr_;
     }
+  else
+    {
+      // TSS not initialized yet - first thread to hit this, so doing
+      // the double-checked locking thing
+      ACE_MT (ACE_GUARD_RETURN (ACE_Recursive_Thread_Mutex, ace_mon,
+                                *ACE_Static_Object_Lock::instance (), 0));
 
-  ACE_Service_Gestalt *stored = ACE_TSS_GET (*current_, ACE_Service_Gestalt);
-  if (stored != 0)
-    return stored;
+      if (ACE_Service_Config::impl_ () != 0)
+        {
+          // 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)
+            return current_i (global ());
 
-  // Stash a pointer to the global configuration, so it can be returned out
-  // of TSS the next time a thread asks for it
-  ACE_Service_Config::current (tmp);
+          return (*ACE_Service_Config::impl_ ())->ptr_;
+        }
 
-  return tmp;
+      return current_i (global ());
+    }
 }
 
 /// A mutator to set the "current" (TSS) gestalt instance.
-int
+ACE_Service_Gestalt*
 ACE_Service_Config::current (ACE_Service_Gestalt *newcurrent)
 {
-  ACE_TSS_SET (*ACE_Service_Config::current_, ACE_Service_Gestalt*, newcurrent);
-  return 0;
+   ACE_MT (ACE_GUARD_RETURN (ACE_Recursive_Thread_Mutex, ace_mon,
+                             *ACE_Static_Object_Lock::instance (), 0));
+
+  return current_i (newcurrent);
+}
+
+/// A private, non-locking mutator to set the "current" (TSS) gestalt instance.
+/// Make sure to call with the proper locks held!
+ACE_Service_Gestalt*
+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_Service_Config::impl_ ())->ptr_ = newcurrent;
+  return newcurrent;
 }
 
 
@@ -397,19 +427,20 @@ ACE_Service_Config::resume (const ACE_TCHAR svc_name[])
 ACE_Service_Config::ACE_Service_Config (int ignore_static_svcs,
                                         size_t size,
                                         int signum)
+  : ACE_Service_Gestalt (size, false, ignore_static_svcs)
 {
   ACE_TRACE ("ACE_Service_Config::ACE_Service_Config");
 
-  this->no_static_svcs_ = (ignore_static_svcs);
+  //  this->no_static_svcs_ = (ignore_static_svcs);
 
   ACE_Service_Config::signum_ = signum;
 
   // Initialize the Service Repository.
-  ACE_Service_Repository::instance (static_cast<int> (size));
+  //  ACE_Service_Repository::instance (static_cast<int> (size));
 
   // Initialize the ACE_Reactor (the ACE_Reactor should be the same
   // size as the ACE_Service_Repository).
-  ACE_Reactor::instance ();
+  (void)ACE_Reactor::instance ();
 }
 
 
@@ -472,18 +503,20 @@ ACE_Service_Config::create_service_type_impl (const ACE_TCHAR *name,
 
 ACE_Service_Config::ACE_Service_Config (const ACE_TCHAR program_name[],
                                         const ACE_TCHAR *logger_key)
+  : ACE_Service_Gestalt (ACE_Service_Repository::DEFAULT_SIZE, false)
 {
   ACE_TRACE ("ACE_Service_Config::ACE_Service_Config");
 
   if (this->open (program_name,
-                  logger_key) == -1
-      && errno != ENOENT)
+                  logger_key) == -1 && errno != ENOENT)
+    {
 
-    // Only print out an error if it wasn't the svc.conf file that was
-    // missing.
-    ACE_ERROR ((LM_ERROR,
-                ACE_LIB_TEXT ("%p\n"),
-                program_name));
+      // Only print out an error if it wasn't the svc.conf file that was
+      // missing.
+      ACE_ERROR ((LM_ERROR,
+                  ACE_LIB_TEXT ("(%P|%t) SC failed to open: %p\n"),
+                  program_name));
+    }
 }
 
 // Signal handling API to trigger dynamic reconfiguration.
diff --git a/ace/Service_Config.h b/ace/Service_Config.h
index 33bfced8b88..0d5769627f8 100644
--- a/ace/Service_Config.h
+++ b/ace/Service_Config.h
@@ -121,6 +121,7 @@ public:
   bool operator!= (ACE_Static_Svc_Descriptor &) const;
 };
 
+
 #define ACE_Component_Config ACE_Service_Config
 
 /**
@@ -156,6 +157,7 @@ public:
  */
 class ACE_Export ACE_Service_Config: public ACE_Service_Gestalt
 {
+
 public:
 
   // = Initialization and termination methods.
@@ -213,7 +215,22 @@ protected:
 
   /// = Static interfaces
 
+private:
+
+  /// A private, non-locking mutator to set the "current" (TSS) gestalt instance.
+  /// Make sure to call with the proper locks held!
+  static ACE_Service_Gestalt* current_i (ACE_Service_Gestalt *newcurrent);
+
+  /// A Wrapper for the TSS-stored pointer.
+  struct TSS_Resources {
+    TSS_Resources (void) : ptr_ (0) {};
+    ACE_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);
 
 protected:
 
@@ -221,7 +238,7 @@ protected:
   /// classes, like ACE_Service_Config_Guard which when instantiated on the
   /// stack, can temporarily change which gestalt instance is viewed as
   /// global from the point of view of the static initializers in DLLs.
-  static int current (ACE_Service_Gestalt*);
+  static ACE_Service_Gestalt* current (ACE_Service_Gestalt*);
 
 
 public:
@@ -532,10 +549,6 @@ private:
   /// Handles the reconfiguration signals.
   static ACE_Sig_Adapter *signal_handler_;
 
-  /// Typedef for syntactic convenience
-  typedef ACE_TSS_TYPE (ACE_TSS_Type_Adapter< ACE_Service_Gestalt* >)
-    ACE_Service_Gestalt_TSS_Ptr;
-
   /// Pointer to the Singleton (ACE_Cleanup) Gestalt instance.
   /// There is thread-specific global instance pointer, which is used to
   /// temporarily change which Gestalt instance is used for static service
@@ -552,7 +565,6 @@ private:
   /// deallocated, but the global service repository will still "think"
   /// it must finalize the (DLL's) static services - with disastrous
   /// consequences, occurring in the post-main code (at_exit()).
-  static ACE_Service_Gestalt_TSS_Ptr *current_;
 
   /// This class needs the intimate access to be able to swap the
   /// current TSS pointer for the global Gestalt.
diff --git a/ace/Service_Gestalt.cpp b/ace/Service_Gestalt.cpp
index 7f64fa71d00..b166dd37766 100644
--- a/ace/Service_Gestalt.cpp
+++ b/ace/Service_Gestalt.cpp
@@ -173,37 +173,59 @@ ACE_Service_Gestalt::~ACE_Service_Gestalt (void)
 {
   ACE_ASSERT (this->repo_ != 0);
 
-  if (this->repo_is_owned_)
+  if (this->svc_repo_is_owned_)
     delete this->repo_;
 }
 
-ACE_Service_Gestalt::ACE_Service_Gestalt (size_t size)
-  : repo_ (new ACE_Service_Repository (size))
-  , repo_is_owned_ (true)
+ACE_Service_Gestalt::ACE_Service_Gestalt (size_t size, bool svc_repo_is_owned, bool no_static_svcs)
+  : svc_repo_is_owned_ (svc_repo_is_owned)
   , is_opened_ (0)
-  , svc_conf_file_queue_ (0)
-  , static_svcs_ (new ACE_STATIC_SVCS)
-  , svc_queue_ (0)
   , logger_key_ (ACE_DEFAULT_LOGGER_KEY)
-  , no_static_svcs_ (1)
-{
-  ACE_ASSERT (this->repo_ != 0);
-}
-
-ACE_Service_Gestalt::ACE_Service_Gestalt (void)
-  : repo_ (ACE_Service_Repository::instance ())
-  , repo_is_owned_ (false)
-  , is_opened_ (0)
-  , svc_conf_file_queue_ (0)
-  , static_svcs_ (new ACE_STATIC_SVCS)
+  , no_static_svcs_ (no_static_svcs)
   , svc_queue_ (0)
-  , logger_key_ (ACE_DEFAULT_LOGGER_KEY)
-  , no_static_svcs_ (1)
+  , svc_conf_file_queue_ (0)
 {
-  ACE_ASSERT (this->repo_ != 0);
-}
+  if (svc_repo_is_owned)
+    ACE_NEW_NORETURN (this->repo_,
+                      ACE_Service_Repository (size));
+  else
+    {
+      (void)ACE_Service_Repository::instance (size);
+      this->repo_ = ACE_Service_Repository::instance ();
+    }
 
+  ACE_NEW_NORETURN (this->static_svcs_,
+                    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.
@@ -343,15 +365,19 @@ ACE_Service_Type_Factory::make_service_type (ACE_Service_Gestalt *cfg) const
       if (stp == 0)
         ++yyerrno;
 
-      return new ACE_Service_Type (this->name (),
-                                   stp,
-                                   this->location_->dll (),
-                                   this->is_active_);
+      ACE_Service_Type *tmp = 0;
+      ACE_NEW_RETURN (tmp,
+                      ACE_Service_Type (this->name (),
+                                        stp,
+                                        this->location_->dll (),
+                                        this->is_active_),
+                      0);
+      return tmp;
     }
   else
     {
       ACE_ERROR ((LM_ERROR,
-                  ACE_LIB_TEXT ("Unable to find service: %s\n"),
+                  ACE_LIB_TEXT ("(%P|%t) Unable to find service \'%s\'\n"),
                   this->name ()));
       ++yyerrno;
       return 0;
diff --git a/ace/Service_Gestalt.h b/ace/Service_Gestalt.h
index 14912c2e589..b192adccff6 100644
--- a/ace/Service_Gestalt.h
+++ b/ace/Service_Gestalt.h
@@ -58,13 +58,14 @@ 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);
+//   /// 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);
+//   /// 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);
 
   /// Perform user-specified close activities and remove dynamic
   /// memory.
@@ -349,12 +350,9 @@ protected:
 
 protected:
 
-  /// The service repository to hold the services.
-  ACE_Service_Repository* const repo_;
-
   /// Do we own the service repository instance or have only been given a ptr
   /// to the singleton one?
-  bool repo_is_owned_;
+  bool svc_repo_is_owned_;
 
   /// Keep track of the number of times the instance has been
   /// initialized (opened). "If so, we can't allow <yyparse> to be called since
@@ -364,24 +362,27 @@ protected:
   /// <ACE_Service_Gestalt::close> method.
   int is_opened_;
 
+  /// Indicates where to write the logging output.  This is typically
+  /// either a STREAM pipe or a socket
+  const ACE_TCHAR *logger_key_;
+
+  /// Should we avoid loading the static services?
+  int no_static_svcs_;
+
+  /// Queue of services specified on the command-line.
+  ACE_SVC_QUEUE* svc_queue_;
+
   /** Queue of svc.conf files specified on the command-line.
    * @@ This should probably be made to handle unicode filenames...
    */
   ACE_SVC_QUEUE* svc_conf_file_queue_;
 
+  /// The service repository to hold the services.
+  ACE_Service_Repository* repo_;
+
   /// Repository of statically linked services.
   ACE_STATIC_SVCS* static_svcs_;
 
-  /// Queue of services specified on the command-line.
-  ACE_SVC_QUEUE* svc_queue_;
-
-  /// Indicates where to write the logging output.  This is typically
-  /// either a STREAM pipe or a socket
-  const ACE_TCHAR *logger_key_;
-
-  /// Should we avoid loading the static services?
-  int no_static_svcs_;
-
 }; /* class ACE_Service_Gestalt */