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<Representation>
_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 <noreply@anthropic.com>
This commit is contained in:
Petru Conduraru
2026-07-17 10:13:29 +03:00
committed by Thomas Krijnen
parent 705af7ba3a
commit 824c1fc280
2 changed files with 21 additions and 6 deletions
@@ -482,8 +482,8 @@ def create_shape(
""" """
Returns a geometric interpretation of the IFC entity instance 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 The returned element's ``geometry`` keeps a reference to its owning element, so accessing children
collection when you access its children. See #1124. (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. :raises RuntimeError: If failed to process shape. You can turn detailed logging to get more details.
+19 -4
View File
@@ -793,14 +793,24 @@ struct ShapeRTTI : public boost::static_visitor<PyObject*>
%extend IfcGeom::TriangulationElement { %extend IfcGeom::TriangulationElement {
%pythoncode %{ %pythoncode %{
# Hide the getters with read-only property implementations # 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 { %extend IfcGeom::SerializedElement {
%pythoncode %{ %pythoncode %{
# Hide the getters with read-only property implementations # 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<PyObject*>
%pythoncode %{ %pythoncode %{
# Hide the getters with read-only property implementations # 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_) volume = property(calc_volume_)
surface_area = property(calc_surface_area_) surface_area = property(calc_surface_area_)
%} %}
}; };
/* /*