mirror of
https://github.com/IfcOpenShell/IfcOpenShell.git
synced 2026-09-20 06:58:56 +00:00
Fix #8050: clone mapped items before in place mutation in geometry mapping
The mapping cache returns the same shared taxonomy item on a hit, so mutating a map() result corrupts the cached item that other representations reuse. Under multithreading this produced nondeterministic geometry. Clone before mutating at every site that writes into a map() result, matching the existing idiom in IfcOrientedEdge, IfcDerivedProfileDef and IfcArbitraryOpenProfileDef. This fixes the race at the mutation site and keeps the mapping cache enabled. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
committed by
Thomas Krijnen
parent
2ef49d6311
commit
37c6007b3e
@@ -53,6 +53,9 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcCompositeCurve* inst) {
|
|||||||
auto crv = map(segment->as<IfcSchema::IfcCompositeCurveSegment>()->ParentCurve());
|
auto crv = map(segment->as<IfcSchema::IfcCompositeCurveSegment>()->ParentCurve());
|
||||||
if (crv) {
|
if (crv) {
|
||||||
if (!segment->as<IfcSchema::IfcCompositeCurveSegment>()->SameSense()) {
|
if (!segment->as<IfcSchema::IfcCompositeCurveSegment>()->SameSense()) {
|
||||||
|
// Clone before reversing so we do not corrupt the shared cached
|
||||||
|
// curve that map() may return for other segments.
|
||||||
|
crv.reset(crv->clone_());
|
||||||
crv->reverse();
|
crv->reverse();
|
||||||
}
|
}
|
||||||
if (crv->kind() == taxonomy::EDGE) {
|
if (crv->kind() == taxonomy::EDGE) {
|
||||||
|
|||||||
@@ -43,6 +43,9 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcEdge* inst) {
|
|||||||
auto basis = map(inst->as<IfcSchema::IfcEdgeCurve>()->EdgeGeometry());
|
auto basis = map(inst->as<IfcSchema::IfcEdgeCurve>()->EdgeGeometry());
|
||||||
auto loop = taxonomy::dcast<taxonomy::loop>(basis);
|
auto loop = taxonomy::dcast<taxonomy::loop>(basis);
|
||||||
if (loop && loop->children.size() == 1) {
|
if (loop && loop->children.size() == 1) {
|
||||||
|
// Clone before mutating so calculate_linear_edge_curves does not write
|
||||||
|
// into the child edges of the shared cached loop returned by map().
|
||||||
|
loop.reset(loop->clone_());
|
||||||
loop->calculate_linear_edge_curves();
|
loop->calculate_linear_edge_curves();
|
||||||
basis = loop->children[0]->basis;
|
basis = loop->children[0]->basis;
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -26,18 +26,14 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcFace* inst) {
|
|||||||
auto bounds = inst->Bounds();
|
auto bounds = inst->Bounds();
|
||||||
for (auto& bound : *bounds) {
|
for (auto& bound : *bounds) {
|
||||||
if (auto r = taxonomy::cast<taxonomy::loop>(map(bound->Bound()))) {
|
if (auto r = taxonomy::cast<taxonomy::loop>(map(bound->Bound()))) {
|
||||||
|
// Clone before mutating so we do not corrupt the shared cached loop
|
||||||
|
// that map() may return for other faces referencing the same bound.
|
||||||
|
r.reset(r->clone_());
|
||||||
if (!bound->Orientation()) {
|
if (!bound->Orientation()) {
|
||||||
r->reverse();
|
r->reverse();
|
||||||
}
|
}
|
||||||
// @todo check why loop sets external to true initially
|
// @todo check why loop sets external to true initially
|
||||||
r->external = bound->declaration().is(IfcSchema::IfcFaceOuterBound::Class());
|
r->external = bound->declaration().is(IfcSchema::IfcFaceOuterBound::Class());
|
||||||
/*
|
|
||||||
// Make a copy in case we need immutability later for e.g. caching
|
|
||||||
auto s = r->clone();
|
|
||||||
((taxonomy::loop*)s)->external = true;
|
|
||||||
delete r;
|
|
||||||
r = s;
|
|
||||||
*/
|
|
||||||
face->children.push_back(r);
|
face->children.push_back(r);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user