diff --git a/src/QABugs/QABugs_19.cxx b/src/QABugs/QABugs_19.cxx index 245fe29d85..67f8d9e246 100755 --- a/src/QABugs/QABugs_19.cxx +++ b/src/QABugs/QABugs_19.cxx @@ -49,6 +49,7 @@ #include #include #include +#include #include @@ -158,7 +159,6 @@ static Standard_Integer OCC23237 (Draw_Interpretor& di, Standard_Integer /*argc* #ifdef HAVE_TBB -#include #include #include @@ -3126,6 +3126,76 @@ static Standard_Integer OCC25446 (Draw_Interpretor& theDI, return 0; } +//==================================================== +// Auxiliary functor class for the command OCC25545; +// it gets access to a vertex with the given index and +// checks that X coordinate of the point is equal to index; +// if it is not so then a data race is reported. +//==================================================== +struct OCC25545_Functor +{ + OCC25545_Functor(const std::vector& theShapeVec) + : myShapeVec(&theShapeVec), + myIsRaceDetected(0) + {} + + void operator()(size_t i) const + { + if (!myIsRaceDetected) { + const TopoDS_Vertex& aV = TopoDS::Vertex (myShapeVec->at(i)); + gp_Pnt aP = BRep_Tool::Pnt (aV); + if (aP.X () != static_cast (i)) { + Standard_Atomic_Increment(&myIsRaceDetected); + } + } + } + + const std::vector* myShapeVec; + mutable volatile int myIsRaceDetected; +}; + +//======================================================================= +//function : OCC25545 +//purpose : Tests data race when concurrently accessing TopLoc_Location::Transformation() +//======================================================================= +#ifdef HAVE_TBB +static Standard_Integer OCC25545 (Draw_Interpretor& di, + Standard_Integer, + const char **) +{ + // Place vertices in a vector, giving the i-th vertex the + // transformation that translates it on the vector (i,0,0) from the origin. + size_t n = 1000; + std::vector aShapeVec (n); + std::vector aLocVec (n); + TopoDS_Shape aShape = BRepBuilderAPI_MakeVertex (gp::Origin ()); + aShapeVec[0] = aShape; + for (size_t i = 1; i < n; ++i) { + gp_Trsf aT; + aT.SetTranslation (gp_Vec (1, 0, 0)); + aLocVec[i] = aLocVec[i - 1] * aT; + aShapeVec[i] = aShape.Moved (aLocVec[i]); + } + + // Evaluator function will access vertices geometry + // concurrently + OCC25545_Functor aFunc(aShapeVec); + + //concurrently process + tbb::parallel_for (size_t (0), n, aFunc, tbb::simple_partitioner ()); + QVERIFY (!aFunc.myIsRaceDetected); + return 0; +} +#else +static Standard_Integer OCC25545 (Draw_Interpretor&, + Standard_Integer, + const char **argv) +{ + cout << "Test skipped: command " << argv[0] << " requires TBB library" << endl; + return 0; +} +#endif + //======================================================================= //function : OCC25547 //purpose : @@ -3266,6 +3336,10 @@ void QABugs::Commands_19(Draw_Interpretor& theCommands) { theCommands.Add ("OCC25348", "OCC25348", __FILE__, OCC25348, group); theCommands.Add ("OCC25413", "OCC25413 shape", __FILE__, OCC25413, group); theCommands.Add ("OCC25446", "OCC25446 res b1 b2 op", __FILE__, OCC25446, group); + theCommands.Add ("OCC25545", + "no args; tests data race when concurrently accessing \n" + "\t\tTopLoc_Location::Transformation()", + __FILE__, OCC25545, group); theCommands.Add ("OCC25547", "OCC25547", __FILE__, OCC25547, group); return; } diff --git a/src/TopLoc/TopLoc.cdl b/src/TopLoc/TopLoc.cdl index 7247878722..0f25caea1d 100644 --- a/src/TopLoc/TopLoc.cdl +++ b/src/TopLoc/TopLoc.cdl @@ -34,7 +34,6 @@ uses gp is - pointer TrsfPtr to Trsf from gp; class Datum3D; private class ItemLocation; diff --git a/src/TopLoc/TopLoc_ItemLocation.cdl b/src/TopLoc/TopLoc_ItemLocation.cdl index 93567a4a52..6a66f1b493 100644 --- a/src/TopLoc/TopLoc_ItemLocation.cdl +++ b/src/TopLoc/TopLoc_ItemLocation.cdl @@ -30,31 +30,21 @@ private class ItemLocation from TopLoc uses Datum3D from TopLoc, - Trsf from gp, - TrsfPtr from TopLoc + Trsf from gp is Create(D : Datum3D from TopLoc; - P : Integer from Standard; - fromTrsf : Boolean from Standard = Standard_False) returns ItemLocation from TopLoc; + P : Integer from Standard) returns ItemLocation from TopLoc; ---Purpose: Sets the elementary Datum to -- Sets the exponent to

