0023345: Crash when destroying OpenGl_Element

OpenGl_PrimitiveArray::Release() - avoid possible NULL-pointer dereference
OpenGl_GraphicDriver::RemoveView() - release GL resources within removing last view
Fixed OCC280 test command - do not remove old view until new one is initialized

diff --git a/src/OpenGl/OpenGl_GraphicDriver_7.cxx b/src/OpenGl/OpenGl_GraphicDriver_7.cxx
index b7e6414e71e6f1d8f4738dcaa3fabdefb07e109c..d5451548d2db5a8b45001fea1d01628645808900 100755 (executable)
--- a/src/OpenGl/OpenGl_GraphicDriver_7.cxx
+++ b/src/OpenGl/OpenGl_GraphicDriver_7.cxx
@@ -409,17 +409,30 @@ Standard_Boolean OpenGl_Workspace::BufferDump (OpenGl_FrameBuffer*         theFB
   return Standard_True;
 }
 
-void OpenGl_GraphicDriver::RemoveView (const Graphic3d_CView& ACView)
+void OpenGl_GraphicDriver::RemoveView (const Graphic3d_CView& theCView)
 {
-  if (myMapOfView.IsBound (ACView.ViewId))
-    myMapOfView.UnBind (ACView.ViewId);
+  Handle(OpenGl_Context) aShareCtx = GetSharedContext();
+  if (myMapOfView.IsBound (theCView.ViewId))
+    myMapOfView.UnBind (theCView.ViewId);
+
+  if (myMapOfWS.IsBound (theCView.WsId))
+    myMapOfWS.UnBind (theCView.WsId);
 
-  if (myMapOfWS.IsBound (ACView.WsId))
-    myMapOfWS.UnBind (ACView.WsId);
+  if (myMapOfWS.IsEmpty() && !myMapOfStructure.IsEmpty())
+  {
+    // The last view removed but some objects still present.
+    // Release GL resources now without object destruction.
+    for (NCollection_DataMap<Standard_Integer, OpenGl_Structure*>::Iterator aStructIt (myMapOfStructure);
+         aStructIt.More (); aStructIt.Next())
+    {
+      OpenGl_Structure* aStruct = aStructIt.ChangeValue();
+      aStruct->ReleaseGlResources (aShareCtx);
+    }
+  }
 
-  OpenGl_CView *aCView = (OpenGl_CView *)ACView.ptrView;
+  OpenGl_CView *aCView = (OpenGl_CView *)theCView.ptrView;
   delete aCView;
-  ((Graphic3d_CView *)&ACView)->ptrView = NULL;
+  ((Graphic3d_CView *)&theCView)->ptrView = NULL;
 }
 
 void OpenGl_GraphicDriver::SetLight (const Graphic3d_CView& ACView)

diff --git a/src/OpenGl/OpenGl_PrimitiveArray.cxx b/src/OpenGl/OpenGl_PrimitiveArray.cxx
index 8bb73e82689ab581dd8858d58d5c706d4ed7151d..b29d51e69fd069944ec695a0d262c3d678b2e3d6 100755 (executable)
--- a/src/OpenGl/OpenGl_PrimitiveArray.cxx
+++ b/src/OpenGl/OpenGl_PrimitiveArray.cxx
@@ -17,7 +17,6 @@
 // purpose or non-infringement. Please see the License for the specific terms
 // and conditions governing the rights and limitations under the License.
 
-
 #include <OpenGl_IndexBuffer.hxx>
 #include <OpenGl_Context.hxx>
 
@@ -1552,7 +1551,10 @@ void OpenGl_PrimitiveArray::Release (const Handle(OpenGl_Context)& theContext)
   {
     if (!myVbos[anIter].IsNull())
     {
-      theContext->DelayedRelease (myVbos[anIter]);
+      if (!theContext.IsNull())
+      {
+        theContext->DelayedRelease (myVbos[anIter]);
+      }
       myVbos[anIter].Nullify();
     }
   }

