View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0025938 | Community | OCCT:Modeling Algorithms | public | 2015-03-14 20:16 | 2015-05-14 16:31 |
Reporter | Istvan Csanady | Assigned To | bugmaster | ||
Priority | normal | Severity | minor | ||
Status | closed | Resolution | fixed | ||
Target Version | 6.9.0 | Fixed in Version | 6.9.0 | ||
Summary | 0025938: BRepBuilderAPI_Transform is not thread safe | ||||
Description | The ModifiedShape method uses a static variable to return a reference to a shape, this makes it not thread safe. The quick (and dirty) fix is to create a mutable myModified member, and use it instead of the static SM variable, but I could not figure out, how to make a member variable mutable in CDL, this is why I did not provide a patch. | ||||
Steps To Reproduce | Not needed | ||||
Tags | No tags attached. | ||||
Test case number | Not needed | ||||
|
Wrong title: it should be: BRepBuilderAPI_Transform is not thread safe. |
|
It is not correct to use one shape myModified to return on each request via ModifiedShape(). It can lead to unexpected behavior for the caller, as the shape returned earlier by ModifiedShape changes after the next call to this method with another argument. Of course, this was always incorrect, but if we are to change this source code we can make it perfect. The most safe way is to add the DataMap as the field of the class, and each time ModifiedShape is called to use this map as a cache. Andrey, do you agree with my proposal? |
|
I suggest simpler fix: make ModifiedShape() method to return shape by value, not by reference |
|
I am afraid if we make the method return shape by value it may require modification a ton of code that declares a local const reference to the returned by this method shape. |
|
Branch CR25938 has been created by msv. SHA-1: a6c42e4ab76c78e702e04dc2386c6da027835047 Detailed log of new commits: Author: msv Date: Fri Apr 10 15:43:21 2015 +0300 0025938: BRepBuilderAPI_Transform is not thread safe ModifiedShape() method is made returning shape by value, not by reference. |
|
Please test CR25938. |
|
Branch CR25938 has been updated forcibly by apv. SHA-1: b6c62b2e055d02bff65ab2ba14dff30c1d0049c9 |
|
Branch CR25938 has been rebased on the current master |
|
Dear BugMaster, Branch CR25938 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested. SHA-1: b6c62b2e055d02bff65ab2ba14dff30c1d0049c9 Number of compiler warnings: occt component: Linux: 18 (18 on master) Windows: 0 (0 on master) products component: Linux: 4 (4 on master) Windows: 0 (0 on master) Regressions/Differences: Not detected Testing cases: Not needed Testing on Linux: Total MEMORY difference: 94092230 / 94337269 [-0.26%] Total CPU difference: 50763.299999999144 / 56193.089999999705 [-9.66%] Testing on Windows: Total MEMORY difference: 57129863 / 57127628 [+0.00%] Total CPU difference: 16015.686663999033 / 15976.577213298899 [+0.24%] |
|
Branch CR25938 has been deleted by inv. SHA-1: b6c62b2e055d02bff65ab2ba14dff30c1d0049c9 |
occt: master 346cf025 2015-04-10 12:43:21
Committer: bugmaster Details Diff |
0025938: BRepBuilderAPI_Transform is not thread safe ModifiedShape() method is made returning shape by value, not by reference. |
Affected Issues 0025938 |
|
mod - src/BRepBuilderAPI/BRepBuilderAPI_GTransform.cdl | Diff File | ||
mod - src/BRepBuilderAPI/BRepBuilderAPI_GTransform.cxx | Diff File | ||
mod - src/BRepBuilderAPI/BRepBuilderAPI_ModifyShape.cdl | Diff File | ||
mod - src/BRepBuilderAPI/BRepBuilderAPI_ModifyShape.cxx | Diff File | ||
mod - src/BRepBuilderAPI/BRepBuilderAPI_Transform.cdl | Diff File | ||
mod - src/BRepBuilderAPI/BRepBuilderAPI_Transform.cxx | Diff File | ||
mod - src/QABugs/QABugs_19.cxx | Diff File | ||
mod - src/TNaming/TNaming.cxx | Diff File |
Date Modified | Username | Field | Change |
---|---|---|---|
2015-03-14 20:16 | Istvan Csanady | New Issue | |
2015-03-14 20:16 | Istvan Csanady | Assigned To | => msv |
2015-03-14 20:17 | Istvan Csanady | Note Added: 0038367 | |
2015-03-14 20:37 |
|
Summary | BRepBuilderAPI_Transform is using unnecessary static variables => BRepBuilderAPI_Transform is not thread safe |
2015-03-14 21:05 |
|
Note Added: 0038370 | |
2015-03-14 21:05 |
|
Assigned To | msv => abv |
2015-03-14 21:05 |
|
Status | new => feedback |
2015-03-15 21:15 |
|
Note Added: 0038374 | |
2015-03-15 21:15 |
|
Assigned To | abv => msv |
2015-03-15 23:08 |
|
Note Added: 0038376 | |
2015-03-15 23:08 |
|
Assigned To | msv => abv |
2015-04-10 15:43 | git | Note Added: 0039586 | |
2015-04-10 15:44 |
|
Assigned To | abv => msv |
2015-04-10 15:44 |
|
Status | feedback => resolved |
2015-04-10 15:44 |
|
Steps to Reproduce Updated | |
2015-04-10 15:44 |
|
Note Added: 0039587 | |
2015-04-10 15:44 |
|
Assigned To | msv => bugmaster |
2015-04-10 15:44 |
|
Status | resolved => reviewed |
2015-04-10 15:50 |
|
Assigned To | bugmaster => apv |
2015-04-10 17:23 | git | Note Added: 0039600 | |
2015-04-10 17:24 |
|
Note Added: 0039601 | |
2015-04-13 17:29 |
|
Test case number | => Not needed |
2015-04-13 17:31 |
|
Note Added: 0039645 | |
2015-04-13 17:31 |
|
Assigned To | apv => bugmaster |
2015-04-13 17:31 |
|
Status | reviewed => tested |
2015-04-17 15:40 | bugmaster | Changeset attached | => occt master 346cf025 |
2015-04-17 15:40 | bugmaster | Status | tested => verified |
2015-04-17 15:40 | bugmaster | Resolution | open => fixed |
2015-05-14 15:28 |
|
Status | verified => closed |
2015-05-14 15:31 |
|
Fixed in Version | => 6.9.0 |
2015-05-14 16:31 | git | Note Added: 0041031 |