0031075: Application Framework - reading STEP file into TDocStd_Document leads to... IR-2020-06-19
authormpv <mikhail.ponikarov@opencascade.com>
Thu, 18 Jun 2020 07:24:07 +0000 (10:24 +0300)
committerbugmaster <bugmaster@opencascade.com>
Fri, 19 Jun 2020 16:08:46 +0000 (19:08 +0300)
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

src/QABugs/QABugs_1.cxx
src/Standard/Standard_Type.cxx
src/TDF/TDF_Data.cxx
src/TDocStd/TDocStd_Document.cxx
src/TDocStd/TDocStd_Document.hxx
src/TDocStd/TDocStd_Owner.cxx
src/TDocStd/TDocStd_Owner.hxx
src/TPrsStd/TPrsStd_DriverTable.cxx
tests/bugs/caf/bug31075 [new file with mode: 0644]
tests/bugs/xde/bug22776

index bce0b3b..244aeb4 100644 (file)
@@ -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); 
index 9516915..b18c8b0 100644 (file)
@@ -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,
index 44ba6da..44e103c 100644 (file)
@@ -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;
 }
index f79a5e4..e72d5ca 100644 (file)
@@ -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)();
 }
 
 //=======================================================================
index 0d07b10..2454a65 100644 (file)
@@ -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.
index aeba4b2..9c4e33b 100644 (file)
@@ -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)
 }
 
index 69e5b05..3b36f82 100644 (file)
@@ -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;
 
 
 };
index 32771cd..857ccc9 100644 (file)
@@ -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
@@ -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"
+}
index 9edb189..aeb1526 100755 (executable)
@@ -122,3 +122,5 @@ if { ${status} == 0} {
 }
 
 checkview -display result -3d -path ${imagedir}/${test_image}.png
+
+Close D