diff --git a/src/OpenGl/OpenGl_Structure.cxx b/src/OpenGl/OpenGl_Structure.cxx
index 8730522a4b8fce0a5d6fdce9b64263b91715fada..20d5afd6adc659d133f98b6a409e31f6a869de99 100644 (file)
--- a/src/OpenGl/OpenGl_Structure.cxx
+++ b/src/OpenGl/OpenGl_Structure.cxx
@@ -447,6 +447,42 @@ void OpenGl_Structure::Release (const Handle(OpenGl_Context)& theGlCtx)
   ClearHighlightColor (theGlCtx);
 }
 
+// =======================================================================
+// function : ReleaseGlResources
+// purpose  :
+// =======================================================================
+void OpenGl_Structure::ReleaseGlResources (const Handle(OpenGl_Context)& theGlCtx)
+{
+  for (OpenGl_ListOfGroup::Iterator anIter (myGroups); anIter.More(); anIter.Next())
+  {
+    OpenGl_Group* aGroup = const_cast<OpenGl_Group*& > (anIter.ChangeValue());
+    if (aGroup != NULL)
+    {
+      aGroup->Release (theGlCtx);
+    }
+  }
+  if (myAspectLine != NULL)
+  {
+    myAspectLine->Release (theGlCtx);
+  }
+  if (myAspectFace != NULL)
+  {
+    myAspectFace->Release (theGlCtx);
+  }
+  if (myAspectMarker != NULL)
+  {
+    myAspectMarker->Release (theGlCtx);
+  }
+  if (myAspectText != NULL)
+  {
+    myAspectText->Release (theGlCtx);
+  }
+  if (myHighlightBox != NULL)
+  {
+    myHighlightBox->Release (theGlCtx);
+  }
+}
+
 //=======================================================================
 //function : SetZLayer
 //purpose  : 

diff --git a/src/OpenGl/OpenGl_Structure.hxx b/src/OpenGl/OpenGl_Structure.hxx
index 11d69858233e3a99f051078a0855930de3569f1b..265059272e7472d2eec8ccf5fbb6667b60ad5ee8 100644 (file)
--- a/src/OpenGl/OpenGl_Structure.hxx
+++ b/src/OpenGl/OpenGl_Structure.hxx
@@ -84,6 +84,14 @@ public:
   virtual void Render  (const Handle(OpenGl_Workspace)& theWorkspace) const;
   virtual void Release (const Handle(OpenGl_Context)&   theGlCtx);
 
+  //! This method releases GL resources without actual elements destruction.
+  //! As result structure could be correctly destroyed layter without GL context
+  //! (after last window was closed for example).
+  //!
+  //! Notice however that reusage of this structure after calling this method is incorrect
+  //! and will lead to broken visualization due to loosed data.
+  void ReleaseGlResources (const Handle(OpenGl_Context)& theGlCtx);
+
 protected:
 
   virtual ~OpenGl_Structure();

diff --git a/src/QABugs/QABugs_17.cxx b/src/QABugs/QABugs_17.cxx
index 79b53184330a89832cd6633a51c09b1f3e4de3cf..13f7e4a2901c5a88ce9af6909472002e36b9537c 100644 (file)
--- a/src/QABugs/QABugs_17.cxx
+++ b/src/QABugs/QABugs_17.cxx
@@ -581,8 +581,6 @@ static Standard_Integer OCC280 (Draw_Interpretor& di, Standard_Integer argc, con
 
   Handle(Aspect_Window) asp = anOldView->Window();
   aViewer->SetViewOff (anOldView);
-  anOldView->Remove();
-  anOldView.Nullify();
 
   Handle(V3d_View) aNewView = aViewer->CreateView();
   ViewerTest::CurrentView (aNewView);
@@ -590,6 +588,9 @@ static Standard_Integer OCC280 (Draw_Interpretor& di, Standard_Integer argc, con
   aNewView->SetWindow (asp);
   if (!asp->IsMapped()) asp->Map();
 
+  anOldView->Remove();
+  anOldView.Nullify();
+
   aNewView->Update();
 
   // replace view in event manager