From 824c1fc280c4c9a4c95a6be2dae08bc13ba9e462 Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Fri, 17 Jul 2026 10:13:29 +0300 Subject: [PATCH] ifcwrap: keep geometry's owning element alive to fix silent data corruption (#1124) create_shape() returns a Python-owned Element (SWIG_POINTER_OWN in the boost::variant out typemap). Its .geometry property calls Element::geometry(), which returns a reference into the element's boost::shared_ptr _geometry member. SWIG wraps that reference as a non-owning pointer, so the returned Triangulation/BRep/Serialization proxy does not keep the element alive. When a caller keeps only .geometry (e.g. create_shape(s, e).geometry) and drops the parent element, Python garbage-collects the element, destroying its shared_ptr and freeing the underlying representation. Subsequent reads of verts/faces then return freed memory: empty or implausible float/int garbage, non-deterministically depending on GC and allocator timing. This is silent data corruption, not a crash, and has bitten users since 2020. Fix: in the TriangulationElement/SerializedElement/BRepElement pythoncode, wrap the geometry getter so the returned geometry stores a backreference to its owning element (result._parent = self). This makes the parent's lifetime at least as long as the geometry's, automatically and transparently, so no caller has to remember to hold the element. This is aothms's suggested backreference, applied generically in the binding rather than left as a workaround. Reproduced deterministically (washBasin fixture): before, verts len 0 vs 133500 across repeated GC-pressure runs; after, 133500 every run for all three element types. test_create_shape passes; no regressions. Note: tree.select_ray()'s ray_intersection_result (2024 follow-up in #1124) is a separate ownership mechanism (std::vector element reference + std::array member pointer) and is left as follow-up scope. Co-Authored-By: Claude Sonnet 5 --- .../ifcopenshell/geom/main.py | 4 ++-- src/ifcwrap/IfcGeomWrapper.i | 23 +++++++++++++++---- 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/src/ifcopenshell-python/ifcopenshell/geom/main.py b/src/ifcopenshell-python/ifcopenshell/geom/main.py index 9cf79934aa..c64d262e6f 100644 --- a/src/ifcopenshell-python/ifcopenshell/geom/main.py +++ b/src/ifcopenshell-python/ifcopenshell/geom/main.py @@ -482,8 +482,8 @@ def create_shape( """ Returns a geometric interpretation of the IFC entity instance - Note that in Python, you must store a reference to the element returned by this function to prevent garbage - collection when you access its children. See #1124. + The returned element's ``geometry`` keeps a reference to its owning element, so accessing children + (e.g. ``create_shape(...).geometry.verts``) no longer requires holding onto the element. See #1124. :raises RuntimeError: If failed to process shape. You can turn detailed logging to get more details. diff --git a/src/ifcwrap/IfcGeomWrapper.i b/src/ifcwrap/IfcGeomWrapper.i index 10c9b02395..b6dffb3b6e 100644 --- a/src/ifcwrap/IfcGeomWrapper.i +++ b/src/ifcwrap/IfcGeomWrapper.i @@ -793,14 +793,24 @@ struct ShapeRTTI : public boost::static_visitor %extend IfcGeom::TriangulationElement { %pythoncode %{ # Hide the getters with read-only property implementations - geometry = property(geometry) + # Keep the owning element alive while its geometry is referenced (#1124). + def _geometry_with_backref(self, _f=geometry): + result = _f(self) + result._parent = self + return result + geometry = property(_geometry_with_backref) %} }; %extend IfcGeom::SerializedElement { %pythoncode %{ # Hide the getters with read-only property implementations - geometry = property(geometry) + # Keep the owning element alive while its geometry is referenced (#1124). + def _geometry_with_backref(self, _f=geometry): + result = _f(self) + result._parent = self + return result + geometry = property(_geometry_with_backref) %} }; @@ -825,10 +835,15 @@ struct ShapeRTTI : public boost::static_visitor %pythoncode %{ # Hide the getters with read-only property implementations - geometry = property(geometry) + # Keep the owning element alive while its geometry is referenced (#1124). + def _geometry_with_backref(self, _f=geometry): + result = _f(self) + result._parent = self + return result + geometry = property(_geometry_with_backref) volume = property(calc_volume_) surface_area = property(calc_surface_area_) - %} + %} }; /*