DiscussionsIssue archiveOCCT:Modeling Algorithms

Archived issue #0025938

BRepBuilderAPI_Transform is not thread safe

CommunityOCCT:Modeling Algorithmsclosed10 public notes

Search issues

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

Public activity

10 archived notes

Participants are labeled by their role within this record.

01Author
Wrong title: it should be: BRepBuilderAPI_Transform is not thread safe.
02Commenter 2
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?
03Commenter 3
I suggest simpler fix: make ModifiedShape() method to return shape by value, not by reference
04Commenter 4
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.
05Commenter 2
Branch [archived branch] has been created by Participant.

[revision removed]


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.
06Commenter 6
Please test CR25938.
07Commenter 2
Branch [archived branch] has been updated forcibly by Participant.

[revision removed]
08Commenter 8
Branch [archived branch] has been rebased on the current master
09Commenter 9
Dear Commenter 1,

Branch CR25938 from occt git-repository (and master from products git-repository) was compiled on Linux, MacOS and Windows platforms and tested.
[revision removed]

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%]
10Commenter 2
Branch [archived branch] has been deleted by Participant.

[revision removed]