From 57c06196d908503052c2c6f2e465a032a7e9dcdb Mon Sep 17 00:00:00 2001 From: vsr Date: Mon, 21 Sep 2009 14:19:29 +0000 Subject: [PATCH] Fix memory leaks (integrate patch from C.Caremoli) --- src/GEOMBase/GEOMBase_Helper.cxx | 26 ++++++++++----- src/GEOMClient/GEOM_Client.cxx | 3 +- src/GEOMGUI/GeometryGUI.cxx | 13 ++------ src/GEOMGUI/GeometryGUI.h | 2 -- src/GEOMToolsGUI/GEOMToolsGUI.cxx | 5 ++- src/GEOM_I/GEOM_Gen_i.cc | 54 +++++++++++++++++++++++-------- src/GEOM_I/GEOM_Object_i.cc | 5 +-- 7 files changed, 71 insertions(+), 37 deletions(-) diff --git a/src/GEOMBase/GEOMBase_Helper.cxx b/src/GEOMBase/GEOMBase_Helper.cxx index 328a5d398..1b8235f6e 100755 --- a/src/GEOMBase/GEOMBase_Helper.cxx +++ b/src/GEOMBase/GEOMBase_Helper.cxx @@ -106,6 +106,7 @@ GEOMBase_Helper::~GEOMBase_Helper() if (myDisplayer) delete myDisplayer; + myOperation->Destroy(); } //================================================================ @@ -264,9 +265,10 @@ void GEOMBase_Helper::displayPreview( const bool activate, else { for ( ObjectList::iterator it = objects.begin(); it != objects.end(); ++it ) { - displayPreview( *it, true, activate, false, lineWidth, displayMode, color ); + GEOM::GEOM_Object_var obj=*it; + displayPreview( obj, true, activate, false, lineWidth, displayMode, color ); if ( toRemoveFromEngine ) - getGeomEngine()->RemoveObject( *it ); + obj->Destroy(); } } } @@ -524,6 +526,7 @@ void GEOMBase_Helper::addInStudy( GEOM::GEOM_Object_ptr theObj, const char* theN // Each dialog is responsible for this method implementation, // default implementation does nothing restoreSubShapes(aStudyDS, aSO); + aSO->Destroy(); } //================================================================ @@ -699,6 +702,7 @@ bool GEOMBase_Helper::abortCommand() return false; myCommand->abort(); + delete myCommand; // I don't know where to delete this object here ? myCommand = 0; return true; @@ -714,6 +718,7 @@ bool GEOMBase_Helper::commitCommand( const char* ) return false; myCommand->commit(); + delete myCommand; // I don't know where to delete this object here ? myCommand = 0; return true; @@ -806,11 +811,12 @@ bool GEOMBase_Helper::onAccept( const bool publish, const bool useTransaction ) const int nbObjs = objects.size(); int aNumber = 1; for ( ObjectList::iterator it = objects.begin(); it != objects.end(); ++it ) { + GEOM::GEOM_Object_var obj=*it; if ( publish ) { QString aName = getNewObjectName(); if ( nbObjs > 1 ) { if (aName.isEmpty()) - aName = getPrefix(*it); + aName = getPrefix(obj); if (nbObjs <= 30) { // Try to find a unique name aName = GEOMBase::GetDefaultName(aName); @@ -821,18 +827,22 @@ bool GEOMBase_Helper::onAccept( const bool publish, const bool useTransaction ) } else { // PAL6521: use a prefix, if some dialog box doesn't reimplement getNewObjectName() if ( aName.isEmpty() ) - aName = GEOMBase::GetDefaultName( getPrefix( *it ) ); + aName = GEOMBase::GetDefaultName( getPrefix( obj ) ); } - addInStudy( *it, aName.toLatin1().constData() ); + addInStudy( obj, aName.toLatin1().constData() ); // updateView=false - display( *it, false ); + display( obj, false ); + // obj has been published in study. Its refcount has been incremented. + // It is safe to decrement its refcount + // so that it will be destroyed when the entry in study will be removed + obj->Destroy(); } else { // asv : fix of PAL6454. If publish==false, then the original shape // was modified, and need to be re-cached in GEOM_Client before redisplay - clearShapeBuffer( *it ); + clearShapeBuffer( obj ); // withChildren=true, updateView=false - redisplay( *it, true, false ); + redisplay( obj, true, false ); } } diff --git a/src/GEOMClient/GEOM_Client.cxx b/src/GEOMClient/GEOM_Client.cxx index f892de7f0..470df2704 100644 --- a/src/GEOMClient/GEOM_Client.cxx +++ b/src/GEOMClient/GEOM_Client.cxx @@ -69,7 +69,8 @@ TopoDS_Shape GEOM_Client::Load( GEOM::GEOM_Gen_ptr geom, GEOM::GEOM_Object_ptr a Engines::Container_var ctn_server = geom->GetContainerRef(); long pid_server = ctn_server->getPID(); - if ( (pid_client==pid_server) && (strcmp(hst_client.c_str(), ctn_server->getHostName())==0) ) { + CORBA::String_var hostname = ctn_server->getHostName(); + if ( pid_client == pid_server && !strcmp(hst_client.c_str(), hostname.in()) ) { TopoDS_Shape* S = (TopoDS_Shape*)(aShape->getShape()); return(*S); } else { diff --git a/src/GEOMGUI/GeometryGUI.cxx b/src/GEOMGUI/GeometryGUI.cxx index e33a927cc..81efb22a1 100644 --- a/src/GEOMGUI/GeometryGUI.cxx +++ b/src/GEOMGUI/GeometryGUI.cxx @@ -149,14 +149,6 @@ SALOMEDS::Study_var GeometryGUI::ClientStudyToStudy (_PTR(Study) theStudy) return aDSStudy._retn(); } -//======================================================================= -// function : JoinObjectParameters -// purpose : -//======================================================================= -char* GeometryGUI::JoinObjectParameters(const QStringList& theParametersList) -{ - return theParametersList.join(":").toLatin1().data(); -} //======================================================================= // function : GeometryGUI::GeometryGUI() // purpose : Constructor @@ -167,8 +159,7 @@ GeometryGUI::GeometryGUI() : { if ( CORBA::is_nil( myComponentGeom ) ) { - SALOME_LifeCycleCORBA* ls = new SALOME_LifeCycleCORBA( getApp()->namingService() ); - Engines::Component_var comp = ls->FindOrLoad_Component( "FactoryServer", "GEOM" ); + Engines::Component_var comp = SalomeApp_Application::lcc()->FindOrLoad_Component( "FactoryServer", "GEOM" ); myComponentGeom = GEOM::GEOM_Gen::_narrow( comp ); } @@ -195,6 +186,8 @@ GeometryGUI::~GeometryGUI() while (!myVTKSelectors.isEmpty()) delete myVTKSelectors.takeFirst(); + + qDeleteAll(myGUIMap); } //======================================================================= diff --git a/src/GEOMGUI/GeometryGUI.h b/src/GEOMGUI/GeometryGUI.h index 8282dccc3..a82efcb25 100644 --- a/src/GEOMGUI/GeometryGUI.h +++ b/src/GEOMGUI/GeometryGUI.h @@ -81,8 +81,6 @@ public: static CORBA::Object_var ClientSObjectToObject (_PTR(SObject) theSObject); static SALOMEDS::Study_var ClientStudyToStudy (_PTR(Study) theStudy); - static char* JoinObjectParameters(const QStringList& theParametersList); - GEOM_Client& GetShapeReader() { return myShapeReader; } Standard_CString& GetFatherior() { return myFatherior; } //void SetState( const int state ) { myState = state; } diff --git a/src/GEOMToolsGUI/GEOMToolsGUI.cxx b/src/GEOMToolsGUI/GEOMToolsGUI.cxx index 2099f79ca..fceff718f 100644 --- a/src/GEOMToolsGUI/GEOMToolsGUI.cxx +++ b/src/GEOMToolsGUI/GEOMToolsGUI.cxx @@ -950,7 +950,10 @@ void GEOMToolsGUI::removeObjectWithChildren(_PTR(SObject) obj, disp->Erase(geomObj, true, view); // Remove object from Engine - GeometryGUI::GetGeomGen()->RemoveObject( geomObj ); + // We can't directly remove object from engine. All we can do is to unpublish the object + // from the study. Another client could be using the object. + // Unpublishing is done just after in aStudyBuilder->RemoveObjectWithChildren( child ); + //GeometryGUI::GetGeomGen()->RemoveObject( geomObj ); } } } diff --git a/src/GEOM_I/GEOM_Gen_i.cc b/src/GEOM_I/GEOM_Gen_i.cc index 738fb402d..3bc08c7c8 100644 --- a/src/GEOM_I/GEOM_Gen_i.cc +++ b/src/GEOM_I/GEOM_Gen_i.cc @@ -182,9 +182,12 @@ SALOMEDS::SObject_ptr GEOM_Gen_i::PublishInStudy(SALOMEDS::Study_ptr theStudy, anAttr = aStudyBuilder->FindOrCreateAttribute(aFather, "AttributeName"); SALOMEDS::AttributeName_var aName = SALOMEDS::AttributeName::_narrow(anAttr); aName->SetValue("Geometry"); + aName->Destroy(); anAttr = aStudyBuilder->FindOrCreateAttribute(aFather, "AttributePixMap"); - SALOMEDS::AttributePixMap::_narrow(anAttr)->SetPixMap("ICON_OBJBROWSER_Geometry"); - aStudyBuilder->DefineComponentInstance(aFather, GEOM_Gen::_this()); + SALOMEDS::AttributePixMap_var aPixMap=SALOMEDS::AttributePixMap::_narrow(anAttr); + aPixMap->SetPixMap("ICON_OBJBROWSER_Geometry"); + aPixMap->Destroy(); + aStudyBuilder->DefineComponentInstance(aFather, (GEOM::GEOM_Gen_var)GEOM_Gen::_this()); } if (aFather->_is_nil()) return aResultSO; @@ -197,10 +200,9 @@ SALOMEDS::SObject_ptr GEOM_Gen_i::PublishInStudy(SALOMEDS::Study_ptr theStudy, } anAttr = aStudyBuilder->FindOrCreateAttribute(aResultSO, "AttributeIOR"); SALOMEDS::AttributeIOR_var anIOR = SALOMEDS::AttributeIOR::_narrow(anAttr); - //char *aGeomObjIOR = _orb->object_to_string(theObject); - CORBA::String_var aGeomObjIOR = _orb->object_to_string(theObject); - //anIOR->SetValue(CORBA::string_dup(aGeomObjIOR)); + CORBA::String_var aGeomObjIOR = _orb->object_to_string(theObject); anIOR->SetValue(aGeomObjIOR); + anIOR->Destroy(); anAttr = aStudyBuilder->FindOrCreateAttribute(aResultSO, "AttributePixMap"); SALOMEDS::AttributePixMap_var aPixmap = SALOMEDS::AttributePixMap::_narrow(anAttr); @@ -254,6 +256,7 @@ SALOMEDS::SObject_ptr GEOM_Gen_i::PublishInStudy(SALOMEDS::Study_ptr theStudy, aPixmap->SetPixMap( "ICON_OBJBROWSER_VERTEX" ); aShapeName = "Vertex_"; } + aPixmap->Destroy(); //if (strlen(theName) == 0) aShapeName += TCollection_AsciiString(aResultSO->Tag()); //else aShapeName = TCollection_AsciiString(CORBA::string_dup(theName)); @@ -264,8 +267,7 @@ SALOMEDS::SObject_ptr GEOM_Gen_i::PublishInStudy(SALOMEDS::Study_ptr theStudy, Handle(GEOM_Object) aGShape = _impl->GetObject(aShape->GetStudyID(), entry); TopoDS_Shape TopoSh = aGShape->GetValue(); // find label of main shape - GEOM::GEOM_Object_var aMainShVar = aShape; - GEOM::GEOM_Object_ptr aMainSh = aMainShVar._retn(); + GEOM::GEOM_Object_var aMainSh = aShape; while( !aMainSh->IsMainShape() ) { aMainSh = aMainSh->GetMainShape(); } @@ -309,16 +311,19 @@ SALOMEDS::SObject_ptr GEOM_Gen_i::PublishInStudy(SALOMEDS::Study_ptr theStudy, } //Set the study entry as a name of the published GEOM_Object - aShape->SetStudyEntry(aResultSO->GetID()); + CORBA::String_var anID =aResultSO->GetID(); + aShape->SetStudyEntry(anID.in()); //Set a name of the added shape anAttr = aStudyBuilder->FindOrCreateAttribute(aResultSO, "AttributeName"); SALOMEDS::AttributeName_var aNameAttrib = SALOMEDS::AttributeName::_narrow(anAttr); aNameAttrib->SetValue(aShapeName.ToCString()); + aNameAttrib->Destroy(); //Set NoteBook variables used in the object creation TCollection_AsciiString aVars; - SALOMEDS::ListOfListOfStrings_var aSections = theStudy->ParseVariables(aShape->GetParameters()); + CORBA::String_var aString=aShape->GetParameters(); + SALOMEDS::ListOfListOfStrings_var aSections = theStudy->ParseVariables(aString); for(int i = 0, n = aSections->length(); i < n; i++) { SALOMEDS::ListOfStrings aListOfVars = aSections[i]; for(int j = 0, m = aListOfVars.length(); j < m; j++) { @@ -333,6 +338,9 @@ SALOMEDS::SObject_ptr GEOM_Gen_i::PublishInStudy(SALOMEDS::Study_ptr theStudy, anAttr = aStudyBuilder->FindOrCreateAttribute(aResultSO, "AttributeString"); SALOMEDS::AttributeString_var aStringAttrib = SALOMEDS::AttributeString::_narrow(anAttr); aStringAttrib->SetValue(aVars.ToCString()); + aStringAttrib->Destroy(); + + aFather->Destroy(); //Set a name of the GEOM object aShape->SetName(theName); @@ -591,7 +599,8 @@ CORBA::Boolean GEOM_Gen_i::LoadASCII(SALOMEDS::SComponent_ptr theComponent, //============================================================================ void GEOM_Gen_i::Close(SALOMEDS::SComponent_ptr theComponent) { - _impl->Close(theComponent->GetStudy()->StudyId()); + SALOMEDS::Study_var aStudy= theComponent->GetStudy(); + _impl->Close(aStudy->StudyId()); } //============================================================================ @@ -605,7 +614,10 @@ CORBA::Boolean GEOM_Gen_i::CanCopy(SALOMEDS::SObject_ptr theObject) { SALOMEDS::AttributeIOR_var anIOR = SALOMEDS::AttributeIOR::_narrow(anAttr); - GEOM::GEOM_Object_var anObject = GEOM::GEOM_Object::_narrow(_orb->string_to_object(anIOR->Value())); + CORBA::String_var aString=anIOR->Value(); + anIOR->Destroy(); + CORBA::Object_var anObj = _orb->string_to_object(aString); + GEOM::GEOM_Object_var anObject = GEOM::GEOM_Object::_narrow(anObj); // If the object is null one it can't be copied: return false if (anObject->_is_nil()) return false; return true; @@ -692,6 +704,7 @@ SALOMEDS::SObject_ptr GEOM_Gen_i::PasteInto(const SALOMEDS::TMPFile& theStream, SALOMEDS::AttributeIOR_var anIOR = SALOMEDS::AttributeIOR::_narrow(anAttr); CORBA::String_var objStr = _orb->object_to_string(obj); anIOR->SetValue(objStr.in()); + anIOR->Destroy(); // Return the created in the Study SObject return aNewSO._retn(); @@ -726,6 +739,7 @@ SALOMEDS::SObject_ptr GEOM_Gen_i::AddInStudy (SALOMEDS::Study_ptr theStudy, SALOMEDS::SObject_var aFatherSO = theStudy->FindObjectIOR(IOR.in()); if(aFatherSO->_is_nil()) return aResultSO._retn(); aResultSO = aStudyBuilder->NewObject(aFatherSO); + aFatherSO->Destroy(); //aStudyBuilder->Addreference(aResultSO, aResultSO); } @@ -745,6 +759,8 @@ SALOMEDS::SObject_ptr GEOM_Gen_i::AddInStudy (SALOMEDS::Study_ptr theStudy, if(aSO->_is_nil()) continue; SALOMEDS::SObject_var aSubSO = aStudyBuilder->NewObject(aResultSO); aStudyBuilder->Addreference(aSubSO, aSO); + aSO->Destroy(); + aSubSO->Destroy(); } return aResultSO._retn(); @@ -774,6 +790,7 @@ GEOM::ListOfGO* GEOM_Gen_i::RestoreSubShapesO (SALOMEDS::Study_ptr theStudy, // return aParts._retn(); aParts = RestoreSubShapes(theStudy, theObject, aSO, theArgs, theFindMethod, theInheritFirstArg); + aSO->Destroy(); return aParts._retn(); } @@ -887,6 +904,7 @@ GEOM::ListOfGO* GEOM_Gen_i::RestoreSubShapes(SALOMEDS::Study_ptr theStudy, // set the color of the transformed shape to the color of initial shape theObject->SetColor(aList[0]->GetColor()); + anArgSO->Destroy(); } else { // Get interface, containing method, which we will use to reconstruct sub-shapes @@ -965,7 +983,7 @@ GEOM::ListOfGO* GEOM_Gen_i::RestoreSubShapes(SALOMEDS::Study_ptr theStudy, // Publish the sub-shape SALOMEDS::SObject_var aSubSO; - if (!CORBA::is_nil(theSObject)) { + if (!CORBA::is_nil(theSObject)) { TCollection_AsciiString aSubName ("from_"); aSubName += anArgName; aSubSO = aStudyBuilder->NewObject(theSObject); @@ -989,7 +1007,7 @@ GEOM::ListOfGO* GEOM_Gen_i::RestoreSubShapes(SALOMEDS::Study_ptr theStudy, else { // GetInPlace failed, try to build from published parts if (!CORBA::is_nil(anArgSO)) { SALOMEDS::SObject_var aSubSO; - if (!CORBA::is_nil(theSObject)) + if (!CORBA::is_nil(theSObject)) aSubSO = aStudyBuilder->NewObject(theSObject); // Restore published sub-shapes of the argument @@ -1025,6 +1043,7 @@ GEOM::ListOfGO* GEOM_Gen_i::RestoreSubShapes(SALOMEDS::Study_ptr theStudy, } } } // try to build from published parts + anArgSO->Destroy(); } } // process arguments } @@ -1308,6 +1327,7 @@ GEOM::GEOM_IBasicOperations_ptr GEOM_Gen_i::GetIBasicOperations(CORBA::Long theS GEOM_IBasicOperations_i* aServant = new GEOM_IBasicOperations_i(_poa, engine, _impl->GetIBasicOperations(theStudyID)); + PortableServer::ObjectId_var id = _poa->activate_object(aServant); // activate the CORBA servant GEOM::GEOM_IBasicOperations_var operations = aServant->_this(); return operations._retn(); @@ -1347,6 +1367,7 @@ GEOM::GEOM_I3DPrimOperations_ptr GEOM_Gen_i::GetI3DPrimOperations(CORBA::Long th GEOM_I3DPrimOperations_i* aServant = new GEOM_I3DPrimOperations_i(_poa, engine, _impl->GetI3DPrimOperations(theStudyID)); + PortableServer::ObjectId_var id = _poa->activate_object(aServant); // activate the CORBA servant GEOM::GEOM_I3DPrimOperations_var operations = aServant->_this(); @@ -1614,7 +1635,9 @@ GEOM::GEOM_Object_ptr GEOM_Gen_i::GetObject (CORBA::Long theStudyID, const char* } GEOM::GEOM_Gen_ptr engine = _this(); + //transfer the reference to GEOM_Object_i GEOM_Object_i* servant = new GEOM_Object_i (_poa, engine, handle_object); + PortableServer::ObjectId_var id = _poa->activate_object(servant); obj = servant->_this(); CORBA::String_var objStr = _orb->object_to_string(obj); @@ -1652,9 +1675,12 @@ char* GEOM_Gen_i::getObjectInfo(CORBA::Long studyId, const char* entry) if (!aSObj->_is_nil() && aSObj->FindAttribute(anAttr, "AttributeIOR")) { SALOMEDS::AttributeIOR_var anIOR = SALOMEDS::AttributeIOR::_narrow(anAttr); CORBA::String_var aVal = anIOR->Value(); + anIOR->Destroy(); CORBA::Object_var anObject = aStudy->ConvertIORToObject(aVal); aGeomObject = GEOM::GEOM_Object::_narrow(anObject); } + if (!aSObj->_is_nil() ) + aSObj->Destroy(); const char* aTypeInfo = "Object"; if ( !aGeomObject->_is_nil() ) { @@ -1782,6 +1808,8 @@ char* GEOM_Gen_i::getObjectInfo(CORBA::Long studyId, const char* entry) //===================================================================================== extern "C" { +PortableServer::ObjectId* GEOMEngine_factory(CORBA::ORB*, PortableServer::POA*, PortableServer::ObjectId*, const char*, const char*); + GEOM_I_EXPORT PortableServer::ObjectId * GEOMEngine_factory(CORBA::ORB_ptr orb, PortableServer::POA_ptr poa, diff --git a/src/GEOM_I/GEOM_Object_i.cc b/src/GEOM_I/GEOM_Object_i.cc index 6aee4454c..b99e24c22 100644 --- a/src/GEOM_I/GEOM_Object_i.cc +++ b/src/GEOM_I/GEOM_Object_i.cc @@ -64,6 +64,7 @@ GEOM_Object_i::GEOM_Object_i (PortableServer::POA_ptr thePOA, GEOM::GEOM_Gen_ptr GEOM_Object_i::~GEOM_Object_i() { + MESSAGE("GEOM_Object_i::~GEOM_Object_i"); GEOM_Engine::GetEngine()->RemoveObject(_impl); } @@ -230,7 +231,7 @@ GEOM::ListOfGO* GEOM_Object_i::GetDependency() Handle(GEOM_Object) anObj = Handle(GEOM_Object)::DownCast(aSeq->Value(i)); if (anObj.IsNull()) continue; TDF_Tool::Entry(anObj->GetEntry(), anEntry); - GEOM::GEOM_Object_var obj = GEOM::GEOM_Object::_duplicate(_engine->GetObject(anObj->GetDocID(), anEntry.ToCString())); + GEOM::GEOM_Object_var obj = _engine->GetObject(anObj->GetDocID(), anEntry.ToCString()); aList[i-1] = obj; } @@ -352,7 +353,7 @@ GEOM::GEOM_Object_ptr GEOM_Object_i::GetMainShape() if(aLabel.IsNull()) return obj._retn(); TCollection_AsciiString anEntry; TDF_Tool::Entry(aLabel, anEntry); - return GEOM::GEOM_Object::_duplicate(_engine->GetObject(_impl->GetDocID(), anEntry.ToCString())); + return _engine->GetObject(_impl->GetDocID(), anEntry.ToCString()); } return obj._retn();