0029182: BOPAlgo_PaveFiller sometimes raises exception in parallel mode

Data races have been prevented in the code of BOPAlgo_PaveFiller that makes pcurves of edges on faces. For that:
- Put into treatment only unique edge-face pairs.
- If the same edge is treated with different faces in different threads simultaneously this also causes data races. To avoid this make the edge's copy in each thread and update the copy. The original edge is updated only after finishing parallel processing.

The new method BOPTools_AlgoTools::CopyEdge has been added to make a copy of an edge with vertices.

Big screenshot in the test script tests/bugs/modalg_7/bug28200 has been replaced with a small one.

src/BOPAlgo/BOPAlgo_PaveFiller_7.cxx

@@ -29,6 +29,7 @@
 #include <BOPDS_Interf.hxx>
 #include <BOPDS_Iterator.hxx>
 #include <BOPDS_ListOfPaveBlock.hxx>
+#include <BOPDS_MapOfPair.hxx>
 #include <BOPDS_MapOfPaveBlock.hxx>
 #include <BOPDS_Pave.hxx>
 #include <BOPDS_PaveBlock.hxx>
@@ -246,28 +247,42 @@ class BOPAlgo_MPC : public BOPAlgo_Algo  {
     {
       OCC_CATCH_SIGNALS
 
-      Standard_Integer iErr;
-      //
-      iErr=1;
-      if (!myEz.IsNull()) {
-        TopoDS_Edge aSpz;
-        //
-        BOPTools_AlgoTools::MakeSplitEdge(myEz,myV1, myT1, 
-                                          myV2, myT2, aSpz);
-        //
-        iErr=
-          BOPTools_AlgoTools2D::AttachExistingPCurve(aSpz, 
-                                                     myE, 
-                                                     myF, 
-                                                     myContext);
+      // Check if edge has pcurve. If no then make its copy to avoid data races,
+      // and use it to build pcurve.
+      TopoDS_Edge aCopyE = myE;
+      Standard_Real f, l;
+      Handle(Geom2d_Curve) aC2d = BRep_Tool::CurveOnSurface(aCopyE, myF, f, l);
+      if (aC2d.IsNull())
+      {
+        aCopyE = BOPTools_AlgoTools::CopyEdge(aCopyE);
+
+        Standard_Integer iErr = 1;
+        if (!myEz.IsNull())
+        {
+          // Attach pcurve from the original edge
+          TopoDS_Edge aSpz;
+          BOPTools_AlgoTools::MakeSplitEdge(myEz, myV1, myT1,
+                                            myV2, myT2, aSpz);
+          iErr = BOPTools_AlgoTools2D::AttachExistingPCurve(aSpz,
+                                                            aCopyE, 
+                                                            myF,
+                                                            myContext);
+        }
+        if (iErr)
+          BOPTools_AlgoTools2D::BuildPCurveForEdgeOnFace(aCopyE, myF, myContext);
+
+        myNewC2d = BRep_Tool::CurveOnSurface(aCopyE, myF, f, l);
+        if (myNewC2d.IsNull())
+        {
+          AddError(new BOPAlgo_AlertBuildingPCurveFailed(TopoDS_Shape()));
+          return;
+        }
+        else
+          myNewTol = BRep_Tool::Tolerance(aCopyE);
       }
-      //
-      if (iErr) { 
-        BOPTools_AlgoTools2D::BuildPCurveForEdgeOnFace(myE, myF, myContext);
-      }
-      // 
+
       if (myFlag) {
-        UpdateVertices(myE, myF);
+        UpdateVertices(aCopyE, myF);
       }
     }
     catch (Standard_Failure)
@@ -275,7 +290,17 @@ class BOPAlgo_MPC : public BOPAlgo_Algo  {
       AddError(new BOPAlgo_AlertBuildingPCurveFailed(TopoDS_Shape()));
     }
   }
-  //
+
+  const Handle(Geom2d_Curve)& GetNewPCurve() const
+  {
+    return myNewC2d;
+  }
+
+  Standard_Real GetNewTolerance() const
+  {
+    return myNewTol;
+  }
+
  protected:
   Standard_Boolean myFlag;
   TopoDS_Edge myE;
@@ -285,6 +310,8 @@ class BOPAlgo_MPC : public BOPAlgo_Algo  {
   Standard_Real myT1;
   TopoDS_Vertex myV2;
   Standard_Real myT2;
+  Handle(Geom2d_Curve) myNewC2d;
+  Standard_Real myNewTol;
   //
   Handle(IntTools_Context) myContext;
 };
@@ -541,7 +568,7 @@ void BOPAlgo_PaveFiller::MakePCurves()
       (!mySectionAttribute.PCurveOnS1() && !mySectionAttribute.PCurveOnS2()))
     return;
   Standard_Boolean bHasPC;
-  Standard_Integer i, nF1, nF2, aNbC, k, nE, aNbFF, aNbFI, nEx;
+  Standard_Integer i, nF1, aNbC, k, nE, aNbFF, aNbFI, nEx;
   Standard_Integer j, aNbPBIn, aNbPBOn;
   BOPDS_ListIteratorOfListOfPaveBlock aItLPB;
   TopoDS_Face aF1F, aF2F;
