From f67d0512ac557098bfbf5d25b7b2b08ffa1ad1c7 Mon Sep 17 00:00:00 2001 From: dbv Date: Fri, 20 Jul 2012 17:18:29 +0400 Subject: [PATCH] 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 --- src/BRepMesh/BRepMesh_FastDiscretFace.cdl | 3 +- src/BRepMesh/BRepMesh_FastDiscretFace.cxx | 35 +++++++++++-------- .../TopTools_MutexForShapeProvider.cxx | 12 ++++--- .../TopTools_MutexForShapeProvider.hxx | 17 ++++++--- 4 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/BRepMesh/BRepMesh_FastDiscretFace.cdl b/src/BRepMesh/BRepMesh_FastDiscretFace.cdl index 53e2c69326..b35e17fad6 100755 --- 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 5cacdf6762..825b9b5e2a 100755 --- 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 aa5222bde2..b9572c0487 100644 --- a/src/TopTools/TopTools_MutexForShapeProvider.cxx +++ b/src/TopTools/TopTools_MutexForShapeProvider.cxx @@ -19,8 +19,10 @@ #include +#include #include + // 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::Iterator anIter; + for (NCollection_DataMap::Iterator anIter; anIter.More(); anIter.Next()) { delete anIter.Value(); diff --git a/src/TopTools/TopTools_MutexForShapeProvider.hxx b/src/TopTools/TopTools_MutexForShapeProvider.hxx index 409116b99d..98753f7286 100644 --- 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 #include -#include #include -#include -#include +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 myMap; + NCollection_DataMap 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 -- 2.20.1