From e8d586bcbfce0796c32729f8234a74ca44a5a5c4 Mon Sep 17 00:00:00 2001 From: iliyan Date: Mon, 25 Sep 2006 14:50:35 +0000 Subject: ChangeLogTag: Fri Sep 22 21:02:58 UTC 2006 Iliyan Jeliazkov --- ChangeLog | 79 +++++++++++ ace/DLL.cpp | 2 +- ace/DLL.h | 2 +- ace/DLL_Manager.cpp | 31 +++-- ace/Dynamic_Service_Base.cpp | 15 +- ace/Parse_Node.cpp | 29 ++-- ace/Service_Gestalt.cpp | 317 ++++++++++++++++++++++++++++--------------- ace/Service_Object.cpp | 33 +++-- ace/Service_Object.h | 5 +- ace/Service_Object.inl | 7 + ace/Service_Repository.cpp | 176 ++++++++++++++++-------- ace/Service_Repository.h | 34 ++++- 12 files changed, 518 insertions(+), 212 deletions(-) diff --git a/ChangeLog b/ChangeLog index a63b483848b..6b690b38a2e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,82 @@ +Fri Sep 22 21:02:58 UTC 2006 Iliyan Jeliazkov + + This change fixes bug#2612. See the description for the gory + details. In a nutshell, the problem is caused by the fact that + static services, loaded from a DLL, as part of a dynamic service + initialization can not be safely finalized when the dynamic + service's DLL gets unloaded. This was historically handled by + simply not unloading DLLs, and by finalizing all services at + process exit. With the introduction of the local configurations + feature, this changed. + + The finalization order was supposed to be guaranteed by the + ordering of the services in the repository. It really was a + kludge, because when dynamically loading and removing services + the order can change. Then whenever the process tries to clean + up, the dynamic service's DLL can be unloaded (because it gets + finalized first), and the static services' destructor code would + become inaccessible. The situation results in a SEGV at exit(). + + * ace/DLL.h: + * ace/DLL.cpp: + + Made assignment operator return a non-const reference to make it + well-formed. See C++ Standard, section "[special]". + + * ace/DLL_Manager.cpp: + * ace/Dynamic_Service_Base.cpp: + * ace/Parse_Node.cpp: + + Fixed formatting. + + * ace/Service_Gestalt.cpp: + + Added ACE_Service_Dynamic_Guard (formerly + ACE_Service_Type_Forward_Declaration_Guard) which helps to + resolve an issue with hybrid services, i.e. dynamic services, + accompanied by static services in the same DLL. Only automatic + instances of SDG are supposed to exist. Those are created during + (dynamic) service initialization and serve to: + + (a) Ensure the service we are loading is ordered last in the + repository, following any other services it may cause to + register, as part of its own registration. This is a common case + when loading dynamic services from DLLs - there are often static + initializers, which register static services. + + (b) The SDG instance destructor detects if the dynamic service + initialized successfully and "fixes-up" all the newly registered + static services to hold a reference to the DLL, from which they + have originated. + + Updated comments and formatting. + + * ace/Service_Object.h: + * ace/Service_Object.inl: + + Added void dll (const ACE_DLL&) to make it possible to + "relocate" services registered through static initializers found + in DLL's code segment(s). + + * ace/Service_Object.cpp: + + Modified ACE_Service_Type::fini() to ensure the DLL associated + with the service is properly closed. This in conjunction with + the gestalt changes above, makes it possible to safely and + completely unload a service. + + * ace/Service_Repository.h: + * ace/Service_Repository.cpp: + + Added relocate(), which allows association of a (static) service + objects with a DLL. Per changes to ACE_Service_Gestalt, + above. Added a counterpart private relocate_i(), which does not + obtain locks. + + * tests/Service_Config_Test.cpp: + + Minor test simplification. + Mon Sep 25 11:39:35 UTC 2006 Boris Kolpackov * ace/Bound_Ptr.inl: diff --git a/ace/DLL.cpp b/ace/DLL.cpp index 2d034635ac2..2492e2d0c93 100644 --- a/ace/DLL.cpp +++ b/ace/DLL.cpp @@ -48,7 +48,7 @@ ACE_DLL::ACE_DLL (const ACE_DLL &rhs) // Assignment operator -const ACE_DLL & +ACE_DLL & ACE_DLL::operator= (const ACE_DLL &rhs) { ACE_TRACE ("ACE_DLL::operator= (const ACE_DLL &)"); diff --git a/ace/DLL.h b/ace/DLL.h index 306b77635a4..be6b43b68ae 100644 --- a/ace/DLL.h +++ b/ace/DLL.h @@ -57,7 +57,7 @@ public: explicit ACE_DLL (int close_handle_on_destruction = 1); /// Allow assignment - const ACE_DLL& operator= (const ACE_DLL &rhs); + ACE_DLL& operator= (const ACE_DLL &rhs); /** diff --git a/ace/DLL_Manager.cpp b/ace/DLL_Manager.cpp index b796c4b6e55..0b4789bc8d3 100644 --- a/ace/DLL_Manager.cpp +++ b/ace/DLL_Manager.cpp @@ -223,9 +223,17 @@ ACE_DLL_Handle::open (const ACE_TCHAR *dll_name, } ++this->refcount_; + + if (ACE::debug ()) + ACE_DEBUG ((LM_DEBUG, + ACE_LIB_TEXT ("ACE (%P|%t) DLL_Handle::open - %s (%d), refcount=%d\n"), + this->dll_name_, + this->handle_, + this->refcount_)); return 0; } + int ACE_DLL_Handle::close (int unload) { @@ -244,8 +252,7 @@ ACE_DLL_Handle::close (int unload) if (ACE::debug ()) ACE_DEBUG ((LM_DEBUG, - ACE_LIB_TEXT ("ACE (%P|%t) DLL_Handle::close: ") - ACE_LIB_TEXT ("closing %s (%d), refcount is down to %d\n"), + ACE_LIB_TEXT ("ACE (%P|%t) DLL_Handle::close - %s (%d), refcount=%d\n"), this->dll_name_, this->handle_, this->refcount_)); @@ -265,9 +272,9 @@ ACE_DLL_Handle::close (int unload) ACE_Framework_Repository * frPtr= ACE_Framework_Repository::instance (); if (frPtr) - { - frPtr->remove_dll_components (this->dll_name_); - } + { + frPtr->remove_dll_components (this->dll_name_); + } retval = ACE_OS::dlclose (this->handle_); this->handle_ = ACE_SHLIB_INVALID_HANDLE; @@ -552,32 +559,32 @@ ACE_DLL_Manager::open_dll (const ACE_TCHAR *dll_name, ACE_DLL_Handle, 0); - dll_handle = temp_handle; + dll_handle = temp_handle; } } if (dll_handle) { if (dll_handle->open (dll_name, open_mode, handle) != 0) - { - // Error while openind dll. Free temp handle + { + // Error while openind dll. Free temp handle if (ACE::debug ()) ACE_ERROR ((LM_ERROR, ACE_LIB_TEXT ("ACE_DLL_Manager::open_dll: Could not ") ACE_LIB_TEXT ("open dll %s.\n"), dll_name)); - delete temp_handle; + delete temp_handle; return 0; } // Add the handle to the vector only if the dll is successfully // opened. if (temp_handle != NULL) - { - this->handle_vector_[this->current_size_] = dll_handle; + { + this->handle_vector_[this->current_size_] = dll_handle; this->current_size_++; - } + } } return dll_handle; diff --git a/ace/Dynamic_Service_Base.cpp b/ace/Dynamic_Service_Base.cpp index 35b963ca64d..4541a9b58bc 100644 --- a/ace/Dynamic_Service_Base.cpp +++ b/ace/Dynamic_Service_Base.cpp @@ -86,22 +86,21 @@ ACE_Dynamic_Service_Base::instance (const ACE_Service_Gestalt* repo, { type = svc_rec->type (); if (type != 0) - obj = type->object (); + obj = type->object (); } if (ACE::debug ()) { ACE_DEBUG ((LM_DEBUG, - ACE_LIB_TEXT ("(%P|%t) DSB::instance, repo=%@, name=%s, ") - ACE_LIB_TEXT ("type=%@ => %@"), - repo->repo_, name, type, obj)); + ACE_LIB_TEXT ("(%P|%t) DSB::instance, repo=%@, name=%s") + ACE_LIB_TEXT (" type=%@ => %@"), + repo->repo_, name, type, obj)); if (repo->repo_ != repo_found->repo_) - ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT (" [in repo=%@]\n"), - repo_found->repo_)); + ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT (" [in repo=%@]\n"), + repo_found->repo_)); else - ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT ("\n"))); - + ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT ("\n"))); } return obj; diff --git a/ace/Parse_Node.cpp b/ace/Parse_Node.cpp index c3b194499d4..f54d00957f3 100644 --- a/ace/Parse_Node.cpp +++ b/ace/Parse_Node.cpp @@ -783,19 +783,19 @@ ACE_Service_Type_Factory::make_service_type (ACE_Service_Gestalt *cfg) const u_int flags = ACE_Service_Type::DELETE_THIS | (this->location_->dispose () == 0 ? 0 : ACE_Service_Type::DELETE_OBJ); + int yyerrno = 0; ACE_Service_Object_Exterminator gobbler = 0; - int yyerrno = 0; void *sym = this->location_->symbol (cfg, yyerrno, &gobbler); if (sym != 0) { - ACE_Service_Type_Impl *stp - = ACE_Service_Config::create_service_type_impl (this->name (), - this->type_, - sym, - flags, - gobbler); + ACE_Service_Type_Impl *stp = + ACE_Service_Config::create_service_type_impl (this->name (), + this->type_, + sym, + flags, + gobbler); if (stp == 0) ++yyerrno; @@ -808,16 +808,15 @@ ACE_Service_Type_Factory::make_service_type (ACE_Service_Gestalt *cfg) const 0); return tmp; } - else - { + #ifndef ACE_NLOGGING - ACE_ERROR ((LM_ERROR, - ACE_LIB_TEXT ("(%P|%t) Unable to find service \'%s\'\n"), - this->name ())); + ACE_ERROR ((LM_ERROR, + ACE_LIB_TEXT ("(%P|%t) Unable to create ") + ACE_LIB_TEXT ("service object for %s\n"), + this->name ())); #endif - ++yyerrno; - return 0; - } + ++yyerrno; + return 0; } ACE_TCHAR const* diff --git a/ace/Service_Gestalt.cpp b/ace/Service_Gestalt.cpp index 2c8a83f3114..34aa9889c8e 100644 --- a/ace/Service_Gestalt.cpp +++ b/ace/Service_Gestalt.cpp @@ -33,40 +33,77 @@ ACE_RCSID (ace, Service_Gestalt, "$Id$") - ACE_BEGIN_VERSIONED_NAMESPACE_DECL - -// This is here, in the implementation, because it depends on the ACE_DLL type, -// which would be unnecessary to introduuce all over the place, had we declared -// this in the header file. - -// A forward service declaration guard. Used to declare a Service Type with a -// specific name in a Service Repository, which may later be replaced by the -// "real" Service Type. The only application is in the implementation of -// ACE_Service_Gestalt::initialize (), hence the declaration scoping in -// this file. -class ACE_Service_Type_Forward_Declaration_Guard +ACE_BEGIN_VERSIONED_NAMESPACE_DECL + +/// This is in the implementation file because it depends on the +/// ACE_DLL type, which would be unnecessary to introduuce all over +/// the place, had we declared this in the header file. + +/// @class ACE_Service_Type_Dynamic_Guard +/// +/// @brief A forward service declaration guard. +/// +/// Helps to resolve an issue with hybrid services, i.e. dynamic +/// services, accompanied by static services in the same DLL. Only +/// automatic instances of SDG are supposed to exist. Those are +/// created during (dynamic) service initialization and serve to: +/// +/// (a) Ensure the service we are loading is ordered last in the +/// repository, following any other services it may cause to register, +/// as part of its own registration. This is a common case when +/// loading dynamic services from DLLs - there are often static +/// initializers, which register static services. +/// +/// (b) The SDG instance destructor detects if the dynamic service +/// initialized successfully and "fixes-up" all the newly registered +/// static services to hold a reference to the DLL, from which they +/// have originated. + +class ACE_Service_Type_Dynamic_Guard { public: - ACE_Service_Type_Forward_Declaration_Guard (ACE_Service_Repository *r, - ACE_TCHAR const *name); + ACE_Service_Type_Dynamic_Guard (ACE_Service_Repository &r, + ACE_TCHAR const *name); - ~ACE_Service_Type_Forward_Declaration_Guard (void); + ~ACE_Service_Type_Dynamic_Guard (void); private: const ACE_DLL dummy_dll_; - ACE_Service_Repository *repo_; + ACE_Service_Repository & repo_; + size_t repo_begin_; ACE_TCHAR const * const name_; ACE_Service_Type const * dummy_; +# if defined (ACE_MT_SAFE) && (ACE_MT_SAFE != 0) + ACE_Guard< ACE_Recursive_Thread_Mutex > repo_monitor_; +#endif }; -ACE_Service_Type_Forward_Declaration_Guard::ACE_Service_Type_Forward_Declaration_Guard - (ACE_Service_Repository *r, const ACE_TCHAR *name) +ACE_Service_Type_Dynamic_Guard::ACE_Service_Type_Dynamic_Guard + (ACE_Service_Repository &r, const ACE_TCHAR *name) : repo_ (r) + , repo_begin_ (r.current_size ()) , name_ (name) +# if defined (ACE_MT_SAFE) && (ACE_MT_SAFE != 0) + // On this thread (for the duration of the initialize() method), + // we're about to do two things that require locking: (1) fiddle + // with the repository and (2) load a DLL and hence lock the + // DLL_Manager. + // + // Now if we don't lock the repo here, it is possible that two + // threads may deadlock on initialization because they can acquire + // locks (1) and (2) in different order, for instance: + // + // T1: loads a DLL (2) and registers a service (1); + // + // T2: may be relocating a service (1), which could lead to a + // (re)opening or uping the ref count on a DLL (2); + // + // To prevent this, we lock the repo here, using the repo_monitor_ + // member guard. + , repo_monitor_ (r.lock_) +#endif { - 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 @@ -77,33 +114,38 @@ ACE_Service_Type_Forward_Declaration_Guard::ACE_Service_Type_Forward_Declaration if(ACE::debug ()) ACE_DEBUG ((LM_DEBUG, - ACE_LIB_TEXT ("ACE (%P|%t) FWDCL::start, repo=%@, \'%s\' ") - ACE_LIB_TEXT ("- type=%@ (impl=(nil))\n"), - 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. - this->repo_->insert (this->dummy_); + ACE_LIB_TEXT ("ACE (%P|%t) STDG::, repo=%@ [%d], ") + ACE_LIB_TEXT ("name=%s, type=%@, impl=%@, object=%@, active=%d - inserting dummy forward\n"), + &this->repo_, this->repo_begin_, this->name_, this->dummy_, + this->dummy_->type (), + (this->dummy_->type () != 0) ? this->dummy_->type ()->object () : 0, + this->dummy_->active ())); + + // Note that the dummy_'s memory may be deallocated between invoking + // the ctor and dtor, if the expected ("real") dynamic service is + // inserted in the repository. See how this affects the destructor's + // behavior, below. + this->repo_.insert (this->dummy_); } -ACE_Service_Type_Forward_Declaration_Guard::~ACE_Service_Type_Forward_Declaration_Guard (void) + +/// Destructor + +ACE_Service_Type_Dynamic_Guard::~ACE_Service_Type_Dynamic_Guard (void) { const ACE_Service_Type *tmp = 0; // Lookup without ignoring suspended services. Making sure // not to ignore any inactive services, since those may be forward // declarations - int ret = this->repo_->find (this->name_, &tmp, 0); + int ret = this->repo_.find_i (this->name_, &tmp, 0); // We inserted it (as inactive), so we expect to find it, right? if (ret < 0 && ret != -2) { if(ACE::debug ()) ACE_ERROR ((LM_WARNING, - ACE_LIB_TEXT ("ACE (%P|%t) FWDCL::end - Failed (%d) to find %s\n"), + ACE_LIB_TEXT ("ACE (%P|%t) STDG:: - Failed (%d) to find %s\n"), ret, this->name_)); return; } @@ -111,62 +153,69 @@ ACE_Service_Type_Forward_Declaration_Guard::~ACE_Service_Type_Forward_Declaratio if (tmp != 0 && tmp->type () != 0) { // Something has registered a proper (non-forward-decl) service with - // the same name as our dummy. The ACE_Service_Gestalt::insert() modifies - // the memory for the previous ACE_Service_Type instance. It has in fact - // taken ownership and deleted the instance when it replaced it with the - // actual implementation, so nothing is left to do. We are hereby giving - // up any ownership claims. - this->dummy_ = 0; + // the same name as our dummy. if(ACE::debug ()) - { - ACE_DEBUG ((LM_DEBUG, - ACE_LIB_TEXT ("ACE (%P|%t) FWDCL::end, repo=%@ - ") - ACE_LIB_TEXT ("Found different decl - "), - this->repo_, - this->name_)); - tmp->dump (); - } + ACE_DEBUG ((LM_DEBUG, + ACE_LIB_TEXT ("ACE (%P|%t) STDG::, repo=%@, name=%s - updating [%d - %d]\n"), + &this->repo_, + this->name_, + this->repo_begin_, + this->repo_.current_size ())); + + // Relocate any static services. If any have been registered in + // the context of this guard, those really aren't static + // services because their code is in the DLL's code segment + this->repo_.relocate_i (this->repo_begin_, this->repo_.current_size (), tmp->dll()); + + // The ACE_Service_Gestalt::insert() modifies the memory for the + // original ACE_Service_Type instance. It deletes our dummy + // instance when replacing it with the actual implementation, so + // we are hereby simply giving up ownership. + this->dummy_ = 0; + if(ACE::debug ()) + ACE_DEBUG ((LM_DEBUG, + ACE_LIB_TEXT ("ACE (%P|%t) STDG::, repo=%@ [%d], ") + ACE_LIB_TEXT ("name=%s, type=%@, impl=%@, object=%@, active=%d - loaded\n"), + &this->repo_, this->repo_begin_, this->name_, tmp, tmp->type (), + (tmp->type () != 0) ? tmp->type ()->object () : 0, + tmp->active ())); } else { if(ACE::debug ()) - { - ACE_DEBUG ((LM_DEBUG, - ACE_LIB_TEXT ("ACE (%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 ("ACE (%P|%t) STDG::, repo=%@, ") + ACE_LIB_TEXT ("name=%s, type=%@, impl=%@, object=%@, active=%d - removing dummy forward\n"), + &this->repo_, this->name_, this->dummy_, this->dummy_->type (), + (this->dummy_->type () != 0) ? this->dummy_->type ()->object () : 0, + this->dummy_->active ())); // 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) + if (this->repo_.remove_i (this->name_, + const_cast< ACE_Service_Type**> (&this->dummy_)) == 0) { + // If it is a dummy then deleting it while holding the repo lock is okay. There will be no + // call to service object's fini() and no possibility for deadlocks. delete this->dummy_; } else { - if(ACE::debug ()) - { - ACE_ERROR ((LM_ERROR, - ACE_LIB_TEXT ("ACE (%P|%t) FWDCL::end, repo=%@ - ") - ACE_LIB_TEXT ("Failed to remove incomplete decl"), - this->repo_, - this->name_)); - this->dummy_->dump (); - } + ACE_ERROR ((LM_WARNING, + ACE_LIB_TEXT ("ACE (%P|%t) STDG::, repo=%@, name=%s, ") + ACE_LIB_TEXT ("type=%@, impl=%@, object=%@, active=%d - dummy remove failed\n"), + &this->repo_, this->name_, this->dummy_, this->dummy_->type (), + (this->dummy_->type () != 0) ? this->dummy_->type ()->object () : 0, + this->dummy_->active ())); } } // Clean up this->dummy_ = 0; - this->repo_ = 0; } @@ -180,20 +229,11 @@ Processed_Static_Svc (const ACE_Static_Svc_Descriptor *assd) { ACE_NEW_NORETURN (name_, ACE_TCHAR[ACE_OS::strlen(assd->name_)+1]); ACE_OS::strcpy(name_,assd->name_); -// if (ACE::debug ()) -// ACE_DEBUG ((LM_DEBUG, -// ACE_LIB_TEXT ("ACE (%P|%t) PSS::ctor - name = %s\n"), -// name_)); } ACE_Service_Gestalt::Processed_Static_Svc:: ~Processed_Static_Svc (void) { -// if (ACE::debug ()) -// ACE_DEBUG ((LM_DEBUG, -// ACE_LIB_TEXT ("ACE (%P|%t) PSS::dtor - name = %s\n"), -// name_)); - delete [] name_; } @@ -309,6 +349,8 @@ ACE_Service_Gestalt::find_static_svc_descriptor (const ACE_TCHAR* name, return -1; } +/// @brief + const ACE_Static_Svc_Descriptor* ACE_Service_Gestalt::find_processed_static_svc (const ACE_TCHAR* name) { @@ -323,10 +365,35 @@ ACE_Service_Gestalt::find_processed_static_svc (const ACE_TCHAR* name) return 0; } + + +/// @brief Captures a list of the direcives processed (explicitely) for this +/// Gestalt so that services can be replicated in other repositories +/// upon their first initialization. +/// +/// This is part of the mechanism ensuring distinct local instances +/// for static service objects, loaded in another repository. + void ACE_Service_Gestalt::add_processed_static_svc (const ACE_Static_Svc_Descriptor *assd) { + + /// When process_directive(Static_Svc_Descriptor&) is called, it + /// associates a service object with the Gestalt and makes the + /// resource (a Service Object) local to the repository. This is but + /// the first step in using such SO. The next is the + /// "initialization" step. It is typicaly done through a "static" + /// service configuration directive. + /// + /// In contrast a "dynamic" directive, when processed through the + /// overloaded process_directives(string) both creates the SO + /// locally and initializes it, where the statis directive must + /// first locate the SO and then calls the init() method. This means + /// that durig the "static" initialization there's no specific + /// information about the hosting repository and the gestalt must + /// employ some lookup strategy to find it elsewhere. + if (this->processed_static_svcs_ == 0) ACE_NEW (this->processed_static_svcs_, ACE_PROCESSED_STATIC_SVCS); @@ -345,26 +412,41 @@ ACE_Service_Gestalt::add_processed_static_svc Processed_Static_Svc *tmp = 0; ACE_NEW (tmp,Processed_Static_Svc(assd)); this->processed_static_svcs_->insert(tmp); + + if (ACE::debug ()) + ACE_DEBUG ((LM_DEBUG, + ACE_LIB_TEXT ("ACE (%P|%t) SG::add_processed_statisc_svc, ") + ACE_LIB_TEXT ("repo=%@ - %s\n"), + this->repo_, + assd->name_)); } + +/// Queues static service object descriptor which, during open() will +/// be given to process_directive() to create the Service +/// Object. Normally, only called from static initializers, prior to +/// calling open(). + int ACE_Service_Gestalt::insert (ACE_Static_Svc_Descriptor *stsd) { if (ACE::debug ()) { + static int pid = ACE_OS::getpid (); + // If called during static initialization ACE_Log_Msg may not // have been initialized yet, so use printf intead. Using a "//" // prefix in case the executable is a C++ code generator and the // output gets embedded in the generated code. ACE_OS::fprintf (stderr, "// (%d|0) SG::insert" - " repo=%p, name=%s - queuing a Static_Svc_Descriptor:" - " active=%d, repo opened=%d.\n", - ACE_OS::getpid (), + " repo=%p (opened=%d) - enqueue %s," + " active=%d.\n", + pid, this->repo_, + this->is_opened_, stsd->name_, - stsd->active_, - this->is_opened_); + stsd->active_); } // Inserting a service after the Gestalt has been opened makes it @@ -441,9 +523,9 @@ ACE_Service_Gestalt::initialize (const ACE_TCHAR *svc_name, { // ... report and remove this entry. ACE_ERROR ((LM_ERROR, - ACE_LIB_TEXT ("ACE (%P|%t) SG::initialize - static init of \'%s\'") - ACE_LIB_TEXT (" failed (%p)\n"), - svc_name)); + ACE_LIB_TEXT ("ACE (%P|%t) SG::initialize - static init of \'%s\'") + ACE_LIB_TEXT (" failed (%p)\n"), + svc_name)); this->repo_->remove (svc_name); return -1; } @@ -516,18 +598,23 @@ ACE_Service_Gestalt::initialize (const ACE_Service_Type_Factory *stf, // registered *after* the dynamic service that loads them, so that // their finalization is complete *before* finalizing the dynamic // service. - ACE_Service_Type_Forward_Declaration_Guard dummy (this->repo_, - stf->name ()); + ACE_Service_Type_Dynamic_Guard dummy (*this->repo_, + stf->name ()); // make_service_type() is doing the dynamic loading and also runs // any static initializers ACE_Auto_Ptr tmp (stf->make_service_type (this)); + if (tmp.get () != 0 && this->initialize_i (tmp.get (), parameters) == 0) { - // All good the ACE_Service_Type instance is now owned by the repository - // and we should make sure it is not destroyed upon exit from this method. - (void)tmp.release (); + // All good. Tthe ACE_Service_Type instance is now owned by the + // repository and we should make sure it is not destroyed upon + // exit from this method. + tmp.release (); + + + return 0; } @@ -676,16 +763,6 @@ int ACE_Service_Gestalt::process_directive_i (const ACE_Static_Svc_Descriptor &ssd, int force_replace) { -#ifndef ACE_NLOGGING - if (ACE::debug ()) - ACE_DEBUG ((LM_DEBUG, - ACE_LIB_TEXT ("ACE (%P|%t) SG::process_directive, ") - ACE_LIB_TEXT ("repo=%@, replace=%d - %s\n"), - this->repo_, - force_replace, - ssd.name_)); -#endif - if (!force_replace) { if (this->repo_->find (ssd.name_, 0, 0) >= 0) @@ -695,6 +772,7 @@ ACE_Service_Gestalt::process_directive_i (const ACE_Static_Svc_Descriptor &ssd, } } + ACE_Service_Object_Exterminator gobbler; void *sym = (ssd.alloc_)(&gobbler); @@ -711,7 +789,9 @@ ACE_Service_Gestalt::process_directive_i (const ACE_Static_Svc_Descriptor &ssd, ACE_Service_Type *service_type; // This is just a temporary to force the compiler to use the right - // constructor in ACE_Service_Type + // constructor in ACE_Service_Type. Note that, in cases where we are + // called from a static initializer which is part of a DLL, there is + // not enough information about the actuall DLL in this context. ACE_DLL tmp_dll; ACE_NEW_RETURN (service_type, @@ -721,6 +801,17 @@ ACE_Service_Gestalt::process_directive_i (const ACE_Static_Svc_Descriptor &ssd, ssd.active_), -1); +#ifndef ACE_NLOGGING + if (ACE::debug ()) + ACE_DEBUG ((LM_DEBUG, + ACE_LIB_TEXT ("ACE (%P|%t) SG::process_directive_i, ") + ACE_LIB_TEXT ("repo=%@ - %s, dll=%s, force=%d\n"), + this->repo_, + ssd.name_, + (tmp_dll.dll_name_ == 0) ? ACE_LIB_TEXT ("") : tmp_dll.dll_name_, + force_replace)); +#endif + return this->repo_->insert (service_type); } @@ -810,15 +901,16 @@ ACE_Service_Gestalt::process_file (const ACE_TCHAR file[]) if (this->repo_->find (file, 0, 0) >=0) { ACE_DEBUG ((LM_WARNING, - ACE_TEXT ("ACE (%P|%t) Configuration file %s is currently") - ACE_TEXT (" being processed. Ignoring recursive process_file().\n"), - file)); + ACE_TEXT ("ACE (%P|%t) Configuration file %s is currently") + ACE_TEXT (" being processed. Ignoring recursive process_file().\n"), + file)); return 0; } // Register a dummy service as a forward decl, using the file name as name. // The entry will be automaticaly removed once the thread exits this block. - ACE_Service_Type_Forward_Declaration_Guard recursion_guard (this->repo_, file); + ACE_Service_Type_Dynamic_Guard recursion_guard (*this->repo_, + file); /* * @TODO: Test with ACE_USES_CLASSIC_SVC_CONF turned off! @@ -877,11 +969,13 @@ ACE_Service_Gestalt::process_directive (const ACE_TCHAR directive[]) { ACE_TRACE ("ACE_Service_Gestalt::process_directive"); +#ifndef ACE_NLOGGING if (ACE::debug ()) ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT ("ACE (%P|%t) SG::process_directive, repo=%@ - %s\n"), this->repo_, directive)); +#endif #if (ACE_USES_CLASSIC_SVC_CONF == 1) ACE_UNUSED_ARG (directive); @@ -927,11 +1021,14 @@ ACE_Service_Gestalt::init_svc_conf_file_queue (void) this->svc_conf_file_queue_ = tmp; } +#ifndef ACE_NLOGGING if (ACE::debug ()) ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT ("ACE (%P|%t) SG::init_svc_conf_file_queue ") ACE_LIB_TEXT ("- this=%@, repo=%@\n"), this, this->repo_)); +#endif + return 0; } /* init_svc_conf_file_queue () */ @@ -955,11 +1052,13 @@ ACE_Service_Gestalt::open_i (const ACE_TCHAR /*program_name*/[], u_long old_thread_mask = log_msg->priority_mask (ACE_Log_Msg::THREAD); +#ifndef ACE_NLOGGING if (ACE::debug ()) ACE_DEBUG ((LM_DEBUG, ACE_TEXT ("ACE (%P|%t) SG::open_i - this=%@, ") ACE_TEXT ("opened=%d, loadstatics=%d\n"), this, this->is_opened_, this->no_static_svcs_)); +#endif // Guard against reentrant processing. For example, // if the singleton gestalt (ubergestalt) was already open, @@ -1029,8 +1128,8 @@ ACE_Service_Gestalt::process_commandline_directives (void) if (this->process_directive ((sptr->fast_rep ())) != 0) { ACE_ERROR ((LM_ERROR, - ACE_LIB_TEXT ("ACE (%P|%t) %p\n"), - ACE_LIB_TEXT ("process_directive"))); + ACE_LIB_TEXT ("ACE (%P|%t) %p\n"), + ACE_LIB_TEXT ("process_directive"))); result = -1; } } @@ -1167,11 +1266,13 @@ ACE_Service_Gestalt::close (void) delete this->svc_conf_file_queue_; this->svc_conf_file_queue_ = 0; +#ifndef ACE_NLOGGING // Delete the dynamically allocated static_svcs instance. if (ACE::debug ()) ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT ("ACE (%P|%t) SG::close - this=%@, repo=%@, pss = %@\n"), this, this->repo_, this->processed_static_svcs_)); +#endif delete this->static_svcs_; this->static_svcs_ = 0; @@ -1189,11 +1290,13 @@ ACE_Service_Gestalt::close (void) } delete this->processed_static_svcs_; this->processed_static_svcs_ = 0; + +#ifndef ACE_NLOGGING if (ACE::debug ()) ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT ("ACE (%P|%t) SG::close - complete this=%@, repo=%@\n"), this, this->repo_)); - +#endif return 0; diff --git a/ace/Service_Object.cpp b/ace/Service_Object.cpp index fbdc5296564..6e3d5bd9efc 100644 --- a/ace/Service_Object.cpp +++ b/ace/Service_Object.cpp @@ -87,20 +87,29 @@ ACE_Service_Type::~ACE_Service_Type (void) int ACE_Service_Type::fini (void) { - if (!this->fini_already_called_) + if (this->fini_already_called_) + return 0; + + this->fini_already_called_ = 1; + + if (this->type_ == 0) { - this->fini_already_called_ = 1; - if (this->type_ != 0) - return this->type_->fini (); - else - return 1; // No implementation was found. - // Currently only makes sense for dummy ST, used to "reserve" - // a spot (kind of like forward-declarations) for a dynamic - // service. This is necessary to help enforce the correct - // finalization order, when such service also has any - // (dependent) static services + // Returning 1 currently only makes sense for dummy instances, used + // to "reserve" a spot (kind of like forward-declarations) for a + // dynamic service. This is necessary to help enforce the correct + // finalization order, when such service also has any (dependent) + // static services + + return 1; // No implementation was found. } - return 0; + + int ret = this->type_->fini (); + + // Ensure that closing the DLL is done after type_->fini() as it may + // require access to the code for the service object destructor, + // which resides in the DLL + return (ret | this->dll_.close()); + } int diff --git a/ace/Service_Object.h b/ace/Service_Object.h index 027e217bde7..abc4781d9c8 100644 --- a/ace/Service_Object.h +++ b/ace/Service_Object.h @@ -126,7 +126,10 @@ public: void dump (void) const; /// Get to the DLL's implentation - const ACE_DLL & dll () const; + const ACE_DLL & dll (void) const; + + /// Sets the DLL + void dll (const ACE_DLL&); /// Declare the dynamic allocation hooks. ACE_ALLOC_HOOK_DECLARE; diff --git a/ace/Service_Object.inl b/ace/Service_Object.inl index a675d4e942b..5e58bc57588 100644 --- a/ace/Service_Object.inl +++ b/ace/Service_Object.inl @@ -65,8 +65,15 @@ ACE_Service_Type::fini_called (void) const ACE_INLINE const ACE_DLL & ACE_Service_Type::dll () const { + ACE_TRACE ("ACE_Service_Type::dll"); return this->dll_; } +ACE_INLINE void ACE_Service_Type::dll (const ACE_DLL &adll) +{ + ACE_TRACE ("ACE_Service_Type::dll"); + this->dll_ = adll; +} + ACE_END_VERSIONED_NAMESPACE_DECL diff --git a/ace/Service_Repository.cpp b/ace/Service_Repository.cpp index 2c607f225df..9367cdcc296 100644 --- a/ace/Service_Repository.cpp +++ b/ace/Service_Repository.cpp @@ -68,7 +68,6 @@ ACE_Service_Repository::instance (size_t size /* = ACE_Service_Repository::DEFAU } } - // ACE_ASSERT (ACE_Service_Repository::svc_rep_ != 0); return ACE_Service_Repository::svc_rep_; } @@ -151,28 +150,24 @@ ACE_Service_Repository::fini (void) // order. for (int i = this->current_size_ - 1; i >= 0; i--) - { - ACE_Service_Type *s = - const_cast (this->service_vector_[i]); - - if (ACE::debug ()) - { - ACE_DEBUG ((LM_DEBUG, - ACE_LIB_TEXT ("(%P|%t) SR::fini, %@ [%d] (%d): "), - this, i, this->total_size_)); - s->dump(); - } - - // Collect any errors. - int ret = s->fini (); - if (ACE::debug ()) - { - ACE_DEBUG ((LM_DEBUG, - ACE_LIB_TEXT ("(%P|%t) SR::fini, returned %d\n"), - ret)); - } - retval += ret; - } + { + ACE_Service_Type *s = + const_cast (this->service_vector_[i]); + +#ifndef ACE_NLOGGING + if (ACE::debug ()) + ACE_DEBUG ((LM_DEBUG, + ACE_LIB_TEXT ("ACE (%P|%t) SR::fini, repo=%@ [%d] (%d), ") + ACE_LIB_TEXT ("name=%s, type=%@, impl=%@, object=%@, active=%d\n"), + this, i, this->total_size_, s->name(), s->type (), + (s->type () != 0) ? s->type ()->object () : 0, + s->active ())); +#endif + + // Collect any errors. + int ret = s->fini (); + retval += ret; + } } return (retval == 0) ? 0 : -1; @@ -194,21 +189,25 @@ ACE_Service_Repository::close (void) // compaction. However, the common case is not to remove // services, so typically they are deleted in reverse order. +#ifndef ACE_NLOGGING if(ACE::debug ()) ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT ("(%P|%t) SR::close, this=%@, size=%d\n"), this, this->current_size_)); +#endif for (int i = this->current_size_ - 1; i >= 0; i--) { + +#ifndef ACE_NLOGGING if(ACE::debug ()) ACE_DEBUG ((LM_DEBUG, ACE_LIB_TEXT ("(%P|%t) SR::close, this=%@, delete so[%d]=%@ (%s)\n"), - this, - i, + this, i, this->service_vector_[i], this->service_vector_[i]->name ())); +#endif ACE_Service_Type *s = const_cast (this->service_vector_[i]); --this->current_size_; @@ -226,8 +225,10 @@ ACE_Service_Repository::close (void) ACE_Service_Repository::~ACE_Service_Repository (void) { ACE_TRACE ("ACE_Service_Repository::~ACE_Service_Repository"); +#ifndef ACE_NLOGGING if(ACE::debug ()) ACE_DEBUG ((LM_DEBUG, "(%P|%t) SR::, this=%@\n", this)); +#endif this->close (); } @@ -271,6 +272,45 @@ ACE_Service_Repository::find_i (const ACE_TCHAR name[], return -1; } + +/// @brief Relocate (a static) service to another DLL. +/// +/// Works by having the service type keep a reference to a specific +/// DLL. No locking, caller makes sure calling it is safe. You can +/// forcefully relocate any DLLs in the given range, not only the +/// static ones - but that will cause Very Bad Things (tm) to happen. + +int +ACE_Service_Repository::relocate_i (size_t begin, + size_t end, + const ACE_DLL& adll, + bool static_only) +{ + ACE_SHLIB_HANDLE new_handle = adll.get_handle (0); + + for (size_t i = begin; i < end; i++) + { + ACE_Service_Type *type = + const_cast (this->service_vector_[i]); + + ACE_SHLIB_HANDLE old_handle = type->dll ().get_handle (0); + if (static_only && old_handle != ACE_SHLIB_INVALID_HANDLE) + continue; + +#ifndef ACE_NLOGGING + if (ACE::debug ()) + ACE_DEBUG ((LM_DEBUG, + ACE_TEXT ("ACE (%P|%t) SR::relocate, repo=%@ [%d] (size=%d): name=%s - DLL from=%d to=%d\n"), + this, i, this->total_size_, type->name (), + old_handle, + new_handle)); +#endif + type->dll (adll); + } + + return 0; +} + int ACE_Service_Repository::find (const ACE_TCHAR name[], const ACE_Service_Type **srp, @@ -295,11 +335,11 @@ ACE_Service_Repository::insert (const ACE_Service_Type *sr) int return_value = -1; ACE_Service_Type *s = 0; + size_t i = 0; { // @TODO: Do we need a recursive mutex here? ACE_MT (ACE_GUARD_RETURN (ACE_Recursive_Thread_Mutex, ace_mon, this->lock_, -1)); - size_t i; // Check to see if this is a duplicate. for (i = 0; i < this->current_size_; i++) @@ -326,27 +366,30 @@ ACE_Service_Repository::insert (const ACE_Service_Type *sr) return_value = 0; } +#ifndef ACE_NLOGGING if (ACE::debug ()) - { - ACE_DEBUG ((LM_DEBUG, - ACE_TEXT ("ACE (%P|%t) SR::insert, repo=%@ [%d] (size=%d): "), - this, - i, - this->total_size_)); - sr->dump(); - } + ACE_DEBUG ((LM_DEBUG, + ACE_LIB_TEXT ("ACE (%P|%t) SR::insert, repo=%@ [%d] (%d), ") + ACE_LIB_TEXT ("name=%s, type=%@, impl=%@, object=%@, active=%d\n"), + this, i, this->total_size_, sr->name(), + sr->type (), + (sr->type () != 0) ? sr->type ()->object () : 0, + sr->active ())); +#endif } // Delete outside the lock if (s != 0) { +#ifndef ACE_NLOGGING if (ACE::debug ()) - { - ACE_DEBUG ((LM_DEBUG, - ACE_TEXT ("ACE (%P|%t) SR::insert, repo=%@ - destroying the former: "), - s)); - s->dump(); - } + ACE_DEBUG ((LM_DEBUG, + ACE_LIB_TEXT ("ACE (%P|%t) SR::insert - destroying (replacing), repo=%@ [%d] (%d), ") + ACE_LIB_TEXT ("name=%s, type=%@, impl=%@, object=%@, active=%d\n"), + this, i, this->total_size_, s->name(), s->type (), + (s->type () != 0) ? s->type ()->object () : 0, + s->active ())); +#endif delete s; } @@ -391,10 +434,38 @@ ACE_Service_Repository::suspend (const ACE_TCHAR name[], } +/** + * @brief Completely remove a entry from the Repository and + * dynamically unlink it if it was originally dynamically linked. + */ + +int +ACE_Service_Repository::remove (const ACE_TCHAR name[], ACE_Service_Type **ps) +{ + ACE_TRACE ("ACE_Service_Repository::remove"); + ACE_Service_Type *s = 0; + { + ACE_MT (ACE_GUARD_RETURN (ACE_Recursive_Thread_Mutex, ace_mon, this->lock_, -1)); + + // Not found!? + if (this->remove_i (name, &s) == -1) + return -1; + } + + if (ps != 0) + *ps = s; + else + delete s; + return 0; +} + /** * @brief Completely remove a entry from the Repository and * dynamically unlink it if it was originally dynamically linked. * + * Return a ptr to the entry in @code ps. There is no locking so make + * sure you hold the repo lock when calling. + * * Since the order of services in the Respository matters, we can't * simply overwrite the entry being deleted with the last and * decrement the by 1 - we must "pack" the array. A @@ -404,35 +475,31 @@ ACE_Service_Repository::suspend (const ACE_TCHAR name[], * _before_the dynamic service that owns them. Otherwice the TEXT * segment, containing the code for the static service's desructor may * be unloaded with the DLL. + * + * @note: (IJ) The above is not entirely true, since the introduction + * of the ACE_Service_Dynamic_Guard, which fixes-up any stray static + * services to hold a reference to the DLL. This allows out-of order + * removals and perhaps allows to skip packing the repo. I left it in + * because a packed repo is a lot easier to debug. */ - int -ACE_Service_Repository::remove (const ACE_TCHAR name[], ACE_Service_Type **ps) +ACE_Service_Repository::remove_i (const ACE_TCHAR name[], ACE_Service_Type **ps) { - ACE_TRACE ("ACE_Service_Repository::remove"); - ACE_Service_Type *s = 0; - { - ACE_MT (ACE_GUARD_RETURN (ACE_Recursive_Thread_Mutex, ace_mon, this->lock_, -1)); int i = this->find_i (name, 0, 0); // Not found if (i == -1) return -1; - // We need the old ptr to be delete outside the lock - s = const_cast (this->service_vector_[i]); + // We may need the old ptr - to be delete outside the lock! + *ps = const_cast (this->service_vector_[i]); // Pack the array --this->current_size_; for (size_t j = i; j < this->current_size_; j++) this->service_vector_[j] = this->service_vector_[j+1]; - } - if (ps != 0) - *ps = s; - else - delete s; - return 0; + return 0; } ACE_ALLOC_HOOK_DEFINE(ACE_Service_Repository_Iterator) @@ -445,6 +512,7 @@ ACE_Service_Repository_Iterator::dump (void) const #endif /* ACE_HAS_DUMP */ } + // Initializes the iterator and skips over any suspended entries at // the beginning of the table, if necessary. Note, you must not // perform destructive operations on elements during this iteration... diff --git a/ace/Service_Repository.h b/ace/Service_Repository.h index bbeafbe3602..8cd55344d29 100644 --- a/ace/Service_Repository.h +++ b/ace/Service_Repository.h @@ -27,6 +27,7 @@ ACE_BEGIN_VERSIONED_NAMESPACE_DECL class ACE_Service_Type; +class ACE_DLL; #define ACE_Component_Repository ACE_Service_Repository /** @@ -132,12 +133,40 @@ public: ACE_ALLOC_HOOK_DECLARE; private: + + friend class ACE_Service_Type_Dynamic_Guard; + + /// Remove an existing service record. It requires @a sr != 0, which + /// receives the service record pointer and the caller is + /// responsible for properly disposing of it. + int remove_i (const ACE_TCHAR[], ACE_Service_Type **sr); + /// Locates . Must be called without locks being /// held... + int find_i (const ACE_TCHAR service_name[], const ACE_Service_Type ** = 0, int ignore_suspended = 1) const; + /// @brief Relocate (static) services to another DLL. + /// + /// If any have been registered in the context of a "forward + /// declaration" guard, those really aren't static services. Their + /// code is in the DLL's code segment, or in one of the dependent + /// DLLs. Therefore, such services need to be associated with the + /// proper DLL in order to prevent failures upon finalization. The + /// method locks the repo. + /// + /// Works by having the service type keep a reference to a specific + /// DLL. No locking, caller makes sure calling it is safe. You can + /// forcefully relocate any DLLs in the given range, not only the + /// static ones - but that will cause Very Bad Things (tm) to happen. + + int relocate_i (size_t begin, + size_t end, + const ACE_DLL &adll, + bool static_only = true); + /// Contains all the configured services. const ACE_Service_Type **service_vector_; @@ -178,6 +207,8 @@ public: /// Destructor. ~ACE_Service_Repository_Iterator (void); + +public: // = Iteration methods. /// Pass back the that hasn't been seen in the repository. @@ -199,9 +230,10 @@ public: private: bool valid (void) const; - ACE_Service_Repository_Iterator (const ACE_Service_Repository_Iterator&); private: + ACE_Service_Repository_Iterator (const ACE_Service_Repository_Iterator&); + /// Reference to the Service Repository we are iterating over. ACE_Service_Repository &svc_rep_; -- cgit v1.2.1