From 4776bd763921d19e35df001c3f0f1539927a190d Mon Sep 17 00:00:00 2001 From: Petru Conduraru Date: Tue, 7 Jul 2026 12:49:08 +0300 Subject: [PATCH] Atomic IFC file writes to prevent corruption on interrupted save (#4797) file.write() streamed directly onto the target path, so a crash mid-write left a truncated file with dangling STEP references. Serialize to a temp file in the same directory, then atomically rename it onto the target. - New IfcUtil::path::atomic_rename_file: std::rename on POSIX, MoveFileExW with MOVEFILE_REPLACE_EXISTING on Windows. Unlike rename_file it never unlinks the destination first, so there is no window where it goes missing. - Fully in C++/swig (per aothms), so the FILE_NAME header is untouched: it comes from the model header, not the output path (verified empirically). - Temp lives next to the target so the rename stays on one filesystem. - Stream is closed before the rename (Windows cannot move an open file). - On any write error the temp is removed and the original target is intact. Co-Authored-By: Claude Opus 4.8 --- src/ifcparse/IfcUtil.cpp | 17 ++++++++++++ src/ifcparse/utils.h | 7 +++++ src/ifcwrap/IfcParseWrapper.i | 50 +++++++++++++++++++++++++++++++---- 3 files changed, 69 insertions(+), 5 deletions(-) diff --git a/src/ifcparse/IfcUtil.cpp b/src/ifcparse/IfcUtil.cpp index a47083262f..efb5426021 100644 --- a/src/ifcparse/IfcUtil.cpp +++ b/src/ifcparse/IfcUtil.cpp @@ -353,6 +353,17 @@ IFC_PARSE_API bool IfcUtil::path::rename_file(const std::string& old_filename, c return success; } +IFC_PARSE_API bool IfcUtil::path::atomic_rename_file(const std::string& old_filename, const std::string& new_filename) { + std::wstring old_filename_w = from_utf8(old_filename); + std::wstring new_filename_w = from_utf8(new_filename); + // MOVEFILE_REPLACE_EXISTING makes the replace atomic on NTFS (no unlink + // of the destination first). MOVEFILE_WRITE_THROUGH waits until the move + // is flushed to disk before returning. + const bool success = !!MoveFileExW(old_filename_w.c_str(), new_filename_w.c_str(), + MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH); + return success; +} + IFC_PARSE_API bool IfcUtil::path::delete_file(const std::string& filename) { std::wstring filename_w = from_utf8(filename); const bool success = !!DeleteFileW(filename_w.c_str()); @@ -368,6 +379,12 @@ IFC_PARSE_API bool IfcUtil::path::rename_file(const std::string& old_filename, c return std::rename(old_filename.c_str(), new_filename.c_str()) == 0; } +IFC_PARSE_API bool IfcUtil::path::atomic_rename_file(const std::string& old_filename, const std::string& new_filename) { + // POSIX rename() atomically replaces an existing destination on the same + // filesystem, so there is no window in which new_filename is missing. + return std::rename(old_filename.c_str(), new_filename.c_str()) == 0; +} + IFC_PARSE_API bool IfcUtil::path::delete_file(const std::string& filename) { return std::remove(filename.c_str()) != 0; } diff --git a/src/ifcparse/utils.h b/src/ifcparse/utils.h index e1623a8cdb..d9e7eca621 100644 --- a/src/ifcparse/utils.h +++ b/src/ifcparse/utils.h @@ -37,6 +37,13 @@ namespace path { IFC_PARSE_API bool delete_file(const std::string& filename); IFC_PARSE_API bool rename_file(const std::string& old_filename, const std::string& new_filename); +/// Atomically renames old_filename onto new_filename, replacing an existing +/// destination in a single filesystem operation. Unlike rename_file(), the +/// destination is never unlinked before the rename, so an interruption can +/// never leave the destination missing. This requires both paths to live on +/// the same filesystem. Returns true on success. +IFC_PARSE_API bool atomic_rename_file(const std::string& old_filename, const std::string& new_filename); + #if defined(_MSC_VER) && defined(_UNICODE) /// Uses windows.h string conversion functions diff --git a/src/ifcwrap/IfcParseWrapper.i b/src/ifcwrap/IfcParseWrapper.i index 9826c03da2..10f9d5af49 100644 --- a/src/ifcwrap/IfcParseWrapper.i +++ b/src/ifcwrap/IfcParseWrapper.i @@ -117,10 +117,51 @@ PyObject* get_feature(const std::string& x) { %{ +#include +#include + static const std::string& helper_fn_declaration_get_name(const IfcParse::declaration* decl) { return decl->name(); } +// Atomic IFC/STEP write (issue #4797): serialize to a temporary file next to +// the destination, then atomically rename it onto the destination. If the +// process is interrupted mid-write, the destination is never truncated or +// left with dangling STEP references; at most a stray temp file remains, which +// the caller can safely ignore. Keeping the temp in the same directory means +// the rename stays on a single filesystem and is therefore atomic. The temp +// path never leaks into the FILE_NAME header, which is derived from the model +// header, not the output path. +template +static void helper_fn_atomic_write(T& file_obj, const std::string& fn) { + std::random_device rd; + const std::string temp_fn = fn + "." + std::to_string(rd()) + ".tmp"; + { + // Same open mode as a plain write so the bytes are identical. + std::ofstream f(IfcUtil::path::from_utf8(temp_fn).c_str()); + if (!f.good()) { + // The temp file could not be created (e.g. directory not + // writable). Nothing was touched; report as a normal write error. + throw std::runtime_error("Failed to write to path: '" + fn + "', check folder and file permissions."); + } + f << file_obj; + f.flush(); + if (!f.good()) { + // Serialization failed (e.g. disk full). Clean up the partial temp + // and abort. The existing destination is left intact. + f.close(); + IfcUtil::path::delete_file(temp_fn); + throw std::runtime_error("Failed to write to path: '" + fn + "', the file may be incomplete."); + } + // The ofstream destructor at the end of this scope closes the stream. + // On Windows the file must be closed before it can be renamed. + } + if (!IfcUtil::path::atomic_rename_file(temp_fn, fn)) { + IfcUtil::path::delete_file(temp_fn); + throw std::runtime_error("Failed to write to path: '" + fn + "', could not replace the existing file."); + } +} + static IfcUtil::ArgumentType helper_fn_attribute_type(const IfcUtil::IfcBaseClass* inst, unsigned i) { const IfcParse::parameter_type* pt = 0; if (inst->declaration().as_entity()) { @@ -219,11 +260,10 @@ private: } void write(const std::string& fn) { - std::ofstream f(IfcUtil::path::from_utf8(fn).c_str()); - if (!f.good()) { - throw std::runtime_error("Failed to write to path: '" + fn + "', check folder and file permissions."); - } - f << (*$self); + // Atomic write: serialize to a temp file next to the target, then + // atomically rename it into place, so an interrupted write can never + // corrupt the destination (issue #4797). + helper_fn_atomic_write(*$self, fn); } std::string to_string() {