0023260: Regression: Instability in parallel incmesh on Linux.

Added protection to the function which may have data race (according to the valgrind report).
Added protection to the BRepMesh_FastDiscretFace::RestoreStructureFromTriangulation function
Slight reordering to optimize use of mutex (lock once)
Now Standard_Mutex::SentryNested are created as named object.
Map inside TopTools_MutexForShapeProvider now store Handle_TopoDS_TShape as a key instead of TopoDS_Shape

diff --git a/src/BRepMesh/BRepMesh_FastDiscretFace.cdl b/src/BRepMesh/BRepMesh_FastDiscretFace.cdl
index 53e2c6932663ded18eba354cdd1ea8d950b10c26..b35e17fad6a5c8daae5ddb04ebb9307cf263ae48 100755 (executable)
--- a/src/BRepMesh/BRepMesh_FastDiscretFace.cdl
+++ b/src/BRepMesh/BRepMesh_FastDiscretFace.cdl
@@ -84,7 +84,8 @@ is
                 theSurf    : HSurface      from BRepAdaptor;
                 theTrigu   : Triangulation from Poly;
                 theDefEdge : Real          from Standard;
-                theLoc     : Location      from TopLoc)
+                theLoc     : Location      from TopLoc;
+                theMutexProvider: MutexForShapeProvider from TopTools)
           returns Boolean from Standard is protected;
         
  

diff --git a/src/BRepMesh/BRepMesh_FastDiscretFace.cxx b/src/BRepMesh/BRepMesh_FastDiscretFace.cxx
index 5cacdf67626eaa8ccbaffd325fc08bd1c86bd7a6..825b9b5e2a10544a981af2879ade87a07a35b819 100755 (executable)
--- a/src/BRepMesh/BRepMesh_FastDiscretFace.cxx
+++ b/src/BRepMesh/BRepMesh_FastDiscretFace.cxx
@@ -57,6 +57,8 @@
 
 #define UVDEFLECTION 1.e-05
 
+static Standard_Mutex DummyMutex;
+
 static Standard_Real FUN_CalcAverageDUV(TColStd_Array1OfReal& P, const Standard_Integer PLen)
 {
   Standard_Integer i, j, n = 0;
@@ -173,8 +175,7 @@ void BRepMesh_FastDiscretFace::Add(const TopoDS_Face&                    theFace
         const TopoDS_Edge& edge = TopoDS::Edge(ex.Value());
         if(edge.IsNull())
           continue;
-
-        RestoreStructureFromTriangulation(edge, face, gFace, aFaceTrigu, theMapDefle(edge), loc);
+        RestoreStructureFromTriangulation(edge, face, gFace, aFaceTrigu, theMapDefle(edge), loc, theMutexProvider);
       }
     }
     
@@ -349,22 +350,28 @@ Standard_Boolean BRepMesh_FastDiscretFace::RestoreStructureFromTriangulation
                                 const Handle(BRepAdaptor_HSurface)& theSurf,
                                 const Handle(Poly_Triangulation)&   theTrigu,
                                 const Standard_Real                 theDefEdge,
