0031075: Application Framework - reading STEP file into TDocStd_Document leads to...

In the TDocStd_Owner keep simple pointer to TDocStd_Document instead of Handle. This causes automatic destruction of the document without explicit call of Close.
In Standard_Type added a static variable theType that initializes theRegistry map earlier. Otherwise exit from Draw interpreter crashes in many test-cases because not-closed transactions are aborted on document handle release from Draw theVariables map.

Corrected method for test OCC159bug due to the fact that Owner does not add a ref count now
Close the document in the end of bugs xde bug22776 otherwise double remove of visualization objects (on library exit and on visualization attributes remove from the document) causes crash on exit from draw
Added a new test bugs caf bug31075

diff --git a/src/QABugs/QABugs_1.cxx b/src/QABugs/QABugs_1.cxx
index bce0b3b3f97bda3a7e031bd3dd7b12ad90800e65..244aeb440bd534c990927d40858b96d770e25ff4 100644 (file)
--- a/src/QABugs/QABugs_1.cxx
+++ b/src/QABugs/QABugs_1.cxx
@@ -86,6 +86,7 @@ static Standard_Integer OCC159bug (Draw_Interpretor& di, Standard_Integer argc,
   } else {
     di << "DocOwner1 = NOTNULL\n";
   }
+  OwnerD1.Nullify();
  
   Handle(TDocStd_Application) A = DDocStd::GetApplication();
   A->Close(D); 

diff --git a/src/Standard/Standard_Type.cxx b/src/Standard/Standard_Type.cxx
index 95169151101ef401c539ccde2027ba6d03539d9c..b18c8b049a0749add3cfd91956449e821765d4b1 100644 (file)
--- a/src/Standard/Standard_Type.cxx
+++ b/src/Standard/Standard_Type.cxx
@@ -98,6 +98,9 @@ namespace {
     static registry_type theRegistry;
     return theRegistry;
   }
