From 90ae4e07bb33f665ad05648955a24788db40c411 Mon Sep 17 00:00:00 2001 From: Anthony Geay Date: Wed, 21 Feb 2024 07:59:23 +0100 Subject: [PATCH] [EDF29150] : prevent deletion of datastructure of perf log tree on orb_destroy called by RuntimeSalome::fini from YACS Clean management of instances and POA desactivation even on orb_destroy. To avoid unexpected destroy of perf log instance on container shutdown after successful YACS run the log instance is no more cleaned at container Shutdown. --- src/Container/SALOME_Container.py | 5 ++- src/Launcher/SALOME_LogManager.cxx | 63 +++++++++--------------------- src/Launcher/SALOME_LogManager.hxx | 59 ++++++++++++++++++++++++++-- 3 files changed, 78 insertions(+), 49 deletions(-) diff --git a/src/Container/SALOME_Container.py b/src/Container/SALOME_Container.py index b4639588c..1fa1d41d0 100644 --- a/src/Container/SALOME_Container.py +++ b/src/Container/SALOME_Container.py @@ -183,8 +183,9 @@ class SALOME_Container_i: def shutdownPy(self): if getSSLMode(): if self._log: - self._log.destroy() - + #self._log.destroy()# TODO : choose to destroy perf report or not. For the moment we keep the report + pass + def setLogFileName(self, logFileName): logging.debug("setLogFileName {} PID = {}".format(logFileName,os.getpid())) if getSSLMode(): diff --git a/src/Launcher/SALOME_LogManager.cxx b/src/Launcher/SALOME_LogManager.cxx index f336d56a2..f006eb245 100644 --- a/src/Launcher/SALOME_LogManager.cxx +++ b/src/Launcher/SALOME_LogManager.cxx @@ -62,13 +62,7 @@ static SALOME::vectorOfByte *FromVectCharToCorba(const std::vector& data) SALOME_ContainerScriptPerfLog::~SALOME_ContainerScriptPerfLog() { - for(auto execSession : _sessions) - { - PortableServer::ServantBase *serv = getPOA()->reference_to_servant(execSession); - PortableServer::ObjectId_var oid = getPOA()->reference_to_id(execSession); - getPOA()->deactivate_object(oid); - } - _sessions.clear(); + DEBUG_MESSAGE("Destruction of SALOME_ContainerScriptPerfLog instance"); } PortableServer::POA_var SALOME_ContainerScriptExecPerfLog::getPOA() @@ -145,12 +139,9 @@ char *SALOME_ContainerScriptPerfLog::getName() void SALOME_ContainerScriptPerfLog::accept(SALOME_VisitorContainerLog &visitor) { visitor.enterContainerScriptPerfLog( *this ); - for(auto session : _sessions) + for(auto& session : _sessions) { - PortableServer::ServantBase *serv = getPOA()->reference_to_servant( session ); - serv->_remove_ref(); - SALOME_ContainerScriptExecPerfLog *servC = dynamic_cast(serv); - visitor.visitContainerScriptExecPerfLog( *servC ); + visitor.visitContainerScriptExecPerfLog( *session.getServ() ); } visitor.leaveContainerScriptPerfLog( *this ); } @@ -160,10 +151,9 @@ Engines::ContainerScriptExecPerfLog_ptr SALOME_ContainerScriptPerfLog::addExecut { SALOME_ContainerScriptExecPerfLog *execution = new SALOME_ContainerScriptExecPerfLog(this); PortableServer::ObjectId_var id(getPOA()->activate_object(execution)); - execution->_remove_ref(); CORBA::Object_var executionPtr(getPOA()->id_to_reference(id)); Engines::ContainerScriptExecPerfLog_var executionPtr2 = Engines::ContainerScriptExecPerfLog::_narrow(executionPtr); - _sessions.push_back( executionPtr2 ); + _sessions.push_back( { executionPtr2,execution } ); { AutoGIL gstate; AutoPyRef result = PyObject_CallMethod(pyObj(),(char*)"addExecution","",nullptr); @@ -185,7 +175,7 @@ Engines::ListOfContainerScriptExecPerfLog *SALOME_ContainerScriptPerfLog::listOf auto sz = this->_sessions.size(); ret->length(sz); for(auto i = 0 ; i < sz ; ++i) - ret[i] = this->_sessions[i]; + ret[i] = this->_sessions[i].getVar(); return ret._retn(); } @@ -193,7 +183,6 @@ Engines::ListOfContainerScriptExecPerfLog *SALOME_ContainerScriptPerfLog::listOf SALOME_ContainerPerfLog::~SALOME_ContainerPerfLog() { - this->destroyInternal(); } PortableServer::POA_var SALOME_ContainerPerfLog::getPOA() @@ -205,10 +194,9 @@ Engines::ContainerScriptPerfLog_ptr SALOME_ContainerPerfLog::addScript(const cha { SALOME_ContainerScriptPerfLog *script = new SALOME_ContainerScriptPerfLog(this,name,code); PortableServer::ObjectId_var id(getPOA()->activate_object(script)); - script->_remove_ref(); CORBA::Object_var scriptPtr(getPOA()->id_to_reference(id)); Engines::ContainerScriptPerfLog_var scriptPtr2 = Engines::ContainerScriptPerfLog::_narrow(scriptPtr); - _scripts.push_back( scriptPtr2 ); + _scripts.push_back( { scriptPtr2,script } ); { AutoGIL gstate; PyObject *result = PyObject_CallMethod(pyObj(),(char*)"addScript","",nullptr); @@ -229,7 +217,7 @@ Engines::ListOfContainerScriptPerfLog *SALOME_ContainerPerfLog::listOfScripts() std::size_t len( this->_scripts.size() ); ret->length( len ); for(std::size_t i = 0 ; i < len ; ++i) - ret[i] = this->_scripts[i]; + ret[i] = this->_scripts[i].getVar(); return ret._retn(); } @@ -241,11 +229,9 @@ void SALOME_ContainerPerfLog::destroy() void SALOME_ContainerPerfLog::destroyInternal() { _father->removeEntryBeforeDying( this ); - for(auto script : _scripts) + for(auto& script : _scripts) { - PortableServer::ServantBase *serv = getPOA()->reference_to_servant(script); - PortableServer::ObjectId_var oid = getPOA()->reference_to_id(script); - getPOA()->deactivate_object(oid); + script.desactivateObjectFromPOA( ); } _scripts.clear(); } @@ -253,12 +239,9 @@ void SALOME_ContainerPerfLog::destroyInternal() void SALOME_ContainerPerfLog::accept(SALOME_VisitorContainerLog &visitor) { visitor.enterContainerPerfLog( *this ); - for(auto script : _scripts) + for(auto& script : _scripts) { - PortableServer::ServantBase *serv = getPOA()->reference_to_servant( script ); - serv->_remove_ref(); - SALOME_ContainerScriptPerfLog *servC = dynamic_cast(serv); - servC->accept(visitor); + script.getServ()->accept(visitor); } visitor.leaveContainerPerfLog( *this ); } @@ -313,16 +296,13 @@ SALOME_LogManager::SALOME_LogManager(CORBA::ORB_ptr orb, PortableServer::POA_var SALOME_LogManager::~SALOME_LogManager() { - this->clear(); } void SALOME_LogManager::clear() { - for(auto cont : _containers) + for(auto& cont : _containers) { - PortableServer::ServantBase *serv = getPOA()->reference_to_servant(cont); - PortableServer::ObjectId_var oid = getPOA()->reference_to_id(cont); - getPOA()->deactivate_object(oid); + cont.desactivateObjectFromPOA( ); } _containers.clear(); } @@ -332,9 +312,8 @@ Engines::ContainerPerfLog_ptr SALOME_LogManager::declareContainer(const char *co SALOME_ContainerPerfLog *cont = new SALOME_ContainerPerfLog(this,contInNS,logfile); PortableServer::ObjectId_var id(_poa->activate_object(cont)); CORBA::Object_var contPtr(_poa->id_to_reference(id)); - cont->_remove_ref(); Engines::ContainerPerfLog_var contPtr2 = Engines::ContainerPerfLog::_narrow(contPtr); - _containers.push_back( contPtr2 ); + _containers.push_back( { contPtr2, cont } ); { AutoGIL gstate; PyObject *result = PyObject_CallMethod(_pyLogManager,(char*)"declareContainer","ss",contInNS,logfile,nullptr); @@ -356,7 +335,7 @@ Engines::ListOfContainerPerfLog *SALOME_LogManager::listOfContainerLogs() ret->length( len ); for(std::size_t i = 0 ; i < len ; ++i) { - ret[i] = this->_containers[i]; + ret[i] = this->_containers[i].getVar(); } return ret._retn(); } @@ -364,12 +343,9 @@ Engines::ListOfContainerPerfLog *SALOME_LogManager::listOfContainerLogs() void SALOME_LogManager::accept(SALOME_VisitorContainerLog &visitor) { visitor.enterLogManager( *this ); - for(auto container : _containers) + for(auto& container : _containers) { - PortableServer::ServantBase *serv = getPOA()->reference_to_servant( container ); - serv->_remove_ref(); - SALOME_ContainerPerfLog *servC = dynamic_cast(serv); - servC->accept(visitor); + container.getServ()->accept(visitor); } visitor.leaveLogManager( *this ); } @@ -415,10 +391,9 @@ void SALOME_LogManager::removeEntryBeforeDying(SALOME_ContainerPerfLog *child) { for(auto cont = _containers.begin() ; cont != _containers.end() ; ++cont ) { - PortableServer::ServantBase *serv = getPOA()->reference_to_servant(*cont); - SALOME_ContainerPerfLog *servCand = dynamic_cast(serv); - if( servCand==child ) + if( (*cont).getServ() == child ) { + (*cont).desactivateObjectFromPOA(); _containers.erase( cont ); break; } diff --git a/src/Launcher/SALOME_LogManager.hxx b/src/Launcher/SALOME_LogManager.hxx index 44948e4aa..33d9f2cf8 100644 --- a/src/Launcher/SALOME_LogManager.hxx +++ b/src/Launcher/SALOME_LogManager.hxx @@ -39,6 +39,51 @@ class SALOME_ContainerPerfLog; class SALOME_ContainerScriptPerfLog; class SALOME_ContainerScriptExecPerfLog; +namespace SALOME +{ + auto ServantDeleter = [](PortableServer::ServantBase *serv) { if(serv) serv->_remove_ref(); }; + using AutoServantDeleter = std::unique_ptr; + + /** + * Class in charge to manage life cycle of servant instance and its associated reference. + * It allows to deal cleanly with management of servant life even in case of ORB_destoy call + * at exit. + * + * As orb_destroy automaticaly desactivates all objects activated by all POAs in this + * it's impossible to deal cleanly maangement of servant lifecyle using only _ptr or _var. + * + */ + template + class RefAndServant + { + public: + RefAndServant(TVar var, TServ *serv):_var(var),_serv(serv,ServantDeleter) { } + RefAndServant(const RefAndServant& other):_var( other._var ),_serv(nullptr,ServantDeleter) + { + other.getServ()->_add_ref(); + _serv.reset( other.getServ() ); + } + ~RefAndServant() = default; + RefAndServant& operator=(const RefAndServant& other) + { + _var = other._var; + other.getServ()->_add_ref(); + _serv.reset( other.getServ() ); + return *this; + } + TVar getVar() const { return _var; } + TServ *getServ() const { return dynamic_cast( _serv.get() ); } + void desactivateObjectFromPOA( ) + { + PortableServer::ObjectId_var oid = getServ()->getPOA()->reference_to_id(_var); + getServ()->getPOA()->deactivate_object(oid); + } + private: + TVar _var; + SALOME::AutoServantDeleter _serv; + }; +} + class SALOMELAUNCHER_EXPORT SALOME_VisitorContainerLog { public: @@ -51,6 +96,7 @@ public: virtual void visitContainerScriptExecPerfLog(SALOME_ContainerScriptExecPerfLog& inst) = 0; }; + class SALOMELAUNCHER_EXPORT SALOME_ContainerScriptExecPerfLog : public POA_Engines::ContainerScriptExecPerfLog { public: @@ -75,6 +121,8 @@ private: std::vector _data; }; +using ContainerScriptExecPerfLogPair = SALOME::RefAndServant< Engines::ContainerScriptExecPerfLog_var, SALOME_ContainerScriptExecPerfLog >; + class SALOMELAUNCHER_EXPORT SALOME_ContainerScriptPerfLog : public POA_Engines::ContainerScriptPerfLog { public: @@ -100,9 +148,11 @@ private: SALOME_ContainerPerfLog *_father = nullptr; std::string _name; std::string _code; - std::vector< Engines::ContainerScriptExecPerfLog_var > _sessions; + std::vector< ContainerScriptExecPerfLogPair > _sessions; }; +using ContainerScriptPerfLogPair = SALOME::RefAndServant< Engines::ContainerScriptPerfLog_var, SALOME_ContainerScriptPerfLog >; + class SALOMELAUNCHER_EXPORT SALOME_ContainerPerfLog : public POA_Engines::ContainerPerfLog { public: @@ -116,6 +166,7 @@ public: char *getContainerEntryInNS() override; Engines::ContainerScriptPerfLog_ptr addScript(const char *name, const char *code) override; Engines::ListOfContainerScriptPerfLog *listOfScripts() override; + /* Call that destroy this object AND unregister from POA all referecences of this*/ void destroy() override; const std::string& nameInNS() const { return _name_in_ns; } const std::string& logFile() const { return _log_file; } @@ -131,7 +182,7 @@ private: SALOME_LogManager *_father = nullptr; std::string _name_in_ns; std::string _log_file; - std::vector< Engines::ContainerScriptPerfLog_var > _scripts; + std::vector< ContainerScriptPerfLogPair > _scripts; }; enum class SafeLoggerActiveVersionType @@ -151,6 +202,8 @@ private: SafeLoggerActiveVersionType _version_activated = SafeLoggerActiveVersionType::NoVersion_Activated; }; +using ContainerPerfLogPair = SALOME::RefAndServant< Engines::ContainerPerfLog_var, SALOME_ContainerPerfLog >; + class SALOMELAUNCHER_EXPORT SALOME_LogManager : public POA_Engines::LogManager { public: @@ -181,7 +234,7 @@ class SALOMELAUNCHER_EXPORT SALOME_LogManager : public POA_Engines::LogManager PyObject *_pyLogManager = nullptr; std::unique_ptr _NS; PortableServer::POA_var _poa; - std::vector _containers; + std::vector< ContainerPerfLogPair > _containers; SALOME_SafeLoggerFileHolder _safe_logger_file_holder; public: static const char NAME_IN_NS[]; -- 2.39.2