- Create(anOther : ItemLocation from TopLoc) returns ItemLocation from TopLoc; - - Assign(me : in out; anOther : ItemLocation from TopLoc) returns ItemLocation from TopLoc; - ---C++: alias operator= - ---C++: return & - - Destroy(me : in out); - ---C++: alias ~ - fields - myDatum : Datum3D from TopLoc; - myPower : Integer from Standard; - myTrsf : TrsfPtr from TopLoc; + myDatum : Datum3D from TopLoc; + myPower : Integer from Standard; + myTrsf : Trsf from gp; friends - class Location from TopLoc - + class Location from TopLoc, + class SListOfItemLocation from TopLoc + end ItemLocation; diff --git a/src/TopLoc/TopLoc_ItemLocation.cxx b/src/TopLoc/TopLoc_ItemLocation.cxx index 05488e5aaa..8a9ca709d0 100644 --- a/src/TopLoc/TopLoc_ItemLocation.cxx +++ b/src/TopLoc/TopLoc_ItemLocation.cxx @@ -23,45 +23,9 @@ TopLoc_ItemLocation::TopLoc_ItemLocation (const Handle(TopLoc_Datum3D)& D, - const Standard_Integer P, -// const Standard_Boolean fromtrsf) : - const Standard_Boolean ) : + const Standard_Integer P) : myDatum(D), myPower(P), - myTrsf(NULL) + myTrsf (D->Transformation().Powered (P)) { } - - -TopLoc_ItemLocation::TopLoc_ItemLocation(const TopLoc_ItemLocation& anOther): myTrsf(NULL) -{ - if (anOther.myTrsf != NULL) { - myTrsf = new gp_Trsf; - *myTrsf = *(anOther.myTrsf); - } - myDatum = anOther.myDatum; - myPower = anOther.myPower; -} - -TopLoc_ItemLocation& TopLoc_ItemLocation::Assign(const TopLoc_ItemLocation& anOther) -{ - if (anOther.myTrsf == NULL) { - if (myTrsf != NULL) delete myTrsf; - myTrsf = NULL; - } - else if (myTrsf != anOther.myTrsf) { - if (myTrsf == NULL) myTrsf = new gp_Trsf; - *myTrsf = *(anOther.myTrsf); - } - myDatum = anOther.myDatum; - myPower = anOther.myPower; - - return *this; -} - -void TopLoc_ItemLocation::Destroy() -{ - if (myTrsf != NULL) delete myTrsf; - myTrsf = NULL; -} - diff --git a/src/TopLoc/TopLoc_Location.cxx b/src/TopLoc/TopLoc_Location.cxx index db5bd5b0e2..6f384eda4a 100644 --- a/src/TopLoc/TopLoc_Location.cxx +++ b/src/TopLoc/TopLoc_Location.cxx @@ -22,22 +22,9 @@ #include #include #include -#include static const gp_Trsf TheIdentity; - -static Standard_Boolean IsInternalIdentity(const TopLoc_Location& loc) -{ - if (loc.IsIdentity()) { - return Standard_True; - } -// if (loc.FirstDatum()->Transformation().Form() == gp_Identity) { -// return Standard_True; -// } - return Standard_False; -} - //======================================================================= //function : TopLoc_Location //purpose : constructor Identity @@ -74,19 +61,10 @@ TopLoc_Location::TopLoc_Location(const gp_Trsf& T) const gp_Trsf& TopLoc_Location::Transformation() const { - if (IsInternalIdentity(*this)) + if (IsIdentity()) return TheIdentity; - else { - if (myItems.Value().myTrsf == NULL) { - TopLoc_ItemLocation *I = (TopLoc_ItemLocation*) (void*) &this->myItems.Value(); - // CLE - if (I->myTrsf == NULL) I->myTrsf = new gp_Trsf; - *(I->myTrsf) = I->myDatum->Transformation(); - I->myTrsf->Power(I->myPower); - I->myTrsf->PreMultiply(NextLocation().Transformation()); - } - return *(myItems.Value().myTrsf); - } + else + return myItems.Value().myTrsf; } TopLoc_Location::operator gp_Trsf() const @@ -172,7 +150,7 @@ TopLoc_Location TopLoc_Location::Predivided (const TopLoc_Location& Other) TopLoc_Location TopLoc_Location::Powered (const Standard_Integer pwr) const { - if (IsInternalIdentity(*this)) return *this; + if (IsIdentity()) return *this; if (pwr == 1) return *this; if (pwr == 0) return TopLoc_Location(); diff --git a/src/TopLoc/TopLoc_SListNodeOfItemLocation.cdl b/src/TopLoc/TopLoc_SListNodeOfItemLocation.cdl index 8c3ae835fe..854fa8879c 100644 --- a/src/TopLoc/TopLoc_SListNodeOfItemLocation.cdl +++ b/src/TopLoc/TopLoc_SListNodeOfItemLocation.cdl @@ -23,10 +23,6 @@ uses is Create(I : ItemLocation from TopLoc; aTail : SListOfItemLocation from TopLoc) returns SListNodeOfItemLocation from TopLoc; ---C++:inline - - Count(me) returns Integer; - ---C++:inline - ---C++: return & Tail(me) returns SListOfItemLocation from TopLoc; ---C++:inline diff --git a/src/TopLoc/TopLoc_SListOfItemLocation.cdl b/src/TopLoc/TopLoc_SListOfItemLocation.cdl index d2386a4bc4..52d7b3073d 100644 --- a/src/TopLoc/TopLoc_SListOfItemLocation.cdl +++ b/src/TopLoc/TopLoc_SListOfItemLocation.cdl @@ -31,20 +31,6 @@ private class SListOfItemLocation from TopLoc -- SListOfItemLocation Iterator; -- for (Iterator = S; Iterator.More(); Iterator.Next()) -- X = Iterator.Value(); - -- - -- Memory usage is automatically managed for SListOfItemLocations - -- (using reference counts). - ---Example: - -- If S1 and S2 are SListOfItemLocations : - -- if S1.Value() is X. - -- - -- And the following is done : - -- S2 = S1; - -- S2.SetValue(Y); - -- - -- S1.Value() becomes also Y. So SListOfItemLocation must be used - -- with care. Mainly the SetValue() method is - -- dangerous. uses SListNodeOfItemLocation from TopLoc, @@ -56,6 +42,7 @@ raises is Create returns SListOfItemLocation from TopLoc; ---Purpose: Creates an empty List. + ---C++: inline Create(anItem : ItemLocation from TopLoc; aTail : SListOfItemLocation from TopLoc) returns SListOfItemLocation from TopLoc; @@ -64,6 +51,7 @@ is Create(Other : SListOfItemLocation from TopLoc) returns SListOfItemLocation from TopLoc; ---Purpose: Creates a list from an other one. The lists are shared. + ---C++: inline Assign(me : in out; Other : SListOfItemLocation from TopLoc) returns SListOfItemLocation from TopLoc @@ -83,6 +71,7 @@ is ---Level: Public ---Purpose: Sets the list to be empty. ---C++: alias ~ + ---C++: inline is static; Value(me) returns any ItemLocation from TopLoc @@ -94,26 +83,6 @@ is NoSuchObject from Standard is static; - ChangeValue(me : in out) returns any ItemLocation from TopLoc - ---Level: Public - ---Purpose: Returns the current value of the list. An error is - -- raised if the list is empty. This value may be - -- modified. A method modifying the value can be - -- called. The value will be modified in the list. - ---Example: AList.ChangeValue().Modify() - ---C++: return & - raises - NoSuchObject from Standard - is static; - - SetValue(me : in out; anItem : ItemLocation from TopLoc) - ---Level: Public - ---Purpose: Changes the current value in the list. An error is - -- raised if the list is empty. - raises - NoSuchObject from Standard - is static; - Tail(me) returns SListOfItemLocation from TopLoc ---Level: Public ---Purpose: Returns the current tail of the list. On an empty @@ -121,21 +90,6 @@ is ---C++: return const & is static; - ChangeTail(me : in out) returns SListOfItemLocation from TopLoc - ---Level: Public - ---Purpose: Returns the current tail of the list. This tail - -- may be modified. A method modifying the tail can - -- be called. The tail will be modified in the list. - ---Example: AList.ChangeTail().Modify() - ---C++: return & - is static; - - SetTail(me : in out; aList : SListOfItemLocation from TopLoc) - ---Level: Public - ---Purpose: Changes the current tail in the list. On an empty - -- list SetTail is Assign. - is static; - Construct(me : in out; anItem : ItemLocation from TopLoc) ---Level: Public ---Purpose: Replaces the list by a list with as Value @@ -143,25 +97,11 @@ is ---C++: inline is static; - Constructed(me; anItem : ItemLocation from TopLoc) returns SListOfItemLocation from TopLoc - ---Level: Public - ---Purpose: Returns a new list with as Value an the - -- list as tail. - ---C++: inline - is static; - ToTail(me : in out) ---Level: Public ---Purpose: Replaces the list by its tail. ---C++: inline is static; - - Initialize(me : in out; aList : SListOfItemLocation from TopLoc) - ---Level: Public - ---Purpose: Sets the iterator to iterate on the content of - -- . This is Assign(). - ---C++: inline - is static; More(me) returns Boolean ---Level: Public diff --git a/src/TopLoc/TopLoc_SListOfItemLocation.cxx b/src/TopLoc/TopLoc_SListOfItemLocation.cxx index 63fed01e6f..ee66c0e5e0 100644 --- a/src/TopLoc/TopLoc_SListOfItemLocation.cxx +++ b/src/TopLoc/TopLoc_SListOfItemLocation.cxx @@ -23,27 +23,14 @@ //purpose : //======================================================================= -TopLoc_SListOfItemLocation::TopLoc_SListOfItemLocation() -{} - -//======================================================================= -//function : TopLoc_SListOfItemLocation -//purpose : -//======================================================================= - TopLoc_SListOfItemLocation::TopLoc_SListOfItemLocation(const TopLoc_ItemLocation& anItem, const TopLoc_SListOfItemLocation& aTail) : myNode(new TopLoc_SListNodeOfItemLocation(anItem,aTail)) -{} - -//======================================================================= -//function : TopLoc_SListOfItemLocation -//purpose : -//======================================================================= - -TopLoc_SListOfItemLocation::TopLoc_SListOfItemLocation(const TopLoc_SListOfItemLocation& Other) : - myNode(Other.myNode) { + if (!myNode->Tail().IsEmpty()) { + const gp_Trsf& aT = myNode->Tail().Value().myTrsf; + myNode->Value().myTrsf.PreMultiply (aT); + } } //======================================================================= @@ -60,18 +47,6 @@ TopLoc_SListOfItemLocation& TopLoc_SListOfItemLocation::Assign(const TopLoc_SLis return *this; } -//======================================================================= -//function : Clear -//purpose : -//======================================================================= - -void TopLoc_SListOfItemLocation::Clear() -{ - if (!myNode.IsNull()) { - myNode.Nullify(); - } -} - //======================================================================= //function : Value //purpose : @@ -83,28 +58,6 @@ const TopLoc_ItemLocation& TopLoc_SListOfItemLocation::Value() const return myNode->Value(); } -//======================================================================= -//function : ChangeValue -//purpose : -//======================================================================= - -TopLoc_ItemLocation& TopLoc_SListOfItemLocation::ChangeValue() -{ - Standard_NoSuchObject_Raise_if(myNode.IsNull(),"TopLoc_SListOfItemLocation::Value"); - return myNode->Value(); -} - -//======================================================================= -//function : SetValue -//purpose : -//======================================================================= - -void TopLoc_SListOfItemLocation::SetValue(const TopLoc_ItemLocation& anItem) -{ - Standard_NoSuchObject_Raise_if(myNode.IsNull(),"TopLoc_SListOfItemLocation::Value"); - myNode->Value() = anItem; -} - //======================================================================= //function : Tail //purpose : @@ -117,29 +70,3 @@ const TopLoc_SListOfItemLocation& TopLoc_SListOfItemLocation::Tail() const else return *this; } - -//======================================================================= -//function : ChangeTail -//purpose : -//======================================================================= - -TopLoc_SListOfItemLocation& TopLoc_SListOfItemLocation::ChangeTail() -{ - if (!myNode.IsNull()) - return myNode->Tail(); - else - return *this; -} - -//======================================================================= -//function : SetTail -//purpose : -//======================================================================= - -void TopLoc_SListOfItemLocation::SetTail(const TopLoc_SListOfItemLocation& aList) -{ - if (!myNode.IsNull()) - myNode->Tail() = aList; - else - Assign(aList); -} diff --git a/src/TopLoc/TopLoc_SListOfItemLocation.lxx b/src/TopLoc/TopLoc_SListOfItemLocation.lxx index cfc2839afa..576691794f 100644 --- a/src/TopLoc/TopLoc_SListOfItemLocation.lxx +++ b/src/TopLoc/TopLoc_SListOfItemLocation.lxx @@ -14,6 +14,24 @@ // Alternatively, this file may be used under the terms of Open CASCADE // commercial license or contractual agreement. +//======================================================================= +//function : TopLoc_SListOfItemLocation +//purpose : +//======================================================================= + +inline TopLoc_SListOfItemLocation::TopLoc_SListOfItemLocation() +{} + +//======================================================================= +//function : TopLoc_SListOfItemLocation +//purpose : +//======================================================================= + +inline TopLoc_SListOfItemLocation::TopLoc_SListOfItemLocation(const TopLoc_SListOfItemLocation& Other) : + myNode(Other.myNode) +{ +} + //======================================================================= //function : IsEmpty //purpose : @@ -25,10 +43,17 @@ inline Standard_Boolean TopLoc_SListOfItemLocation::IsEmpty() const } //======================================================================= -//function : Construct +//function : Clear //purpose : //======================================================================= +inline void TopLoc_SListOfItemLocation::Clear() +{ + if (!myNode.IsNull()) { + myNode.Nullify(); + } +} + //======================================================================= //function : Construct //purpose : @@ -39,16 +64,6 @@ inline void TopLoc_SListOfItemLocation::Construct(const TopLoc_ItemLocation& anI Assign(TopLoc_SListOfItemLocation(anItem,*this)); } -//======================================================================= -//function : Constructed -//purpose : -//======================================================================= - -inline TopLoc_SListOfItemLocation TopLoc_SListOfItemLocation::Constructed(const TopLoc_ItemLocation& anItem) const -{ - return TopLoc_SListOfItemLocation(anItem,*this); -} - //======================================================================= //function : ToTail //purpose : @@ -59,16 +74,6 @@ inline void TopLoc_SListOfItemLocation::ToTail() Assign(Tail()); } -//======================================================================= -//function : Initialize -//purpose : -//======================================================================= - -inline void TopLoc_SListOfItemLocation::Initialize(const TopLoc_SListOfItemLocation& aList) -{ - Assign(aList); -} - //======================================================================= //function : More //purpose : diff --git a/src/gp/gp_Trsf.cdl b/src/gp/gp_Trsf.cdl index e5dd8e3475..6ea6f21c68 100644 --- a/src/gp/gp_Trsf.cdl +++ b/src/gp/gp_Trsf.cdl @@ -343,7 +343,7 @@ is Power (me : in out; N : Integer) raises ConstructionError is static; - Powered (me : in out; N : Integer) returns Trsf + Powered (me; N : Integer) returns Trsf ---C++: inline --- Purpose : -- Computes the following composition of transformations diff --git a/src/gp/gp_Trsf.lxx b/src/gp/gp_Trsf.lxx index 4082841dba..3c280b0db6 100644 --- a/src/gp/gp_Trsf.lxx +++ b/src/gp/gp_Trsf.lxx @@ -92,7 +92,7 @@ inline gp_Trsf gp_Trsf::Multiplied (const gp_Trsf& T) const return Tresult; } -inline gp_Trsf gp_Trsf::Powered (const Standard_Integer N) +inline gp_Trsf gp_Trsf::Powered (const Standard_Integer N) const { gp_Trsf T = *this; T.Power (N); diff --git a/tests/bugs/fclasses/bug25545 b/tests/bugs/fclasses/bug25545 new file mode 100644 index 0000000000..e743b23633 --- /dev/null +++ b/tests/bugs/fclasses/bug25545 @@ -0,0 +1,11 @@ +puts "============" +puts "OCC25545" +puts "============" +puts "" +####################################################################### +# TopLoc_Location::Transformation() provokes data races +####################################################################### + +pload QAcommands + +OCC25545