@@ -637,44 +664,52 @@ void BOPAlgo_PaveFiller::MakePCurves()
     }
   }// for (i=0; i<aNbFI; ++i) {
   //
-  // 2. Process section edges
+  // 2. Process section edges. P-curves on them must already be computed.
+  // However, we must provide the call to UpdateVertices.
   Standard_Boolean bPCurveOnS[2];
-  Standard_Integer m;
-  TopoDS_Face aFf[2];
-  //
   bPCurveOnS[0]=mySectionAttribute.PCurveOnS1();
   bPCurveOnS[1]=mySectionAttribute.PCurveOnS2();
   //
   if (bPCurveOnS[0] || bPCurveOnS[1]) {
+    // container to remember already added edge-face pairs
+    BOPDS_MapOfPair anEFPairs;
     BOPDS_VectorOfInterfFF& aFFs=myDS->InterfFF();
     aNbFF=aFFs.Extent();
     for (i=0; i<aNbFF; ++i) {
       const BOPDS_InterfFF& aFF=aFFs(i);
-      aFF.Indices(nF1, nF2);
+      const BOPDS_VectorOfCurve& aVNC = aFF.Curves();
+      aNbC = aVNC.Extent();
+      if (aNbC == 0)
+        continue;
+      Standard_Integer nF[2];
+      aFF.Indices(nF[0], nF[1]);
       //
-      aFf[0]=(*(TopoDS_Face *)(&myDS->Shape(nF1)));
+      TopoDS_Face aFf[2];
+      aFf[0] = (*(TopoDS_Face *)(&myDS->Shape(nF[0])));
       aFf[0].Orientation(TopAbs_FORWARD);
       //
-      aFf[1]=(*(TopoDS_Face *)(&myDS->Shape(nF2)));
+      aFf[1]=(*(TopoDS_Face *)(&myDS->Shape(nF[1])));
       aFf[1].Orientation(TopAbs_FORWARD);
       //
-      const BOPDS_VectorOfCurve& aVNC=aFF.Curves();
-      aNbC=aVNC.Extent();
-      for (k=0; k<aNbC; ++k) {
+      for (k=0; k<aNbC; ++k)
+      {
         const BOPDS_Curve& aNC=aVNC(k);
         const BOPDS_ListOfPaveBlock& aLPB=aNC.PaveBlocks();
         aItLPB.Initialize(aLPB);
-        for(; aItLPB.More(); aItLPB.Next()) {
+        for(; aItLPB.More(); aItLPB.Next())
+        {
           const Handle(BOPDS_PaveBlock)& aPB=aItLPB.Value();
           nE=aPB->Edge();
           const TopoDS_Edge& aE=(*(TopoDS_Edge *)(&myDS->Shape(nE))); 
           //
-          for (m=0; m<2; ++m) {
-            if (bPCurveOnS[m]) {
-              BOPAlgo_MPC& aMPC=aVMPC.Append1();
+          for (Standard_Integer m = 0; m<2; ++m)
+          {
+            if (bPCurveOnS[m] && anEFPairs.Add(BOPDS_Pair(nE, nF[m])))
+            {
+              BOPAlgo_MPC& aMPC = aVMPC.Append1();
               aMPC.SetEdge(aE);
               aMPC.SetFace(aFf[m]);
-              aMPC.SetFlag(bPCurveOnS[m]);
+              aMPC.SetFlag(Standard_True);
               aMPC.SetProgressIndicator(myProgressIndicator);
             }
           }
@@ -687,18 +722,27 @@ void BOPAlgo_PaveFiller::MakePCurves()
   BOPAlgo_MPCCnt::Perform(myRunParallel, aVMPC, myContext);
   //======================================================
 
-  // Add warnings of the failed projections
+  // Add warnings of the failed projections and update edges with new pcurves
   Standard_Integer aNb = aVMPC.Extent();
   for (i = 0; i < aNb; ++i)
   {
-    if (aVMPC(i).HasErrors())
+    const BOPAlgo_MPC& aMPC = aVMPC(i);
+    if (aMPC.HasErrors())
     {
       TopoDS_Compound aWC;
       BRep_Builder().MakeCompound(aWC);
-      BRep_Builder().Add(aWC, aVMPC(i).Edge());
-      BRep_Builder().Add(aWC, aVMPC(i).Face());
+      BRep_Builder().Add(aWC, aMPC.Edge());
+      BRep_Builder().Add(aWC, aMPC.Face());
       AddWarning(new BOPAlgo_AlertBuildingPCurveFailed(aWC));
     }
+    else
+    {
+      const Handle(Geom2d_Curve)& aNewPC = aMPC.GetNewPCurve();
+      // if aNewPC is null we do not need to update the edge because it already contains
+      // valid p-curve, and only vertices have been updated.
+      if (!aNewPC.IsNull())
+        BRep_Builder().UpdateEdge(aMPC.Edge(), aNewPC, aMPC.Face(), aMPC.GetNewTolerance());
+    }
   }
 }
 //=======================================================================

src/BOPTools/BOPTools_AlgoTools.hxx

@@ -248,7 +248,9 @@ public:
 
   //! Compute a 3D-point on the edge <aEdge> at parameter <aPrm>
   Standard_EXPORT static void PointOnEdge (const TopoDS_Edge& aEdge, const Standard_Real aPrm, gp_Pnt& aP);
-  
+
+  //! Makes a copy of <theEdge> with vertices.
+  Standard_EXPORT static TopoDS_Edge CopyEdge(const TopoDS_Edge& theEdge);
 
   //! Make the edge from base edge <aE1> and two vertices <aV1,aV2>
   //! at parameters <aP1,aP2>

src/BOPTools/BOPTools_AlgoTools_2.cxx

@@ -132,6 +132,20 @@ void BOPTools_AlgoTools::MakeSectEdge(const IntTools_Curve& aIC,
   
 }
 
+//=======================================================================
+// function: CopyEdge
+// purpose: 
+//=======================================================================
+TopoDS_Edge BOPTools_AlgoTools::CopyEdge(const TopoDS_Edge& theEdge)
+{
+  TopoDS_Edge aNewEdge = TopoDS::Edge(theEdge.Oriented(TopAbs_FORWARD));
+  aNewEdge.EmptyCopy();
+  for (TopoDS_Iterator it(theEdge, Standard_False); it.More(); it.Next())
+    BRep_Builder().Add(aNewEdge, it.Value());
+  aNewEdge.Orientation(theEdge.Orientation());
+  return aNewEdge;
+}
+
 //=======================================================================
 // function: MakeSplitEdge
 // purpose: 
@@ -143,9 +157,6 @@ void BOPTools_AlgoTools::MakeSplitEdge(const TopoDS_Edge&   aE,
                                        const Standard_Real  aP2,
                                        TopoDS_Edge& aNewEdge)
 {
-  Standard_Real aTol;//f, l, 
-  aTol=BRep_Tool::Tolerance(aE);
-  //
   TopoDS_Edge E = TopoDS::Edge(aE.Oriented(TopAbs_FORWARD));
   E.EmptyCopy();
   //
@@ -157,7 +168,6 @@ void BOPTools_AlgoTools::MakeSplitEdge(const TopoDS_Edge&   aE,
     BB.Add  (E, aV2);
   }
   BB.Range(E, aP1, aP2);
-  BB.UpdateEdge(E, aTol);
   aNewEdge=E;
   aNewEdge.Orientation(aE.Orientation());
 }

tests/bugs/modalg_7/bug28200

@@ -33,7 +33,7 @@ donly result
 fit
 checkview -screenshot -2d -path ${imagedir}/${test_image}_1.png
 
-left
+smallview -Y+Z
 fit
 checkview -screenshot -2d -path ${imagedir}/${test_image}_2.png
 

tests/bugs/modalg_7/bug29182

@@ -0,0 +1,24 @@
+puts "========"
+puts "OCC29182"
+puts "========"
+puts ""
+#################################################
+# BOPAlgo_PaveFiller sometimes raises exception in parallel mode
+#################################################
+
+binrestore [locate_data_file bug29182_access_violation.bin] a
+explode a So
+bclearobjects
+bcleartools
+baddobjects a_1
+baddtools a_2 a_3 a_4 a_5 a_6 a_7 a_8 a_9 a_10 a_11 a_12
+brunparallel 1
+bfillds
+bbuild result
+
+checknbshapes result -m result -shell 17 -solid 17
+checkprops result -v 1.18596e+006 -s 1.28028e+006
+checkshape result
+if ![regexp "This shape seems to be OK" [bopcheck result]] {
+  puts "Error: result is self-intersected"
+}

tests/bugs/modalg_7/bug29182_1

@@ -0,0 +1,41 @@
+puts "========"
+puts "OCC29182"
+puts "========"
+puts ""
+#################################################
+# BOPAlgo_PaveFiller sometimes raises exception in parallel mode
+#################################################
+
+# This is synthetic case reproducing the bug.
+# Make a set of cylindrical surface patches intersecting in the same
+# 3d line and run bfillds on them repeatedly 100 times.
+
+cylinder c 0 10 0 1 0 0 10
+mkface fc c pi/3 2*pi/3 -10 10
+
+shape a C
+add fc a
+don a
+
+for {set i 1} {$i <= 10} {incr i} {
+  copy fc f1
+  trotate f1 0 0 0 1 0 0 $i*5
+  add f1 a
+}
+
+brunparallel 1
+bclearobjects
+bcleartools
+baddcompound a
+
+for {set i 1} {$i <= 100} {incr i} {
+  puts "pass $i"
+  set info [string trim [bfillds]]
+  if {$info != "" && [regexp "2D curve" $info]} {
+    break
+  }
+}
+
+if {$i <= 100} {
+  puts "Error: the bug with failure building 2D curve of edge on face is reproduced"
+}