+
+  // To initialize theRegistry map as soon as possible to be destoryed the latest
+  Handle(Standard_Type) theType = STANDARD_TYPE(Standard_Transient);
 }
 
 Standard_Type* Standard_Type::Register (const char* theSystemName, const char* theName,

diff --git a/src/TDF/TDF_Data.cxx b/src/TDF/TDF_Data.cxx
index 44ba6da880b034097fcf414aebafbde4c6e99854..44e103c456f74166501edc3abd27c7c79cf1665b 100644 (file)
--- a/src/TDF/TDF_Data.cxx
+++ b/src/TDF/TDF_Data.cxx
@@ -110,6 +110,14 @@ myAllowModification     (Standard_True)
 void TDF_Data::Destroy()
 {
   AbortUntilTransaction(1);
+  // Forget the Owner attribute from the root label to avoid referencing document before
+  // desctuction of the framework (on custom attributes forget). Don't call ForgetAll because
+  // it may call backup.
+  while(!myRoot->FirstAttribute().IsNull()) {
+    static Handle(TDF_Attribute) anEmpty;
+    Handle(TDF_Attribute) aFirst = myRoot->FirstAttribute();
+    myRoot->RemoveAttribute(anEmpty, aFirst);
+  }
   myRoot->Destroy (myLabelNodeAllocator);
   myRoot = NULL;
 }

diff --git a/src/TDocStd/TDocStd_Document.cxx b/src/TDocStd/TDocStd_Document.cxx
index f79a5e45e1731dc0a4344101201dc6ca0d96e845..e72d5cacb0638a0440209b8c37fcddd7093f2a2f 100644 (file)
--- a/src/TDocStd/TDocStd_Document.cxx
+++ b/src/TDocStd/TDocStd_Document.cxx
@@ -63,7 +63,11 @@ theList.Remove(it);
 
 Handle(TDocStd_Document) TDocStd_Document::Get (const TDF_Label& acces)
 {
-  return TDocStd_Owner::GetDocument(acces.Data());
+  // avoid creation of Handle(TDF_Data) during TDF_Data destruction
+  if (acces.Root().HasAttribute()) {
+    return TDocStd_Owner::GetDocument(acces.Data());
+  }
+  return Handle(TDocStd_Document)();
 }
 
 //=======================================================================

diff --git a/src/TDocStd/TDocStd_Document.hxx b/src/TDocStd/TDocStd_Document.hxx
index 0d07b103d1df4f183215802c1d7ed073f342266e..2454a6525234f6cd523b3d32c78e8e2c1a6114f1 100644 (file)
--- a/src/TDocStd/TDocStd_Document.hxx
+++ b/src/TDocStd/TDocStd_Document.hxx
@@ -63,6 +63,9 @@ public:
   
   //! Constructs a document object defined by the
   //! string astorageformat.
+  //! If a document is created outside of an application using this constructor, it must be
+  //! managed by a Handle. Otherwise memory problems could appear: call of TDocStd_Owner::GetDocument
+  //! creates a Handle(TDocStd_Document), so, releasing it will produce a crash.
   Standard_EXPORT TDocStd_Document(const TCollection_ExtendedString& astorageformat);
   
   //! the document is saved in a file.

diff --git a/src/TDocStd/TDocStd_Owner.cxx b/src/TDocStd/TDocStd_Owner.cxx
index aeba4b2b18a87b583e38e2be27dbdd1c94d27545..9c4e33b7b2409550e333b6d87407864c3690be4c 100644 (file)
--- a/src/TDocStd/TDocStd_Owner.cxx
+++ b/src/TDocStd/TDocStd_Owner.cxx
@@ -43,12 +43,31 @@ const Standard_GUID& TDocStd_Owner::GetID()
 //=======================================================================
 
 void TDocStd_Owner::SetDocument (const Handle(TDF_Data)& indata,
-                                const Handle(TDocStd_Document)& D) 
+                                const Handle(TDocStd_Document)& doc) 
 {
   Handle(TDocStd_Owner) A;
   if (!indata->Root().FindAttribute (TDocStd_Owner::GetID(), A)) {
     A = new TDocStd_Owner (); 
-    A->SetDocument(D);
+    A->SetDocument(doc);
+    indata->Root().AddAttribute(A);
+  }
+  else {  
+    throw Standard_DomainError("TDocStd_Owner::SetDocument : already called");
+  }
+}
+
+//=======================================================================
+//function : SetDocument
+//purpose  : 
+//=======================================================================
+
+void TDocStd_Owner::SetDocument (const Handle(TDF_Data)& indata,
+         TDocStd_Document* doc) 
+{
+  Handle(TDocStd_Owner) A;
+  if (!indata->Root().FindAttribute (TDocStd_Owner::GetID(), A)) {
+    A = new TDocStd_Owner (); 
+    A->SetDocument(doc);
     indata->Root().AddAttribute(A);
   }
   else {  
@@ -75,7 +94,7 @@ Handle(TDocStd_Document) TDocStd_Owner::GetDocument (const Handle(TDF_Data)& ofd
 //purpose  : 
 //=======================================================================
 
-TDocStd_Owner::TDocStd_Owner () { }
+TDocStd_Owner::TDocStd_Owner() { }
 
 
 //=======================================================================
@@ -83,19 +102,29 @@ TDocStd_Owner::TDocStd_Owner () { }
 //purpose  : 
 //=======================================================================
 
-void TDocStd_Owner::SetDocument(const Handle( TDocStd_Document)& D) 
+void TDocStd_Owner::SetDocument (const Handle( TDocStd_Document)& document) 
 {
-  myDocument = D;
+  myDocument = document.get();
 }
 
+//=======================================================================
+//function : SetDocument
+//purpose  : 
+//=======================================================================
+
+void TDocStd_Owner::SetDocument (TDocStd_Document* document)
+{
+  myDocument = document;
+}
 
 //=======================================================================
 //function : Get
 //purpose  : 
 //=======================================================================
 
-Handle(TDocStd_Document) TDocStd_Owner::GetDocument () const 
-{ return myDocument; 
+Handle(TDocStd_Document) TDocStd_Owner::GetDocument() const 
+{
+  return Handle(TDocStd_Document)(myDocument); 
 }
 
 
@@ -104,7 +133,7 @@ Handle(TDocStd_Document) TDocStd_Owner::GetDocument () const
 //purpose  : 
 //=======================================================================
 
-const Standard_GUID& TDocStd_Owner::ID () const { return GetID(); }
+const Standard_GUID& TDocStd_Owner::ID() const { return GetID(); }
 
 
 //=======================================================================
@@ -112,7 +141,7 @@ const Standard_GUID& TDocStd_Owner::ID () const { return GetID(); }
 //purpose  : 
 //=======================================================================
 
-Handle(TDF_Attribute) TDocStd_Owner::NewEmpty () const
+Handle(TDF_Attribute) TDocStd_Owner::NewEmpty() const
 {
   Handle(TDF_Attribute) dummy;
   return dummy;
@@ -123,7 +152,7 @@ Handle(TDF_Attribute) TDocStd_Owner::NewEmpty () const
 //purpose  : 
 //=======================================================================
 
-void TDocStd_Owner::Restore(const Handle(TDF_Attribute)&) 
+void TDocStd_Owner::Restore (const Handle(TDF_Attribute)&) 
 {
 }
 
@@ -156,6 +185,6 @@ void TDocStd_Owner::DumpJson (Standard_OStream& theOStream, Standard_Integer the
 {
   OCCT_DUMP_TRANSIENT_CLASS_BEGIN (theOStream)
 
-  OCCT_DUMP_FIELD_VALUES_DUMPED (theOStream, theDepth, myDocument.get())
+  OCCT_DUMP_FIELD_VALUES_DUMPED (theOStream, theDepth, myDocument)
 }
 

diff --git a/src/TDocStd/TDocStd_Owner.hxx b/src/TDocStd/TDocStd_Owner.hxx
index 69e5b0534fc484d79216902b3b626fef38e9dc55..3b36f82380810b1c3cc013c52ba056c284bdfe85 100644 (file)
--- a/src/TDocStd/TDocStd_Owner.hxx
+++ b/src/TDocStd/TDocStd_Owner.hxx
@@ -47,7 +47,9 @@ public:
   Standard_EXPORT static const Standard_GUID& GetID();
   
   Standard_EXPORT static void SetDocument (const Handle(TDF_Data)& indata, const Handle(TDocStd_Document)& doc);
-  
+
+  Standard_EXPORT static void SetDocument (const Handle(TDF_Data)& indata, TDocStd_Document* doc);
+
   //! Owner methods
   //! ===============
   Standard_EXPORT static Handle(TDocStd_Document) GetDocument (const Handle(TDF_Data)& ofdata);
@@ -55,7 +57,9 @@ public:
   Standard_EXPORT TDocStd_Owner();
   
   Standard_EXPORT void SetDocument (const Handle(TDocStd_Document)& document);
-  
+
+  Standard_EXPORT void SetDocument (TDocStd_Document* document);
+
   Standard_EXPORT Handle(TDocStd_Document) GetDocument() const;
   
   Standard_EXPORT const Standard_GUID& ID() const Standard_OVERRIDE;
@@ -83,8 +87,8 @@ protected:
 
 private:
 
-
-  Handle(TDocStd_Document) myDocument;
+  //! It keeps pointer to the document to avoid handles cyclic dependency
+  TDocStd_Document* myDocument;
 
 
 };

diff --git a/src/TPrsStd/TPrsStd_DriverTable.cxx b/src/TPrsStd/TPrsStd_DriverTable.cxx
index 32771cde249152f5faf1989136cdb95d873bb0d2..857ccc9f7402415f2c309a181383d9c53ac57172 100644 (file)
--- a/src/TPrsStd/TPrsStd_DriverTable.cxx
+++ b/src/TPrsStd/TPrsStd_DriverTable.cxx
@@ -47,6 +47,8 @@ Handle(TPrsStd_DriverTable) TPrsStd_DriverTable::Get()
   if ( drivertable.IsNull() )
   {
     drivertable = new TPrsStd_DriverTable;
+    // it must be never destroyed, even this library is unloaded
+    new Handle(TPrsStd_DriverTable)(drivertable);
 #ifdef OCCT_DEBUG
     std::cout << "The new TPrsStd_DriverTable was created" << std::endl;
 #endif

diff --git a/tests/bugs/caf/bug31075 b/tests/bugs/caf/bug31075
new file mode 100644 (file)
index 0000000..e453f0d
--- /dev/null
+++ b/tests/bugs/caf/bug31075
@@ -0,0 +1,17 @@
+puts "============"
+puts "0031075: Application Framework - reading STEP file into TDocStd_Document leads to memory leaks"
+puts "============"
+
+pload XDE
+
+puts "Create a new document outside of application, add a shape an release all variables in cycle to see if allocated heap memory grows"
+set listmem {}
+for {set i 1} {$i < 100} {incr i} {
+  NewDocument D
+  box aBox 10 10 10
+  XAddShape D aBox
+  unset aBox
+  unset D
+  lappend listmem [meminfo h]
+  checktrend $listmem 0 0 "Memory leak"
+}

diff --git a/tests/bugs/xde/bug22776 b/tests/bugs/xde/bug22776
index 9edb1892378b7d27349638d01989e4d8bdc1cb6c..aeb15262dc8f5921d162f5d41439871d5812e4fe 100755 (executable)
--- a/tests/bugs/xde/bug22776
+++ b/tests/bugs/xde/bug22776
@@ -122,3 +122,5 @@ if { ${status} == 0} {
 }
 
 checkview -display result -3d -path ${imagedir}/${test_image}.png
+
+Close D