From e52af2baad4098f8d53889cb93f9687fec9916cc Mon Sep 17 00:00:00 2001 From: Andrej730 Date: Thu, 3 Jul 2025 18:05:59 +0500 Subject: [PATCH] Iterator.validate_iterator_state to avoid crashes using next() and get() Just to avoid accidental crashes using .get() / .next() when using them from Python - in cases when iterator was initialized but there were no elements or if iterator got exhausted. --- src/ifcgeom/Iterator.cpp | 33 ++++++++++++++++++++++++++------- src/ifcgeom/Iterator.h | 2 ++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/ifcgeom/Iterator.cpp b/src/ifcgeom/Iterator.cpp index 7db1f412b9..3ad9e920b0 100644 --- a/src/ifcgeom/Iterator.cpp +++ b/src/ifcgeom/Iterator.cpp @@ -1,6 +1,10 @@ #include "Iterator.h" /** +* Initialize iterator's list of tasks. +* +* Will automatically process first element, if 'defer-processing-first-element' is not set to `true`. +* * @return Returns true if the iterator is initialized with any elements, false otherwise. * * @note @@ -462,10 +466,29 @@ void IfcGeom::Iterator::log_timepoints() const { } } +void IfcGeom::Iterator::validate_iterator_state() const { + if (!initialization_outcome_) { + throw std::runtime_error("Iterator not initialized"); + } + + // Causes: + // - iterator was initialized but there were no elements to process + // - iterator was initialized but 'defer-processing-first-element' setting is enabled + // and some element should be processed manually first + if (!task_result_ptr_initialized) { + throw std::runtime_error("No elements processed"); + } + + if (task_result_ptr_exhausted) { + throw std::runtime_error("Iterator is exhausted"); + } +} + /// Moves to the next shape representation, create its geometry, and returns the associated product. /// Use get() to retrieve the created geometry. const IfcUtil::IfcBaseClass* IfcGeom::Iterator::next() { using std::chrono::high_resolution_clock; + validate_iterator_state(); if (*native_task_result_iterator_ != *task_result_iterator_) { delete* native_task_result_iterator_; @@ -477,6 +500,7 @@ const IfcUtil::IfcBaseClass* IfcGeom::Iterator::next() { Logger::SetProduct(boost::none); time_points[3] = high_resolution_clock::now(); log_timepoints(); + task_result_ptr_exhausted = true; return nullptr; } @@ -492,6 +516,7 @@ const IfcUtil::IfcBaseClass* IfcGeom::Iterator::next() { Logger::SetProduct(boost::none); time_points[3] = high_resolution_clock::now(); log_timepoints(); + task_result_ptr_exhausted = true; return nullptr; } } @@ -506,13 +531,7 @@ const IfcUtil::IfcBaseClass* IfcGeom::Iterator::next() { /// Gets the representation of the current geometrical entity. IfcGeom::Element* IfcGeom::Iterator::get() { - if (!initialization_outcome_) { - throw std::runtime_error("Iterator not initialized"); - } - - if (settings_.get().get() && !task_result_ptr_initialized) { - throw std::runtime_error("No elements processed"); - } + validate_iterator_state(); auto ret = *task_result_iterator_; diff --git a/src/ifcgeom/Iterator.h b/src/ifcgeom/Iterator.h index 59f9cbf708..9cec31bd47 100644 --- a/src/ifcgeom/Iterator.h +++ b/src/ifcgeom/Iterator.h @@ -122,6 +122,7 @@ namespace IfcGeom { std::mutex element_ready_mutex_; bool task_result_ptr_initialized = false; + bool task_result_ptr_exhausted = false; size_t async_elements_returned_ = 0; ifcopenshell::geometry::Settings settings_; @@ -207,6 +208,7 @@ namespace IfcGeom { bool wait_for_element(); void log_timepoints() const; + void validate_iterator_state() const; ifcopenshell::geometry::taxonomy::direction3::ptr remove_offset_(); public: