From 82e1e33ddd44ab9360057f22ba5df52d90ff2c3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 2 Apr 2026 23:10:35 +0200 Subject: [PATCH 1/7] Add failing test --- CMakeLists.txt | 1 + test/CoreTest.cpp | 5 + test/Files_Core/CoreTests.hpp | 5 + .../Files_Core/read_nonexistent_attribute.cpp | 106 ++++++++++++++++++ 4 files changed, 117 insertions(+) create mode 100644 test/Files_Core/read_nonexistent_attribute.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 1ada05698a..f2dcc98f82 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -826,6 +826,7 @@ if(openPMD_BUILD_TESTING) elseif(${test_name} STREQUAL "Core") list(APPEND ${out_list} test/Files_Core/automatic_variable_encoding.cpp + test/Files_Core/read_nonexistent_attribute.cpp ) endif() endmacro() diff --git a/test/CoreTest.cpp b/test/CoreTest.cpp index ee266ff56e..8a2be5f27c 100644 --- a/test/CoreTest.cpp +++ b/test/CoreTest.cpp @@ -1750,6 +1750,11 @@ TEST_CASE("automatic_variable_encoding", "[adios2]") automatic_variable_encoding::automatic_variable_encoding(); } +TEST_CASE("read_nonexistent_attribute", "[core]") +{ + read_nonexistent_attribute::read_nonexistent_attribute(); +} + TEST_CASE("unique_ptr", "[core]") { auto stdptr = std::make_unique(5); diff --git a/test/Files_Core/CoreTests.hpp b/test/Files_Core/CoreTests.hpp index fa62279c82..f769beeb99 100644 --- a/test/Files_Core/CoreTests.hpp +++ b/test/Files_Core/CoreTests.hpp @@ -24,3 +24,8 @@ namespace automatic_variable_encoding { auto automatic_variable_encoding() -> void; } + +namespace read_nonexistent_attribute +{ +auto read_nonexistent_attribute() -> void; +} diff --git a/test/Files_Core/read_nonexistent_attribute.cpp b/test/Files_Core/read_nonexistent_attribute.cpp new file mode 100644 index 0000000000..08d584d0fc --- /dev/null +++ b/test/Files_Core/read_nonexistent_attribute.cpp @@ -0,0 +1,106 @@ + +/* Copyright 2026 Franz Poeschel + * + * This file is part of openPMD-api. + * + * openPMD-api is free software: you can redistribute it and/or modify + * it under the terms of of either the GNU General Public License or + * the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * openPMD-api is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License and the GNU Lesser General Public License + * for more details. + * + * You should have received a copy of the GNU General Public License + * and the GNU Lesser General Public License along with openPMD-api. + * If not, see . + */ + +#include + +#if !openPMD_USE_INVASIVE_TESTS + +namespace read_nonexistent_attribute +{ +void read_nonexistent_attribute() +{} +} // namespace read_nonexistent_attribute + +#else + +#define OPENPMD_private public: +#define OPENPMD_protected public: + +#include "CoreTests.hpp" + +#include "openPMD/Error.hpp" +#include "openPMD/IO/AbstractIOHandler.hpp" +#include "openPMD/IO/IOTask.hpp" + +#include +namespace read_nonexistent_attribute +{ +using namespace openPMD; +static auto testedFileExtensions() -> std::vector +{ + auto allExtensions = getFileExtensions(); + auto newEnd = std::remove_if( + allExtensions.begin(), + allExtensions.end(), + []([[maybe_unused]] std::string const &ext) { + // sst and ssc need a receiver for testing + // bp5 is already tested via bp + // toml parsing is very slow and its implementation is equivalent to + // the json backend, so it is only activated for selected tests + return ext == "sst" || ext == "ssc" || ext == "bp5" || + ext == "toml"; + }); + return {allExtensions.begin(), newEnd}; +} + +void run(std::string const &ext) +{ + auto const filename = "../samples/read_nonexistent_attribute." + ext; + + auto do_create = [&filename]() { + Series write(filename, Access::CREATE); + write.close(); + }; + + do_create(); + + // Try reading an attribute from this Series which does not actually exist. + // This tests the bugs fixed in + // https://github.com/openPMD/openPMD-api/pull/1866: + // + // 1. The HDF5 backend should verify that the attribute exists before trying + // to read it. Otherwise, the read failure will print ugly backtraces. + // 2. The HDF5 backend should clean up resources also in case an operations + // returns early. Otherwise the second call to do_create() will fail, + // since the first HDF5 file will remain open (resource leak). + { + Series read(filename, Access::READ_ONLY); + Parameter readAttr; + readAttr.name = "this_attribute_does_hopefully_not_exist"; + read.IOHandler()->enqueue(IOTask(&read, readAttr)); + REQUIRE_THROWS_AS( + read.IOHandler()->flush(internal::defaultFlushParams), + error::ReadError); + } + + do_create(); +} + +void read_nonexistent_attribute() +{ + for (auto const &ext : testedFileExtensions()) + { + run(ext); + } +} +} // namespace read_nonexistent_attribute +#endif From 87bfab8021e33cbadf9da0370b2e089d446fb92d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 26 Mar 2026 13:13:45 +0100 Subject: [PATCH 2/7] HDF5 readAttribute: cleanup also upon failure retrigger ci --- include/openPMD/auxiliary/Defer.hpp | 23 ++++++ src/IO/HDF5/HDF5IOHandler.cpp | 111 ++++++++++++++-------------- 2 files changed, 80 insertions(+), 54 deletions(-) create mode 100644 include/openPMD/auxiliary/Defer.hpp diff --git a/include/openPMD/auxiliary/Defer.hpp b/include/openPMD/auxiliary/Defer.hpp new file mode 100644 index 0000000000..e8e2a8d988 --- /dev/null +++ b/include/openPMD/auxiliary/Defer.hpp @@ -0,0 +1,23 @@ +#pragma once + +#include +#include + +namespace openPMD::auxiliary +{ +template +struct defer_type +{ + F functor; + ~defer_type() + { + std::move(functor)(); + } +}; + +template +auto defer(F &&functor) -> defer_type> +{ + return defer_type>{std::forward(functor)}; +} +} // namespace openPMD::auxiliary diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp index ee02020983..c915f19d74 100644 --- a/src/IO/HDF5/HDF5IOHandler.cpp +++ b/src/IO/HDF5/HDF5IOHandler.cpp @@ -25,6 +25,7 @@ #include "openPMD/IO/Access.hpp" #include "openPMD/IO/FlushParametersInternal.hpp" #include "openPMD/IO/HDF5/HDF5IOHandlerImpl.hpp" +#include "openPMD/auxiliary/Defer.hpp" #include "openPMD/auxiliary/Environment.hpp" #include "openPMD/auxiliary/JSON_internal.hpp" #include "openPMD/auxiliary/Variant.hpp" @@ -2506,6 +2507,15 @@ void HDF5IOHandlerImpl::readAttribute( H5Pset_all_coll_metadata_ops(fapl, true); } #endif + auto defer_close_fapl = auxiliary::defer([&]() { + status = H5Pclose(fapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 " + "attribute during attribute read." + << std::endl; + } + }); obj_id = H5Oopen(file.id, concrete_h5_file_position(writable).c_str(), fapl); @@ -2519,7 +2529,26 @@ void HDF5IOHandlerImpl::readAttribute( concrete_h5_file_position(writable).c_str() + "' during attribute read"); } + auto defer_close_obj_id = auxiliary::defer([&]() { + status = H5Oclose(obj_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close " + + concrete_h5_file_position(writable) + + " during attribute read." + << std::endl; + ; + } + }); std::string const &attr_name = parameters.name; + if (H5Aexists(obj_id, attr_name.c_str()) <= 0) + { + throw error::ReadError( + error::AffectedObject::Attribute, + error::Reason::NotFound, + "HDF5", + parameters.name); + } attr_id = H5Aopen(obj_id, attr_name.c_str(), H5P_DEFAULT); if (attr_id < 0) { @@ -2533,10 +2562,38 @@ void HDF5IOHandlerImpl::readAttribute( concrete_h5_file_position(writable).c_str() + ") during attribute read"); } + auto defer_close_attr_id = auxiliary::defer([&]() { + status = H5Aclose(attr_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close attribute " + + attr_name + " at " + concrete_h5_file_position(writable) + + " during attribute read." + << std::endl; + } + }); hid_t attr_type, attr_space; attr_type = H5Aget_type(attr_id); + auto defer_close_attr_type = auxiliary::defer([&]() { + status = H5Tclose(attr_type); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close attribute " + "file space during attribute read." + << std::endl; + } + }); attr_space = H5Aget_space(attr_id); + auto defer_close_attr_space = auxiliary::defer([&]() { + status = H5Sclose(attr_space); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close attribute " + "datatype during attribute read." + << std::endl; + } + }); int ndims = H5Sget_simple_extent_ndims(attr_space); std::vector dims(ndims, 0); @@ -3065,63 +3122,9 @@ void HDF5IOHandlerImpl::readAttribute( " at " + concrete_h5_file_position(writable)); } - status = H5Tclose(attr_type); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Attribute, - error::Reason::CannotRead, - "HDF5", - "[HDF5] Internal error: Failed to close attribute datatype during " - "attribute read"); - } - status = H5Sclose(attr_space); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Attribute, - error::Reason::CannotRead, - "HDF5", - "[HDF5] Internal error: Failed to close attribute file space " - "during " - "attribute read"); - } - auto dtype = parameters.dtype; *dtype = a.dtype; *parameters.m_resource = a.getAny(); - - status = H5Aclose(attr_id); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Attribute, - error::Reason::CannotRead, - "HDF5", - "[HDF5] Internal error: Failed to close attribute " + attr_name + - " at " + concrete_h5_file_position(writable) + - " during attribute read"); - } - status = H5Oclose(obj_id); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Attribute, - error::Reason::CannotRead, - "HDF5", - "[HDF5] Internal error: Failed to close " + - concrete_h5_file_position(writable) + " during attribute read"); - } - status = H5Pclose(fapl); - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Attribute, - error::Reason::CannotRead, - "HDF5", - "[HDF5] Internal error: Failed to close HDF5 attribute during " - "attribute read"); - } } void HDF5IOHandlerImpl::listPaths( From 889c2e14e5ddc4906a1a02311202f9f1f8fcfc87 Mon Sep 17 00:00:00 2001 From: AI Agent Date: Fri, 27 Mar 2026 16:15:54 +0000 Subject: [PATCH 3/7] HDF5 IO: cleanup resources upon failure in more functions Apply the defer pattern (introduced in readAttribute fix) to ensure HDF5 resources are properly closed when exceptions occur in writeAttribute, readDataset, listPaths, listDatasets, listAttributes, and deleteAttribute. The defer pattern uses auxiliary::defer to register cleanup callbacks that run automatically when the function exits, ensuring resources are released even when errors occur mid-function. --- src/IO/HDF5/HDF5IOHandler.cpp | 252 ++++++++++++++++++++++------------ 1 file changed, 164 insertions(+), 88 deletions(-) diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp index c915f19d74..0fce0b5ee7 100644 --- a/src/IO/HDF5/HDF5IOHandler.cpp +++ b/src/IO/HDF5/HDF5IOHandler.cpp @@ -1816,17 +1816,23 @@ void HDF5IOHandlerImpl::deleteAttribute( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 group during attribute " "deletion"); + herr_t status = 0; + auto defer_close_node_id = + auxiliary::defer([&]() { + status = H5Oclose(node_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 group " + "during attribute deletion." + << std::endl; + } + }); - herr_t status = H5Adelete(node_id, name.c_str()); + status = H5Adelete(node_id, name.c_str()); VERIFY( status == 0, "[HDF5] Internal error: Failed to delete HDF5 attribute"); - - status = H5Oclose(node_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 group during " - "attribute deletion"); } } @@ -1994,6 +2000,7 @@ void HDF5IOHandlerImpl::writeAttribute( auto res = getFile(writable); File file = res ? res.value() : getFile(writable->parent).value(); hid_t node_id, attribute_id; + herr_t status = 0; hid_t fapl = H5Pcreate(H5P_LINK_ACCESS); #if H5_VERSION_GE(1, 10, 0) && openPMD_HAVE_MPI @@ -2002,6 +2009,15 @@ void HDF5IOHandlerImpl::writeAttribute( H5Pset_all_coll_metadata_ops(fapl, true); } #endif + auto defer_close_fapl = auxiliary::defer([&]() { + status = H5Pclose(fapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 " + "property during attribute write." + << std::endl; + } + }); node_id = H5Oopen(file.id, concrete_h5_file_position(writable).c_str(), fapl); @@ -2009,9 +2025,18 @@ void HDF5IOHandlerImpl::writeAttribute( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 object during attribute " "write"); + auto defer_close_node_id = auxiliary::defer([&]() { + status = H5Oclose(node_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close " + + concrete_h5_file_position(writable) + + " during attribute write." + << std::endl; + } + }); Attribute const att(Attribute::from_any, parameters.m_resource); Datatype dtype = parameters.dtype; - herr_t status; GetH5DataType getH5DataType({ {typeid(bool).name(), m_H5T_BOOL_ENUM}, {typeid(std::complex).name(), m_H5T_CFLOAT}, @@ -2023,6 +2048,15 @@ void HDF5IOHandlerImpl::writeAttribute( dataType >= 0, "[HDF5] Internal error: Failed to get HDF5 datatype during attribute " "write"); + auto defer_close_dataType = auxiliary::defer([&]() { + status = H5Tclose(dataType); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 datatype " + "during attribute write." + << std::endl; + } + }); std::string name = parameters.name; auto create_attribute_anew = [&]() { hid_t dataspace = getH5DataSpace(att); @@ -2047,6 +2081,7 @@ void HDF5IOHandlerImpl::writeAttribute( "[HDF5] Internal error: Failed to close HDF5 dataspace during " "attribute write"); }; + bool created_new_attribute = false; if (H5Aexists(node_id, name.c_str()) != 0) { attribute_id = H5Aopen(node_id, name.c_str(), H5P_DEFAULT); @@ -2067,7 +2102,7 @@ void HDF5IOHandlerImpl::writeAttribute( equal >= 0, "[HDF5] Internal error: Failed to compare HDF5 attribute types " "during attribute write"); - if (equal == 0) // unequal + if (equal == 0) // unequal - need to delete and recreate { status = H5Aclose(attribute_id); VERIFY( @@ -2082,12 +2117,30 @@ void HDF5IOHandlerImpl::writeAttribute( "attribute " "during attribute write"); create_attribute_anew(); + created_new_attribute = true; } } else { create_attribute_anew(); + created_new_attribute = true; } + auto defer_close_attribute_id = + auxiliary::defer([&]() { + if (created_new_attribute || attribute_id >= 0) + { + status = H5Aclose(attribute_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close attribute " + + name + " at " + + concrete_h5_file_position(writable) + + " during attribute write." + << std::endl; + } + } + }); using DT = Datatype; switch (dtype) @@ -2294,28 +2347,6 @@ void HDF5IOHandlerImpl::writeAttribute( "[HDF5] Internal error: Failed to write attribute " + name + " at " + concrete_h5_file_position(writable)); - status = H5Tclose(dataType); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 datatype during Attribute " - "write"); - - status = H5Aclose(attribute_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close attribute " + name + " at " + - concrete_h5_file_position(writable) + " during attribute write"); - status = H5Oclose(node_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close " + - concrete_h5_file_position(writable) + " during attribute write"); - status = H5Pclose(fapl); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 property during attribute " - "write"); - m_fileNames[writable] = file.name; } @@ -2331,8 +2362,28 @@ void HDF5IOHandlerImpl::readDataset( dataset_id >= 0, "[HDF5] Internal error: Failed to open HDF5 dataset during dataset " "read"); + auto defer_close_dataset_id = auxiliary::defer([&]() { + status = H5Dclose(dataset_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close dataset during " + "dataset read." + << std::endl; + } + }); filespace = H5Dget_space(dataset_id); + auto defer_close_filespace = auxiliary::defer([&]() { + status = H5Sclose(filespace); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close dataset file space " + "during dataset read." + << std::endl; + } + }); int ndims = H5Sget_simple_extent_ndims(filespace); if (ndims == 0) @@ -2380,6 +2431,16 @@ void HDF5IOHandlerImpl::readDataset( "[HDF5] Internal error: Failed to select hyperslab during dataset " "read"); } + auto defer_close_memspace = auxiliary::defer([&]() { + status = H5Sclose(memspace); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close dataset memory " + "space during dataset read." + << std::endl; + } + }); void *data = parameters.data.get(); @@ -2420,6 +2481,16 @@ void HDF5IOHandlerImpl::readDataset( {typeid(std::complex).name(), m_H5T_CLONG_DOUBLE}, }); hid_t dataType = getH5DataType(a); + auto defer_close_dataType = auxiliary::defer([&]() { + status = H5Tclose(dataType); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close dataset datatype " + "during dataset read." + << std::endl; + } + }); if (H5Tequal(dataType, H5T_NATIVE_LDOUBLE)) { // We have previously determined in openDataset() that this dataset is @@ -2464,26 +2535,6 @@ void HDF5IOHandlerImpl::readDataset( m_datasetTransferProperty, data); VERIFY(status == 0, "[HDF5] Internal error: Failed to read dataset"); - - status = H5Tclose(dataType); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset datatype during " - "dataset read"); - status = H5Sclose(filespace); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset file space during " - "dataset read"); - status = H5Sclose(memspace); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset memory space during " - "dataset read"); - status = H5Dclose(dataset_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset during dataset read"); } void HDF5IOHandlerImpl::readAttribute( @@ -3136,6 +3187,7 @@ void HDF5IOHandlerImpl::listPaths( "listing"); File file = requireFile("listPaths", writable, /* checkParent = */ true); + herr_t status = 0; hid_t gapl = H5Pcreate(H5P_GROUP_ACCESS); #if H5_VERSION_GE(1, 10, 0) && openPMD_HAVE_MPI @@ -3144,15 +3196,34 @@ void HDF5IOHandlerImpl::listPaths( H5Pset_all_coll_metadata_ops(gapl, true); } #endif + auto defer_close_gapl = auxiliary::defer([&]() { + status = H5Pclose(gapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 property " + "during path listing." + << std::endl; + } + }); hid_t node_id = H5Gopen(file.id, concrete_h5_file_position(writable).c_str(), gapl); VERIFY( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 group during path listing"); + auto defer_close_node_id = auxiliary::defer([&]() { + status = H5Gclose(node_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 group " + + concrete_h5_file_position(writable) + + " during path listing." + << std::endl; + } + }); H5G_info_t group_info; - herr_t status = H5Gget_info(node_id, &group_info); + status = H5Gget_info(node_id, &group_info); VERIFY( status == 0, "[HDF5] Internal error: Failed to get HDF5 group info for " + @@ -3169,17 +3240,6 @@ void HDF5IOHandlerImpl::listPaths( paths->emplace_back(name.data(), name_length); } } - - status = H5Gclose(node_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 group " + - concrete_h5_file_position(writable) + " during path listing"); - status = H5Pclose(gapl); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 property during path " - "listing"); } void HDF5IOHandlerImpl::listDatasets( @@ -3191,6 +3251,7 @@ void HDF5IOHandlerImpl::listDatasets( "listing"); File file = requireFile("listDatasets", writable, /* checkParent = */ true); + herr_t status = 0; hid_t gapl = H5Pcreate(H5P_GROUP_ACCESS); #if H5_VERSION_GE(1, 10, 0) && openPMD_HAVE_MPI @@ -3199,6 +3260,15 @@ void HDF5IOHandlerImpl::listDatasets( H5Pset_all_coll_metadata_ops(gapl, true); } #endif + auto defer_close_gapl = auxiliary::defer([&]() { + status = H5Pclose(gapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 property " + "during dataset listing." + << std::endl; + } + }); hid_t node_id = H5Gopen(file.id, concrete_h5_file_position(writable).c_str(), gapl); @@ -3206,9 +3276,19 @@ void HDF5IOHandlerImpl::listDatasets( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 group during dataset " "listing"); + auto defer_close_node_id = auxiliary::defer([&]() { + status = H5Gclose(node_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 group " + + concrete_h5_file_position(writable) + + " during dataset listing." + << std::endl; + } + }); H5G_info_t group_info; - herr_t status = H5Gget_info(node_id, &group_info); + status = H5Gget_info(node_id, &group_info); VERIFY( status == 0, "[HDF5] Internal error: Failed to get HDF5 group info for " + @@ -3225,17 +3305,6 @@ void HDF5IOHandlerImpl::listDatasets( datasets->emplace_back(name.data(), name_length); } } - - status = H5Gclose(node_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 group " + - concrete_h5_file_position(writable) + " during dataset listing"); - status = H5Pclose(gapl); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 property during dataset " - "listing"); } void HDF5IOHandlerImpl::listAttributes( @@ -3249,6 +3318,7 @@ void HDF5IOHandlerImpl::listAttributes( File file = requireFile("listAttributes", writable, /* checkParent = */ true); hid_t node_id; + herr_t status = 0; hid_t fapl = H5Pcreate(H5P_LINK_ACCESS); #if H5_VERSION_GE(1, 10, 0) && openPMD_HAVE_MPI @@ -3257,6 +3327,15 @@ void HDF5IOHandlerImpl::listAttributes( H5Pset_all_coll_metadata_ops(fapl, true); } #endif + auto defer_close_fapl = auxiliary::defer([&]() { + status = H5Pclose(fapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 property " + "during attribute listing." + << std::endl; + } + }); node_id = H5Oopen(file.id, concrete_h5_file_position(writable).c_str(), fapl); @@ -3264,8 +3343,16 @@ void HDF5IOHandlerImpl::listAttributes( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 group during attribute " "listing"); - - herr_t status; + auto defer_close_node_id = auxiliary::defer([&]() { + status = H5Oclose(node_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 object " + + concrete_h5_file_position(writable) + + " during attribute listing." + << std::endl; + } + }); #if H5_VERSION_GE(1, 12, 0) H5O_info2_t object_info; status = H5Oget_info3(node_id, &object_info, H5O_INFO_NUM_ATTRS); @@ -3302,17 +3389,6 @@ void HDF5IOHandlerImpl::listAttributes( H5P_DEFAULT); attributes->emplace_back(name.data(), name_length); } - - status = H5Oclose(node_id); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 object during attribute " - "listing"); - status = H5Pclose(fapl); - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 property during dataset " - "listing"); } void HDF5IOHandlerImpl::deregister( From 8d20d17bdb0eaf7f75da6913b194c01d0822f916 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Mon, 30 Mar 2026 20:12:33 +0200 Subject: [PATCH 4/7] Mark all remaining places that need to be dealt with --- src/IO/HDF5/HDF5IOHandler.cpp | 60 +++++++++++++++++------------------ 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp index 0fce0b5ee7..ecdf53a350 100644 --- a/src/IO/HDF5/HDF5IOHandler.cpp +++ b/src/IO/HDF5/HDF5IOHandler.cpp @@ -451,7 +451,7 @@ void HDF5IOHandlerImpl::createPath( /* Close the groups */ while (!groups.empty()) { - status = H5Gclose(groups.top()); + status = H5Gclose(groups.top()); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 group during path " @@ -466,7 +466,7 @@ void HDF5IOHandlerImpl::createPath( m_fileNames[writable] = file.name; } - status = H5Pclose(gapl); + status = H5Pclose(gapl); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 property during path " @@ -1065,32 +1065,32 @@ void HDF5IOHandlerImpl::createDataset( "creation"); herr_t status; - status = H5Dclose(group_id); + status = H5Dclose(group_id); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 dataset during " "dataset creation"); - status = H5Tclose(datatype); + status = H5Tclose(datatype); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 datatype during " "dataset creation"); - status = H5Pclose(datasetCreationProperty); + status = H5Pclose(datasetCreationProperty); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 dataset creation " "property during dataset creation"); - status = H5Sclose(space); + status = H5Sclose(space); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 dataset space during " "dataset creation"); - status = H5Gclose(node_id); + status = H5Gclose(node_id); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 group during dataset " "creation"); - status = H5Pclose(gapl); + status = H5Pclose(gapl); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 property during " @@ -1168,7 +1168,7 @@ void HDF5IOHandlerImpl::extendDataset( "[HDF5] Internal error: Failed to extend HDF5 dataset during dataset " "extension"); - status = H5Dclose(dataset_id); + status = H5Dclose(dataset_id); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 dataset during dataset " @@ -1232,13 +1232,13 @@ void HDF5IOHandlerImpl::availableChunks( parameters.chunks->emplace_back(std::move(offset), std::move(extent)); herr_t status; - status = H5Sclose(dataset_space); + status = H5Sclose(dataset_space); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 dataset space during " "availableChunks task"); - status = H5Dclose(dataset_id); + status = H5Dclose(dataset_id); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 dataset during " @@ -1363,7 +1363,7 @@ void HDF5IOHandlerImpl::openPath( } herr_t status; - status = H5Gclose(path_id); + status = H5Gclose(path_id); // if (status != 0) { throw error::ReadError( @@ -1376,7 +1376,7 @@ void HDF5IOHandlerImpl::openPath( } herr_t status; - status = H5Gclose(node_id); + status = H5Gclose(node_id); // if (status != 0) { throw error::ReadError( @@ -1386,7 +1386,7 @@ void HDF5IOHandlerImpl::openPath( "[HDF5] Internal error: Failed to close HDF5 group during path " "opening"); } - status = H5Pclose(gapl); + status = H5Pclose(gapl); // if (status != 0) { throw error::ReadError( @@ -1546,12 +1546,12 @@ void HDF5IOHandlerImpl::openDataset( } else if (H5Tequal(dataset_type, next_type)) { - H5Tclose(next_type); + H5Tclose(next_type); // throw_error(); } else { - if (H5Tclose(dataset_type) != 0) + if (H5Tclose(dataset_type) != 0) // { throw error::ReadError( error::AffectedObject::Group, @@ -1599,7 +1599,7 @@ void HDF5IOHandlerImpl::openDataset( } herr_t status; - status = H5Sclose(dataset_space); + status = H5Sclose(dataset_space); // if (status != 0) { throw error::ReadError( @@ -1609,7 +1609,7 @@ void HDF5IOHandlerImpl::openDataset( "Internal error: Failed to close HDF5 dataset space during " "dataset opening"); } - status = H5Tclose(dataset_type); + status = H5Tclose(dataset_type); // if (status != 0) { throw error::ReadError( @@ -1619,7 +1619,7 @@ void HDF5IOHandlerImpl::openDataset( "Internal error: Failed to close HDF5 dataset type during " "dataset opening"); } - status = H5Dclose(dataset_id); + status = H5Dclose(dataset_id); // if (status != 0) { throw error::ReadError( @@ -1629,7 +1629,7 @@ void HDF5IOHandlerImpl::openDataset( "Internal error: Failed to close HDF5 dataset during dataset " "opening"); } - status = H5Gclose(node_id); + status = H5Gclose(node_id); // if (status != 0) { throw error::ReadError( @@ -1639,7 +1639,7 @@ void HDF5IOHandlerImpl::openDataset( "Internal error: Failed to close HDF5 group during dataset " "opening"); } - status = H5Pclose(gapl); + status = H5Pclose(gapl); // if (status != 0) { throw error::ReadError( @@ -1730,7 +1730,7 @@ void HDF5IOHandlerImpl::deletePath( VERIFY( status == 0, "[HDF5] Internal error: Failed to delete HDF5 group"); - status = H5Gclose(node_id); + status = H5Gclose(node_id); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 group during path " @@ -1782,7 +1782,7 @@ void HDF5IOHandlerImpl::deleteDataset( VERIFY( status == 0, "[HDF5] Internal error: Failed to delete HDF5 group"); - status = H5Gclose(node_id); + status = H5Gclose(node_id); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 group during dataset " @@ -1959,22 +1959,22 @@ void HDF5IOHandlerImpl::writeDataset( default: throw std::runtime_error("[HDF5] Datatype not implemented in HDF5 IO"); } - status = H5Tclose(dataType); + status = H5Tclose(dataType); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close dataset datatype during " "dataset write"); - status = H5Sclose(filespace); + status = H5Sclose(filespace); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close dataset file space during " "dataset write"); - status = H5Sclose(memspace); + status = H5Sclose(memspace); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close dataset memory space during " "dataset write"); - status = H5Dclose(dataset_id); + status = H5Dclose(dataset_id); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close dataset " + @@ -2075,7 +2075,7 @@ void HDF5IOHandlerImpl::writeAttribute( node_id >= 0, "[HDF5] Internal error: Failed to create HDF5 attribute during " "attribute write"); - status = H5Sclose(dataspace); + status = H5Sclose(dataspace); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 dataspace during " @@ -2503,7 +2503,7 @@ void HDF5IOHandlerImpl::readDataset( { dataType = m_H5T_LONG_DOUBLE_80_LE; } - status = H5Tclose(checkDatasetTypeAgain); + status = H5Tclose(checkDatasetTypeAgain); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 dataset type during " @@ -2517,7 +2517,7 @@ void HDF5IOHandlerImpl::readDataset( { dataType = m_H5T_CLONG_DOUBLE_80_LE; } - status = H5Tclose(checkDatasetTypeAgain); + status = H5Tclose(checkDatasetTypeAgain); // VERIFY( status == 0, "[HDF5] Internal error: Failed to close HDF5 dataset type during " From 26a85f765ad9399554f09830e7e273ab1cc1b7e8 Mon Sep 17 00:00:00 2001 From: AI Agent Date: Mon, 30 Mar 2026 18:27:32 +0000 Subject: [PATCH 5/7] HDF5 IO: apply defer pattern to remaining functions Apply the defer RAII pattern to ensure HDF5 resources are properly closed when exceptions occur in createPath, createDataset, openPath, openDataset, deletePath, deleteDataset, writeAttribute, and readDataset. Uses auxiliary::defer to register cleanup callbacks that run automatically when functions exit, even on exceptions. --- src/IO/HDF5/HDF5IOHandler.cpp | 442 ++++++++++++++++++++-------------- 1 file changed, 256 insertions(+), 186 deletions(-) diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp index ecdf53a350..cd2421a32c 100644 --- a/src/IO/HDF5/HDF5IOHandler.cpp +++ b/src/IO/HDF5/HDF5IOHandler.cpp @@ -397,7 +397,7 @@ void HDF5IOHandlerImpl::createPath( } #endif - herr_t status; + herr_t status = 0; if (!writable->written) { @@ -427,6 +427,20 @@ void HDF5IOHandlerImpl::createPath( /* Create the path in the file */ std::stack groups; groups.push(node_id); + auto defer_close_groups = auxiliary::defer([&]() { + while (!groups.empty()) + { + status = H5Gclose(groups.top()); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 group " + "during path creation." + << std::endl; + } + groups.pop(); + } + }); for (std::string const &folder : auxiliary::split(path, "/", false)) { // avoid creation of paths that already exist @@ -448,17 +462,6 @@ void HDF5IOHandlerImpl::createPath( groups.push(group_id); } - /* Close the groups */ - while (!groups.empty()) - { - status = H5Gclose(groups.top()); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 group during path " - "creation"); - groups.pop(); - } - writable->written = true; writable->abstractFilePosition = std::make_shared(path); @@ -466,11 +469,15 @@ void HDF5IOHandlerImpl::createPath( m_fileNames[writable] = file.name; } - status = H5Pclose(gapl); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 property during path " - "creation"); + auto defer_close_gapl = auxiliary::defer([&]() { + status = H5Pclose(gapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 property " + "during path creation." + << std::endl; + } + }); } namespace @@ -841,6 +848,7 @@ void HDF5IOHandlerImpl::createDataset( throw error::OperationUnsupportedInBackend( "HDF5", "No support for Datasets with undefined extent."); } + herr_t status = 0; if (!writable->written) { @@ -924,6 +932,27 @@ void HDF5IOHandlerImpl::createDataset( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 group during dataset " "creation"); + auto defer_close_node_id = + auxiliary::defer([&]() { + status = H5Gclose(node_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 group " + "during dataset creation." + << std::endl; + } + }); + auto defer_close_gapl = auxiliary::defer([&]() { + status = H5Pclose(gapl); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 property " + "during dataset creation." + << std::endl; + } + }); if (access::append(m_handler->m_backendAccess)) { @@ -965,9 +994,29 @@ void HDF5IOHandlerImpl::createDataset( space >= 0, "[HDF5] Internal error: Failed to create dataspace during dataset " "creation"); + auto defer_close_space = auxiliary::defer([&]() { + status = H5Sclose(space); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 dataset " + "space during dataset creation." + << std::endl; + } + }); /* enable chunking on the created dataspace */ hid_t datasetCreationProperty = H5Pcreate(H5P_DATASET_CREATE); + auto defer_close_datasetCreationProperty = auxiliary::defer([&]() { + status = H5Pclose(datasetCreationProperty); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 dataset " + "creation property during dataset creation." + << std::endl; + } + }); H5Pset_fill_time(datasetCreationProperty, H5D_FILL_TIME_NEVER); @@ -1051,6 +1100,16 @@ void HDF5IOHandlerImpl::createDataset( datatype >= 0, "[HDF5] Internal error: Failed to get HDF5 datatype during dataset " "creation"); + auto defer_close_datatype = auxiliary::defer([&]() { + status = H5Tclose(datatype); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 datatype " + "during dataset creation." + << std::endl; + } + }); hid_t group_id = H5Dcreate( node_id, name.c_str(), @@ -1063,38 +1122,16 @@ void HDF5IOHandlerImpl::createDataset( group_id >= 0, "[HDF5] Internal error: Failed to create HDF5 group during dataset " "creation"); - - herr_t status; - status = H5Dclose(group_id); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset during " - "dataset creation"); - status = H5Tclose(datatype); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 datatype during " - "dataset creation"); - status = H5Pclose(datasetCreationProperty); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset creation " - "property during dataset creation"); - status = H5Sclose(space); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset space during " - "dataset creation"); - status = H5Gclose(node_id); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 group during dataset " - "creation"); - status = H5Pclose(gapl); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 property during " - "dataset creation"); + auto defer_close_group_id = auxiliary::defer([&]() { + status = H5Dclose(group_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 dataset " + "during dataset creation." + << std::endl; + } + }); writable->written = true; writable->abstractFilePosition = @@ -1135,6 +1172,16 @@ void HDF5IOHandlerImpl::extendDataset( dataset_id >= 0, "[HDF5] Internal error: Failed to open HDF5 dataset during dataset " "extension"); + herr_t status = 0; + auto defer_close_dataset_id = auxiliary::defer([&]() { + status = H5Dclose(dataset_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 dataset " + "during dataset extension." + << std::endl; + } + }); // Datasets may only be extended if they have chunked layout, so let's see // whether this one does @@ -1161,18 +1208,11 @@ void HDF5IOHandlerImpl::extendDataset( for (auto const &val : parameters.extent) size.push_back(static_cast(val)); - herr_t status; status = H5Dset_extent(dataset_id, size.data()); VERIFY( status == 0, "[HDF5] Internal error: Failed to extend HDF5 dataset during dataset " "extension"); - - status = H5Dclose(dataset_id); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset during dataset " - "extension"); } void HDF5IOHandlerImpl::availableChunks( @@ -1192,7 +1232,26 @@ void HDF5IOHandlerImpl::availableChunks( dataset_id >= 0, "[HDF5] Internal error: Failed to open HDF5 dataset during dataset " "read"); + herr_t status = 0; + auto defer_close_dataset_id = auxiliary::defer([&]() { + status = H5Dclose(dataset_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 dataset " + "during availableChunks task." + << std::endl; + } + }); hid_t dataset_space = H5Dget_space(dataset_id); + auto defer_close_dataset_space = auxiliary::defer([&]() { + status = H5Sclose(dataset_space); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 dataset " + "space during availableChunks task." + << std::endl; + } + }); int ndims = H5Sget_simple_extent_ndims(dataset_space); VERIFY( ndims >= 0, @@ -1230,19 +1289,6 @@ void HDF5IOHandlerImpl::availableChunks( extent.push_back(e); } parameters.chunks->emplace_back(std::move(offset), std::move(extent)); - - herr_t status; - status = H5Sclose(dataset_space); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset space during " - "availableChunks task"); - - status = H5Dclose(dataset_id); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset during " - "availableChunks task"); } void HDF5IOHandlerImpl::openFile( @@ -1330,6 +1376,16 @@ void HDF5IOHandlerImpl::openPath( H5Pset_all_coll_metadata_ops(gapl, true); } #endif + herr_t status = 0; + auto defer_close_gapl = auxiliary::defer([&]() { + status = H5Pclose(gapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 property " + "during path opening." + << std::endl; + } + }); node_id = H5Gopen( file.id, concrete_h5_file_position(writable->parent).c_str(), gapl); @@ -1342,6 +1398,15 @@ void HDF5IOHandlerImpl::openPath( "[HDF5] Internal error: Failed to open HDF5 group during path " "opening"); } + auto defer_close_node_id = auxiliary::defer([&]() { + status = H5Gclose(node_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 group " + "during path opening." + << std::endl; + } + }); /* Sanitize path */ std::string path = parameters.path; @@ -1361,40 +1426,17 @@ void HDF5IOHandlerImpl::openPath( "[HDF5] Internal error: Failed to open HDF5 group during path " "opening"); } - - herr_t status; - status = H5Gclose(path_id); // - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "[HDF5] Internal error: Failed to close HDF5 group during path " - "opening"); - } - } - - herr_t status; - status = H5Gclose(node_id); // - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "[HDF5] Internal error: Failed to close HDF5 group during path " - "opening"); - } - status = H5Pclose(gapl); // - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "[HDF5] Internal error: Failed to close HDF5 property during path " - "opening"); + auto defer_close_path_id = + auxiliary::defer([&]() { + status = H5Gclose(path_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 group " + "during path opening." + << std::endl; + } + }); } writable->written = true; @@ -1424,6 +1466,16 @@ void HDF5IOHandlerImpl::openDataset( H5Pset_all_coll_metadata_ops(gapl, true); } #endif + herr_t status = 0; + auto defer_close_gapl = auxiliary::defer([&]() { + status = H5Pclose(gapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 property " + "during dataset opening." + << std::endl; + } + }); node_id = H5Gopen( file.id, concrete_h5_file_position(writable->parent).c_str(), gapl); @@ -1436,6 +1488,15 @@ void HDF5IOHandlerImpl::openDataset( "Internal error: Failed to open HDF5 group during dataset " "opening"); } + auto defer_close_node_id = auxiliary::defer([&]() { + status = H5Gclose(node_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 group " + "during dataset opening." + << std::endl; + } + }); /* Sanitize name */ std::string name = parameters.name; @@ -1454,10 +1515,39 @@ void HDF5IOHandlerImpl::openDataset( "Internal error: Failed to open HDF5 dataset during dataset " "opening"); } + auto defer_close_dataset_id = auxiliary::defer([&]() { + status = H5Dclose(dataset_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 dataset " + "during dataset opening." + << std::endl; + } + }); hid_t dataset_type, dataset_space; dataset_type = H5Dget_type(dataset_id); dataset_space = H5Dget_space(dataset_id); + auto defer_close_dataset_type = auxiliary::defer([&]() { + status = H5Tclose(dataset_type); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 dataset type " + "during dataset opening." + << std::endl; + } + }); + auto defer_close_dataset_space = auxiliary::defer([&]() { + status = H5Sclose(dataset_space); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 dataset space " + "during dataset opening." + << std::endl; + } + }); H5S_class_t dataset_class = H5Sget_simple_extent_type(dataset_space); @@ -1540,18 +1630,26 @@ void HDF5IOHandlerImpl::openDataset( throw_error(); } hid_t next_type = H5Tget_super(dataset_type); + auto defer_close_next_type = auxiliary::defer([&]() { + status = H5Tclose(next_type); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 " + "dataset type during dataset opening." + << std::endl; + } + }); if (next_type == H5I_INVALID_HID) { throw_error(); } else if (H5Tequal(dataset_type, next_type)) { - H5Tclose(next_type); // throw_error(); } else { - if (H5Tclose(dataset_type) != 0) // { throw error::ReadError( error::AffectedObject::Group, @@ -1598,58 +1696,6 @@ void HDF5IOHandlerImpl::openDataset( *extent = e; } - herr_t status; - status = H5Sclose(dataset_space); // - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "Internal error: Failed to close HDF5 dataset space during " - "dataset opening"); - } - status = H5Tclose(dataset_type); // - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "Internal error: Failed to close HDF5 dataset type during " - "dataset opening"); - } - status = H5Dclose(dataset_id); // - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "Internal error: Failed to close HDF5 dataset during dataset " - "opening"); - } - status = H5Gclose(node_id); // - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "Internal error: Failed to close HDF5 group during dataset " - "opening"); - } - status = H5Pclose(gapl); // - if (status != 0) - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "Internal error: Failed to close HDF5 property during dataset " - "opening"); - } - writable->written = true; writable->abstractFilePosition = std::make_shared(name); @@ -1722,20 +1768,26 @@ void HDF5IOHandlerImpl::deletePath( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 group during path " "deletion"); + herr_t status = 0; + auto defer_close_node_id = + auxiliary::defer([&]() { + status = H5Gclose(node_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 group " + "during path deletion." + << std::endl; + } + }); path += static_cast( writable->abstractFilePosition.get()) ->location; - herr_t status = H5Ldelete(node_id, path.c_str(), H5P_DEFAULT); + status = H5Ldelete(node_id, path.c_str(), H5P_DEFAULT); VERIFY( status == 0, "[HDF5] Internal error: Failed to delete HDF5 group"); - status = H5Gclose(node_id); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 group during path " - "deletion"); - writable->written = false; writable->abstractFilePosition.reset(); @@ -1774,20 +1826,26 @@ void HDF5IOHandlerImpl::deleteDataset( node_id >= 0, "[HDF5] Internal error: Failed to open HDF5 group during dataset " "deletion"); + herr_t status = 0; + auto defer_close_node_id = + auxiliary::defer([&]() { + status = H5Gclose(node_id); + if (status != 0) + { + std::cerr + << "[HDF5] Internal error: Failed to close HDF5 group " + "during dataset deletion." + << std::endl; + } + }); name += static_cast( writable->abstractFilePosition.get()) ->location; - herr_t status = H5Ldelete(node_id, name.c_str(), H5P_DEFAULT); + status = H5Ldelete(node_id, name.c_str(), H5P_DEFAULT); VERIFY( status == 0, "[HDF5] Internal error: Failed to delete HDF5 group"); - status = H5Gclose(node_id); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 group during dataset " - "deletion"); - writable->written = false; writable->abstractFilePosition.reset(); @@ -2064,6 +2122,15 @@ void HDF5IOHandlerImpl::writeAttribute( dataspace >= 0, "[HDF5] Internal error: Failed to get HDF5 dataspace during " "attribute write"); + auto defer_close_dataspace = auxiliary::defer([&]() { + status = H5Sclose(dataspace); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 " + "dataspace during attribute write." + << std::endl; + } + }); attribute_id = H5Acreate( node_id, name.c_str(), @@ -2075,11 +2142,6 @@ void HDF5IOHandlerImpl::writeAttribute( node_id >= 0, "[HDF5] Internal error: Failed to create HDF5 attribute during " "attribute write"); - status = H5Sclose(dataspace); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataspace during " - "attribute write"); }; bool created_new_attribute = false; if (H5Aexists(node_id, name.c_str()) != 0) @@ -2499,29 +2561,37 @@ void HDF5IOHandlerImpl::readDataset( // the worked-around m_H5T_LONG_DOUBLE_80_LE. // Check this. hid_t checkDatasetTypeAgain = H5Dget_type(dataset_id); + auto defer_close_checkDatasetTypeAgain = auxiliary::defer([&]() { + status = H5Tclose(checkDatasetTypeAgain); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 " + "dataset type during dataset reading." + << std::endl; + } + }); if (!H5Tequal(checkDatasetTypeAgain, H5T_NATIVE_LDOUBLE)) { dataType = m_H5T_LONG_DOUBLE_80_LE; } - status = H5Tclose(checkDatasetTypeAgain); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset type during " - "dataset reading"); } else if (H5Tequal(dataType, m_H5T_CLONG_DOUBLE)) { // Same deal for m_H5T_CLONG_DOUBLE hid_t checkDatasetTypeAgain = H5Dget_type(dataset_id); + auto defer_close_checkDatasetTypeAgain = auxiliary::defer([&]() { + status = H5Tclose(checkDatasetTypeAgain); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 " + "dataset type during dataset reading." + << std::endl; + } + }); if (!H5Tequal(checkDatasetTypeAgain, m_H5T_CLONG_DOUBLE)) { dataType = m_H5T_CLONG_DOUBLE_80_LE; } - status = H5Tclose(checkDatasetTypeAgain); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close HDF5 dataset type during " - "dataset reading"); } VERIFY( dataType >= 0, From fabc0d8de24b0ef126a6f2a739c66f587e01dca8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Tue, 31 Mar 2026 12:04:46 +0200 Subject: [PATCH 6/7] Fixes --- src/IO/HDF5/HDF5IOHandler.cpp | 93 +++++++++++++++-------------------- 1 file changed, 39 insertions(+), 54 deletions(-) diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp index cd2421a32c..119c97c73e 100644 --- a/src/IO/HDF5/HDF5IOHandler.cpp +++ b/src/IO/HDF5/HDF5IOHandler.cpp @@ -389,6 +389,8 @@ void HDF5IOHandlerImpl::createPath( "[HDF5] Creating a path in a file opened as read only is not " "possible."); + herr_t status = 0; + hid_t gapl = H5Pcreate(H5P_GROUP_ACCESS); #if H5_VERSION_GE(1, 10, 0) && openPMD_HAVE_MPI if (m_hdf5_collective_metadata) @@ -397,7 +399,15 @@ void HDF5IOHandlerImpl::createPath( } #endif - herr_t status = 0; + auto defer_close_gapl = auxiliary::defer([&]() { + status = H5Pclose(gapl); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close HDF5 property " + "during path creation." + << std::endl; + } + }); if (!writable->written) { @@ -468,16 +478,6 @@ void HDF5IOHandlerImpl::createPath( m_fileNames[writable] = file.name; } - - auto defer_close_gapl = auxiliary::defer([&]() { - status = H5Pclose(gapl); - if (status != 0) - { - std::cerr << "[HDF5] Internal error: Failed to close HDF5 property " - "during path creation." - << std::endl; - } - }); } namespace @@ -968,7 +968,7 @@ void HDF5IOHandlerImpl::createDataset( // > should be able to detect and recycle the file space when no // > other reference to the deleted object exists // https://github.com/openPMD/openPMD-api/pull/1007#discussion_r867223316 - herr_t status = H5Ldelete(node_id, name.c_str(), H5P_DEFAULT); + status = H5Ldelete(node_id, name.c_str(), H5P_DEFAULT); VERIFY( status == 0, "[HDF5] Internal error: Failed to delete old dataset '" + @@ -1053,7 +1053,7 @@ void HDF5IOHandlerImpl::createDataset( } else { - herr_t status = H5Pset_chunk( + status = H5Pset_chunk( datasetCreationProperty, chunking->size(), chunking->data()); @@ -1066,7 +1066,7 @@ void HDF5IOHandlerImpl::createDataset( for (auto const &filter : filters) { - herr_t status = std::visit( + status = std::visit( auxiliary::overloaded{ [&](DatasetParams::ByID const &by_id) { return H5Pset_filter( @@ -1527,7 +1527,6 @@ void HDF5IOHandlerImpl::openDataset( hid_t dataset_type, dataset_space; dataset_type = H5Dget_type(dataset_id); - dataset_space = H5Dget_space(dataset_id); auto defer_close_dataset_type = auxiliary::defer([&]() { status = H5Tclose(dataset_type); if (status != 0) @@ -1538,6 +1537,8 @@ void HDF5IOHandlerImpl::openDataset( << std::endl; } }); + + dataset_space = H5Dget_space(dataset_id); auto defer_close_dataset_space = auxiliary::defer([&]() { status = H5Sclose(dataset_space); if (status != 0) @@ -1625,11 +1626,18 @@ void HDF5IOHandlerImpl::openDataset( "HDF5", "Unknown dataset type"); }; + if (remaining_tries == 0) { throw_error(); } + hid_t next_type = H5Tget_super(dataset_type); + if (next_type == H5I_INVALID_HID) + { + throw_error(); + } + auto defer_close_next_type = auxiliary::defer([&]() { status = H5Tclose(next_type); if (status != 0) @@ -1640,29 +1648,15 @@ void HDF5IOHandlerImpl::openDataset( << std::endl; } }); - if (next_type == H5I_INVALID_HID) - { - throw_error(); - } - else if (H5Tequal(dataset_type, next_type)) + + if (H5Tequal(dataset_type, next_type)) { throw_error(); } - else - { - { - throw error::ReadError( - error::AffectedObject::Group, - error::Reason::Other, - "HDF5", - "Internal error: Failed to close HDF5 dataset type " - "during " - "dataset opening"); - } - dataset_type = next_type; - --remaining_tries; - repeat = true; - } + + dataset_type = next_type; + --remaining_tries; + repeat = true; } } while (repeat); } @@ -2143,7 +2137,6 @@ void HDF5IOHandlerImpl::writeAttribute( "[HDF5] Internal error: Failed to create HDF5 attribute during " "attribute write"); }; - bool created_new_attribute = false; if (H5Aexists(node_id, name.c_str()) != 0) { attribute_id = H5Aopen(node_id, name.c_str(), H5P_DEFAULT); @@ -2179,30 +2172,22 @@ void HDF5IOHandlerImpl::writeAttribute( "attribute " "during attribute write"); create_attribute_anew(); - created_new_attribute = true; } } else { create_attribute_anew(); - created_new_attribute = true; } - auto defer_close_attribute_id = - auxiliary::defer([&]() { - if (created_new_attribute || attribute_id >= 0) - { - status = H5Aclose(attribute_id); - if (status != 0) - { - std::cerr - << "[HDF5] Internal error: Failed to close attribute " + - name + " at " + - concrete_h5_file_position(writable) + - " during attribute write." - << std::endl; - } - } - }); + auto defer_close_attribute_id = auxiliary::defer([&]() { + status = H5Aclose(attribute_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close attribute " + + name + " at " + concrete_h5_file_position(writable) + + " during attribute write." + << std::endl; + } + }); using DT = Datatype; switch (dtype) From bac28cfe0139851fb86ea0ad9764d7c55d8fbd1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Tue, 31 Mar 2026 12:36:48 +0200 Subject: [PATCH 7/7] Add remaining fixes --- include/openPMD/auxiliary/Defer.hpp | 23 ++++++++++ src/IO/HDF5/HDF5IOHandler.cpp | 71 ++++++++++++++++++++--------- 2 files changed, 73 insertions(+), 21 deletions(-) diff --git a/include/openPMD/auxiliary/Defer.hpp b/include/openPMD/auxiliary/Defer.hpp index e8e2a8d988..a99bca1d37 100644 --- a/include/openPMD/auxiliary/Defer.hpp +++ b/include/openPMD/auxiliary/Defer.hpp @@ -1,5 +1,6 @@ #pragma once +#include #include #include @@ -9,12 +10,34 @@ template struct defer_type { F functor; + bool do_run_this = true; ~defer_type() { + if (!do_run_this) + { + return; + } + do_run_this = false; std::move(functor)(); } + + auto to_opaque() && -> defer_type> + { + do_run_this = false; + if (!do_run_this) + { + return defer_type>{{}, false}; + } + else + { + return defer_type>{ + std::function{std::move(functor)}}; + } + } }; +using opaque_defer_type = defer_type>; + template auto defer(F &&functor) -> defer_type> { diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp index 119c97c73e..f300a150cd 100644 --- a/src/IO/HDF5/HDF5IOHandler.cpp +++ b/src/IO/HDF5/HDF5IOHandler.cpp @@ -1898,18 +1898,38 @@ void HDF5IOHandlerImpl::writeDataset( File file = requireFile("writeDataset", writable, /* checkParent = */ true); - hid_t dataset_id, filespace, memspace; herr_t status; + hid_t dataset_id, filespace, memspace; dataset_id = H5Dopen( file.id, concrete_h5_file_position(writable).c_str(), H5P_DEFAULT); VERIFY( dataset_id >= 0, "[HDF5] Internal error: Failed to open HDF5 dataset during dataset " "write"); + auto defer_close_dataset = auxiliary::defer([&]() { + status = H5Dclose(dataset_id); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close dataset " + + concrete_h5_file_position(writable) + + " during dataset write" + << std::endl; + } + }); filespace = H5Dget_space(dataset_id); + auto defer_close_filespace = auxiliary::defer([&]() { + status = H5Sclose(filespace); + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close dataset file " + "space during dataset write" + << std::endl; + } + }); int ndims = H5Sget_simple_extent_ndims(filespace); + auxiliary::opaque_defer_type defer_close_memspace; if (ndims == 0) { if (parameters.offset != Offset{0} || parameters.extent != Extent{1}) @@ -1930,6 +1950,16 @@ void HDF5IOHandlerImpl::writeDataset( memspace > 0, "[HDF5] Internal error: Failed to create memspace during dataset " "write"); + defer_close_memspace = + auxiliary::defer([&]() { + status = H5Sclose(memspace); // + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close " + "dataset memory space during dataset write" + << std::endl; + } + }).to_opaque(); } else { @@ -1943,6 +1973,16 @@ void HDF5IOHandlerImpl::writeDataset( block.push_back(static_cast(val)); memspace = H5Screate_simple( static_cast(block.size()), block.data(), nullptr); + defer_close_memspace = + auxiliary::defer([&]() { + status = H5Sclose(memspace); // + if (status != 0) + { + std::cerr << "[HDF5] Internal error: Failed to close " + "dataset memory space during dataset write" + << std::endl; + } + }).to_opaque(); status = H5Sselect_hyperslab( filespace, H5S_SELECT_SET, @@ -1973,6 +2013,15 @@ void HDF5IOHandlerImpl::writeDataset( dataType >= 0, "[HDF5] Internal error: Failed to get HDF5 datatype during dataset " "write"); + auto defer_close_dataType = auxiliary::defer([&]() { + status = H5Tclose(dataType); + if (status == 0) + { + std::cerr << "[HDF5] Internal error: Failed to close dataset " + "datatype during dataset write." + << std::endl; + } + }); switch (a.dtype) { using DT = Datatype; @@ -2011,26 +2060,6 @@ void HDF5IOHandlerImpl::writeDataset( default: throw std::runtime_error("[HDF5] Datatype not implemented in HDF5 IO"); } - status = H5Tclose(dataType); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset datatype during " - "dataset write"); - status = H5Sclose(filespace); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset file space during " - "dataset write"); - status = H5Sclose(memspace); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset memory space during " - "dataset write"); - status = H5Dclose(dataset_id); // - VERIFY( - status == 0, - "[HDF5] Internal error: Failed to close dataset " + - concrete_h5_file_position(writable) + " during dataset write"); m_fileNames[writable] = file.name; }