From ba8dc537180329d93f549e150b0c935d38683af2 Mon Sep 17 00:00:00 2001 From: Thomas Krijnen Date: Tue, 28 Jul 2026 04:19:20 +0200 Subject: [PATCH] Fix streamer header ownership Create file-owned headers after storage is selected but before streaming starts. Let owner-backed streamers use that header directly, and keep owned_header_ exclusively for ownerless streamers. Generated with the assistance of an AI coding tool. --- src/ifcparse/file.h | 3 +-- src/ifcparse/parse.cpp | 58 ++++++++++++++++++++++-------------------- 2 files changed, 31 insertions(+), 30 deletions(-) diff --git a/src/ifcparse/file.h b/src/ifcparse/file.h index 106afecbd2..f40999f643 100644 --- a/src/ifcparse/file.h +++ b/src/ifcparse/file.h @@ -97,7 +97,6 @@ private: Reader* stream_; std::unique_ptr> lexer_; std::unique_ptr owned_header_; - spf_header* header_; ifcopenshell::file* owner_; boost::circular_buffer token_stream_; const ifcopenshell::schema_definition* schema_; @@ -171,7 +170,7 @@ private: const ifcopenshell::schema_definition* schema() const { return schema_; } - const spf_header* header() const { return header_; } + const spf_header* header() const; ~instance_streamer() = default; diff --git a/src/ifcparse/parse.cpp b/src/ifcparse/parse.cpp index 00b7bd4787..c36239efef 100644 --- a/src/ifcparse/parse.cpp +++ b/src/ifcparse/parse.cpp @@ -1666,10 +1666,12 @@ bool ifcopenshell::file::initialize(const std::string& fn, bool mmap) { if (mmap) { file_reader s(fn); storage_.emplace<1>(this, logger_.get()); + header_.reset(new spf_header(this, &logger_.get())); std::get(storage_).read_from_stream(&s, schema_, max_id_, types_to_bypass_loading_); } else { file_reader s(fn); storage_.emplace<1>(this, logger_.get()); + header_.reset(new spf_header(this, &logger_.get())); std::get(storage_).read_from_stream(&s, schema_, max_id_, types_to_bypass_loading_); } @@ -1681,7 +1683,6 @@ bool ifcopenshell::file::initialize(const std::string& fn, bool mmap) { } ifcroot_type_ = schema_ ? schema_->declaration_by_name("IfcRoot") : nullptr; - header_.reset(new spf_header(this, &logger_.get())); return good_ == file_open_status::SUCCESS; } #endif @@ -1696,6 +1697,7 @@ bool ifcopenshell::file::initialize(const std::string& path, filetype ty, bool r if (ty == FT_IFCSPF) { file_reader s(path); storage_.emplace<1>(this, logger_.get()); + header_.reset(new spf_header(this, &logger_.get())); std::get(storage_).read_from_stream(&s, schema_, max_id_, types_to_bypass_loading_); if ((good_ = std::get(storage_).good_)) { @@ -1732,7 +1734,9 @@ bool ifcopenshell::file::initialize(const std::string& path, filetype ty, bool r // throw std::runtime_error("Unsupported file format"); } ifcroot_type_ = schema_ ? schema_->declaration_by_name("IfcRoot") : nullptr; - header_.reset(new spf_header(this, &logger_.get())); + if (!header_) { + header_.reset(new spf_header(this, &logger_.get())); + } return good_ == file_open_status::SUCCESS; } @@ -1763,6 +1767,7 @@ file::file(std::istream& stream, int length, ::logger& log) s.push_next_page(string_data); storage_.emplace<1>(this, logger_.get()); + header_.reset(new spf_header(this, &logger_.get())); std::get(storage_).read_from_stream(&s, schema_, max_id_, types_to_bypass_loading_); good_ = std::get(storage_).good_; ifcroot_type_ = schema_ ? schema_->declaration_by_name("IfcRoot") : nullptr; @@ -1771,7 +1776,6 @@ file::file(std::istream& stream, int length, ::logger& log) byref_excl_ = decltype(byref_excl_)(&std::get(storage_).byref_excl_); byguid_ = decltype(byguid_)(&std::get(storage_).byguid_); - header_.reset(new spf_header(this, &logger_.get())); } file::file(void* data, int length, ::logger& log) @@ -1783,6 +1787,7 @@ file::file(void* data, int length, ::logger& log) file_reader s(std::string((char*)data, length), caller_fed_tag{}); storage_.emplace<1>(this, logger_.get()); + header_.reset(new spf_header(this, &logger_.get())); std::get(storage_).read_from_stream(&s, schema_, max_id_, types_to_bypass_loading_); good_ = std::get(storage_).good_; ifcroot_type_ = schema_ ? schema_->declaration_by_name("IfcRoot") : nullptr; @@ -1791,7 +1796,6 @@ file::file(void* data, int length, ::logger& log) byref_excl_ = decltype(byref_excl_)(&std::get(storage_).byref_excl_); byguid_ = decltype(byguid_)(&std::get(storage_).byguid_); - header_.reset(new spf_header(this, &logger_.get())); } file::file(const ifcopenshell::schema_definition* schema, filetype ty, const std::string& path, ::logger& log) @@ -1901,21 +1905,22 @@ bool try_parse_header( } // namespace +template +const spf_header* ifcopenshell::instance_streamer::header() const { + return owner_ ? &owner_->header() : owned_header_.get(); +} + template spf_header& ifcopenshell::instance_streamer::ensure_header() { - if (header_) { - return *header_; - } - if (owner_ != nullptr) { - header_ = &owner_->header(); - header_->owner_file(owner_); - } else { - owned_header_ = std::make_unique(owner_, &logger_.get()); - header_ = owned_header_.get(); + return owner_->header(); } - return *header_; + if (!owned_header_) { + owned_header_ = std::make_unique(owner_, &logger_.get()); + } + + return *owned_header_; } template @@ -2012,7 +2017,6 @@ void ifcopenshell::instance_streamer::push_page(const std::string& page) template ifcopenshell::instance_streamer::instance_streamer(ifcopenshell::file* f, ::logger& log) : stream_(nullptr) - , header_(nullptr) , owner_(f) , token_stream_(3, token{}) , schema_(nullptr) @@ -2038,7 +2042,6 @@ ifcopenshell::instance_streamer::instance_streamer(ifcopenshell::file* f template ifcopenshell::instance_streamer::instance_streamer(const std::string& fn, bool mmap, ifcopenshell::file* f, ::logger& log) : stream_(nullptr) - , header_(nullptr) , owner_(f) , token_stream_(3, token{}) , schema_(nullptr) @@ -2067,7 +2070,6 @@ ifcopenshell::instance_streamer::instance_streamer(const std::string& fn template ifcopenshell::instance_streamer::instance_streamer(void* data, int length, ifcopenshell::file* f, ::logger& log) : stream_(nullptr) - , header_(nullptr) , owner_(f) , token_stream_(3, token{}) , schema_(nullptr) @@ -2092,7 +2094,6 @@ ifcopenshell::instance_streamer::instance_streamer(void* data, int lengt template ifcopenshell::instance_streamer::instance_streamer(Reader* stream, ifcopenshell::file* f, ::logger& log) : stream_(stream) - , header_(nullptr) , owner_(f) , token_stream_(3, token{}) , schema_(nullptr) @@ -2120,33 +2121,34 @@ template std::optional> ifcopenshell::instance_streamer::read_instance() { std::optional> return_value; - if (yield_header_instances_ && header_ && yielded_header_instances_ < 3) { + const auto* header = this->header(); + if (yield_header_instances_ && header && yielded_header_instances_ < 3) { if (yielded_header_instances_ == 0) { return_value.emplace( 0, - &header_->file_description().declaration(), + &header->file_description().declaration(), #ifdef IFOPSH_SAFE_INSTANCE - header_->file_description().data_weak().lock()); + header->file_description().data_weak().lock()); #else - header_->file_description().data_weak()); + header->file_description().data_weak()); #endif } else if (yielded_header_instances_ == 1) { return_value.emplace( 0, - &header_->file_name().declaration(), + &header->file_name().declaration(), #ifdef IFOPSH_SAFE_INSTANCE - header_->file_name().data_weak().lock()); + header->file_name().data_weak().lock()); #else - header_->file_name().data_weak()); + header->file_name().data_weak()); #endif } else if (yielded_header_instances_ == 2) { return_value.emplace( 0, - &header_->file_schema().declaration(), + &header->file_schema().declaration(), #ifdef IFOPSH_SAFE_INSTANCE - header_->file_schema().data_weak().lock()); + header->file_schema().data_weak().lock()); #else - header_->file_schema().data_weak()); + header->file_schema().data_weak()); #endif } yielded_header_instances_ += 1;