From 37c6007b3e97e7714f33d58f509061cd37c7392e Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Thu, 9 Jul 2026 07:27:56 +0300 Subject: [PATCH] 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 --- src/ifcgeom/mapping/IfcCompositeCurve.cpp | 3 +++ src/ifcgeom/mapping/IfcEdge.cpp | 3 +++ src/ifcgeom/mapping/IfcFace.cpp | 10 +++------- 3 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/ifcgeom/mapping/IfcCompositeCurve.cpp b/src/ifcgeom/mapping/IfcCompositeCurve.cpp index e45d28600c..8bcece327e 100644 --- a/src/ifcgeom/mapping/IfcCompositeCurve.cpp +++ b/src/ifcgeom/mapping/IfcCompositeCurve.cpp @@ -53,6 +53,9 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcCompositeCurve* inst) { auto crv = map(segment->as()->ParentCurve()); if (crv) { if (!segment->as()->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(); } if (crv->kind() == taxonomy::EDGE) { diff --git a/src/ifcgeom/mapping/IfcEdge.cpp b/src/ifcgeom/mapping/IfcEdge.cpp index 67cb4a4995..0278762101 100644 --- a/src/ifcgeom/mapping/IfcEdge.cpp +++ b/src/ifcgeom/mapping/IfcEdge.cpp @@ -43,6 +43,9 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcEdge* inst) { auto basis = map(inst->as()->EdgeGeometry()); auto loop = taxonomy::dcast(basis); 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(); basis = loop->children[0]->basis; } diff --git a/src/ifcgeom/mapping/IfcFace.cpp b/src/ifcgeom/mapping/IfcFace.cpp index 60f08b3c88..23f1299629 100644 --- a/src/ifcgeom/mapping/IfcFace.cpp +++ b/src/ifcgeom/mapping/IfcFace.cpp @@ -26,18 +26,14 @@ taxonomy::ptr mapping::map_impl(const IfcSchema::IfcFace* inst) { auto bounds = inst->Bounds(); for (auto& bound : *bounds) { if (auto r = taxonomy::cast(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()) { r->reverse(); } // @todo check why loop sets external to true initially 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); } }