-                                const TopLoc_Location&              theLoc)
+                                const TopLoc_Location&              theLoc,
+                                const TopTools_MutexForShapeProvider& theMutexProvider)
 {
+  // 2d vertex indices
+  TopAbs_Orientation orEdge = theEdge.Orientation();
+  // Get end points on 2d curve
+  gp_Pnt2d uvFirst, uvLast;
   // oan: changes for right restoring of triangulation data from face & edges
   Handle(Poly_PolygonOnTriangulation) Poly;
-  Poly = BRep_Tool::PolygonOnTriangulation(theEdge, theTrigu, theLoc);
 
-  if (Poly.IsNull() || !Poly->HasParameters())
   {
-    return Standard_False;
+    // lock mutex during querying data from edge curves to prevent parallel change of the same data
+    Standard_Mutex* aMutex = theMutexProvider.GetMutex(theEdge);
+    Standard_Mutex::SentryNested aSentry(aMutex == NULL ? DummyMutex : *aMutex,
+                                  aMutex != NULL);
+
+    Poly = BRep_Tool::PolygonOnTriangulation(theEdge, theTrigu, theLoc);
+    if (Poly.IsNull() || !Poly->HasParameters())
+      return Standard_False;
+
+    BRep_Tool::UVPoints(theEdge, theFace, uvFirst, uvLast);
   }
-  
-  // 2d vertex indices
-  TopAbs_Orientation orEdge = theEdge.Orientation();
-  // Get end points on 2d curve
-  gp_Pnt2d uvFirst, uvLast;
-  BRep_Tool::UVPoints(theEdge, theFace, uvFirst, uvLast);
 
   // Get vertices
   TopoDS_Vertex pBegin, pEnd;
@@ -1523,8 +1530,6 @@ Standard_Real BRepMesh_FastDiscretFace::Control(const Handle(BRepAdaptor_HSurfac
   return Sqrt(maxdef);
 }
 
-static Standard_Mutex DummyMutex;
-
 //=======================================================================
 //function : AddInShape
 //purpose  : 
@@ -1631,7 +1636,7 @@ void BRepMesh_FastDiscretFace::AddInShape(const TopoDS_Face&  theFace,
 
       // lock mutex to prevent parallel change of the same data
       Standard_Mutex* aMutex = theMutexProvider.GetMutex(It.Key());
-      Standard_Mutex::SentryNested (aMutex == NULL ? DummyMutex : *aMutex,
+      Standard_Mutex::SentryNested aSentry(aMutex == NULL ? DummyMutex : *aMutex,
                                     aMutex != NULL);
 
       if ( NOD1 == NOD2 ) {

diff --git a/src/TopTools/TopTools_MutexForShapeProvider.cxx b/src/TopTools/TopTools_MutexForShapeProvider.cxx
index aa5222bde2c3646a8fbe8ef9eb14298b791c042c..b9572c0487e8d4425b85c872411de987705600f4 100644 (file)
--- a/src/TopTools/TopTools_MutexForShapeProvider.cxx
+++ b/src/TopTools/TopTools_MutexForShapeProvider.cxx
@@ -19,8 +19,10 @@
 
 #include <TopTools_MutexForShapeProvider.hxx>
 
+#include <Standard_Mutex.hxx>
 #include <TopoDS_Iterator.hxx>
 
+
 // macro to compare two types of shapes
 #define SAMETYPE(x,y) ((x) == (y))
 #define LESSCOMPLEX(x,y) ((x) > (y))
@@ -73,10 +75,10 @@ void TopTools_MutexForShapeProvider::CreateMutexesForSubShapes(const TopoDS_Shap
 //=======================================================================
 void TopTools_MutexForShapeProvider::CreateMutexForShape(const TopoDS_Shape& theShape)
 {
-  if (!myMap.IsBound(theShape))
+  if (!myMap.IsBound(theShape.TShape()))
   {
     Standard_Mutex* aMutex = new Standard_Mutex();
-    myMap.Bind(theShape, aMutex);
+    myMap.Bind(theShape.TShape(), aMutex);
   }
 }
 
@@ -86,9 +88,9 @@ void TopTools_MutexForShapeProvider::CreateMutexForShape(const TopoDS_Shape& the
 //=======================================================================
 Standard_Mutex* TopTools_MutexForShapeProvider::GetMutex(const TopoDS_Shape& theShape) const
 {
-  if (myMap.IsBound(theShape))
+  if (myMap.IsBound(theShape.TShape()))
   {
-    Standard_Mutex* aMutex = myMap.Find(theShape);
+    Standard_Mutex* aMutex = myMap.Find(theShape.TShape());
     return aMutex;
   }
   else
@@ -103,7 +105,7 @@ Standard_Mutex* TopTools_MutexForShapeProvider::GetMutex(const TopoDS_Shape& the
 //=======================================================================
 void TopTools_MutexForShapeProvider::RemoveAllMutexes()
 {
-  for (NCollection_DataMap<TopoDS_Shape, Standard_Mutex *, TopTools_ShapeMapHasher>::Iterator anIter;
+  for (NCollection_DataMap<TopoDS_Shape, Standard_Mutex *>::Iterator anIter;
          anIter.More(); anIter.Next())
   {
     delete anIter.Value();

diff --git a/src/TopTools/TopTools_MutexForShapeProvider.hxx b/src/TopTools/TopTools_MutexForShapeProvider.hxx
index 409116b99d60579f71e23c845785073a3f42209a..98753f72867eeeebd5b6fd915eb7589101e1ec2a 100644 (file)
--- a/src/TopTools/TopTools_MutexForShapeProvider.hxx
+++ b/src/TopTools/TopTools_MutexForShapeProvider.hxx
@@ -21,17 +21,20 @@
 #ifndef _TopTools_MutexForShapeProvider_HeaderFile
 #define _TopTools_MutexForShapeProvider_HeaderFile
 
+#include <Handle_TopoDS_TShape.hxx>
 #include <NCollection_DataMap.hxx>
-#include <Standard_Mutex.hxx>
 #include <TopAbs_ShapeEnum.hxx>
-#include <TopoDS_Shape.hxx>
-#include <TopTools_ShapeMapHasher.hxx>
 
+class Standard_Mutex;
+class TopoDS_Shape;
 
 //! Class TopTools_MutexForShapeProvider 
 //!   This class is used to create and store mutexes associated with shapes.
 class TopTools_MutexForShapeProvider
 {
+  friend Standard_Boolean IsEqual(const Handle_TopoDS_TShape & theFirstHandle,
+                                  const Handle_TopoDS_TShape & theSecondHandle);
+
 public:
   //! Constructor
   Standard_EXPORT TopTools_MutexForShapeProvider();
@@ -59,8 +62,14 @@ private:
   TopTools_MutexForShapeProvider & operator = (const TopTools_MutexForShapeProvider &);
 
 
-  NCollection_DataMap<TopoDS_Shape, Standard_Mutex *, TopTools_ShapeMapHasher> myMap;
+  NCollection_DataMap<Handle_TopoDS_TShape, Standard_Mutex *> myMap;
 
 };
 
+inline Standard_Boolean IsEqual(const Handle_TopoDS_TShape & theFirstHandle,
+                                const Handle_TopoDS_TShape & theSecondHandle)
+{
+  return (theFirstHandle == theSecondHandle);
+}
+
 #endif
\ No newline at end of file