From 5e04ece8a8be895a78b4079c7314ea74dd11d3ce Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 3 Apr 2025 17:13:18 +0200 Subject: [PATCH 1/4] Add failing test --- test/SerialIOTest.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index b814eb0a9c..508e19830a 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -7757,11 +7757,12 @@ void groupbased_read_write(std::string const &ext) R"({"toml":{"dataset":{"mode":"dataset"}}})"); // create a new iteration auto E_x = write.iterations[2].meshes["E"]["x"]; - E_x.resetDataset(ds); + E_x.resetDataset({Datatype::INT, {10}}); - data = 2; + std::unique_ptr data_unique(new int[10]); + std::iota(data_unique.get(), data_unique.get() + 10, 0); - E_x.storeChunkRaw(&data, {0}, {1}); + E_x.storeChunk(std::move(data_unique), {0}, {10}); E_x.setAttribute("updated_in_run", 2); write.close(); } From bdae184b5b273f5930c85dd6871e3930c2401106 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 3 Apr 2025 18:16:55 +0200 Subject: [PATCH 2/4] Add failing test --- CMakeLists.txt | 1 + test/Files_SerialIO/SerialIOTests.hpp | 4 +++ .../issue_1744_unique_ptrs_at_close_time.cpp | 33 +++++++++++++++++++ test/SerialIOTest.cpp | 8 +++++ 4 files changed, 46 insertions(+) create mode 100644 test/Files_SerialIO/issue_1744_unique_ptrs_at_close_time.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 3533cb29a4..913f604f82 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -803,6 +803,7 @@ if(openPMD_BUILD_TESTING) list(APPEND ${out_list} test/Files_SerialIO/close_and_reopen_test.cpp test/Files_SerialIO/filebased_write_test.cpp + test/Files_SerialIO/issue_1744_unique_ptrs_at_close_time.cpp ) elseif(${test_name} STREQUAL "ParallelIO" AND openPMD_HAVE_MPI) list(APPEND ${out_list} diff --git a/test/Files_SerialIO/SerialIOTests.hpp b/test/Files_SerialIO/SerialIOTests.hpp index 541d4dcaf1..f5e770681b 100644 --- a/test/Files_SerialIO/SerialIOTests.hpp +++ b/test/Files_SerialIO/SerialIOTests.hpp @@ -8,3 +8,7 @@ namespace close_and_reopen_test { auto close_and_reopen_test() -> void; } +namespace issue_1744_unique_ptrs_at_close_time +{ +auto issue_1744_unique_ptrs_at_close_time() -> void; +} diff --git a/test/Files_SerialIO/issue_1744_unique_ptrs_at_close_time.cpp b/test/Files_SerialIO/issue_1744_unique_ptrs_at_close_time.cpp new file mode 100644 index 0000000000..51464e05f7 --- /dev/null +++ b/test/Files_SerialIO/issue_1744_unique_ptrs_at_close_time.cpp @@ -0,0 +1,33 @@ +#include "SerialIOTests.hpp" + +#include "openPMD/Dataset.hpp" +#include "openPMD/openPMD.hpp" + +#include +#include + +// clang-format off +/* + * Tests regression introduced with #1743: + * terminate called after throwing an instance of 'openPMD::error::Internal' + * what(): Internal error: [ADIOS2 backend] Orphaned unique-ptr put operations found when closing file. + * This is a bug. Please report at ' https://github.com/openPMD/openPMD-api/issues'. + */ +// clang-format on + +namespace issue_1744_unique_ptrs_at_close_time +{ +auto issue_1744_unique_ptrs_at_close_time() -> void +{ + openPMD::Series write( + "../samples/issue_1744_unique_ptrs_at_close_time.bp4", + openPMD::Access::CREATE, + R"({"iteration_encoding": "group_based"})"); + std::unique_ptr data_unique(new int[10]); + std::iota(data_unique.get(), data_unique.get() + 10, 0); + auto E_x = write.snapshots()[0].meshes["E"]["x"]; + E_x.resetDataset({openPMD::Datatype::INT, {10}}); + E_x.storeChunk(std::move(data_unique), {0}, {10}); + write.close(); +} +} // namespace issue_1744_unique_ptrs_at_close_time diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index 508e19830a..89eb30c066 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -760,6 +760,14 @@ TEST_CASE("close_and_copy_attributable_test", "[serial]") } } +TEST_CASE("issue_1744_unique_ptrs_at_close_time", "[serial]") +{ +#if openPMD_HAVE_ADIOS2 + issue_1744_unique_ptrs_at_close_time:: + issue_1744_unique_ptrs_at_close_time(); +#endif +} + #if openPMD_HAVE_ADIOS2 TEST_CASE("close_and_reopen_test", "[serial]") { From 1611ff6caff70ee75c38a169f4aef6825200e574 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 3 Apr 2025 18:17:05 +0200 Subject: [PATCH 3/4] Revert "Add failing test" This reverts commit 5e04ece8a8be895a78b4079c7314ea74dd11d3ce. --- test/SerialIOTest.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp index 89eb30c066..e3050ac7da 100644 --- a/test/SerialIOTest.cpp +++ b/test/SerialIOTest.cpp @@ -7765,12 +7765,11 @@ void groupbased_read_write(std::string const &ext) R"({"toml":{"dataset":{"mode":"dataset"}}})"); // create a new iteration auto E_x = write.iterations[2].meshes["E"]["x"]; - E_x.resetDataset({Datatype::INT, {10}}); + E_x.resetDataset(ds); - std::unique_ptr data_unique(new int[10]); - std::iota(data_unique.get(), data_unique.get() + 10, 0); + data = 2; - E_x.storeChunk(std::move(data_unique), {0}, {10}); + E_x.storeChunkRaw(&data, {0}, {1}); E_x.setAttribute("updated_in_run", 2); write.close(); } From 086a185288ee49017377b667c18a0ac4ee735675 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Thu, 3 Apr 2025 18:22:49 +0200 Subject: [PATCH 4/4] Reactivate writing from unique_ptr in finalize() --- src/IO/ADIOS/ADIOS2File.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/IO/ADIOS/ADIOS2File.cpp b/src/IO/ADIOS/ADIOS2File.cpp index 625261a864..4678e13bec 100644 --- a/src/IO/ADIOS/ADIOS2File.cpp +++ b/src/IO/ADIOS/ADIOS2File.cpp @@ -225,16 +225,18 @@ void ADIOS2File::finalize() { return; } - if (!m_uniquePtrPuts.empty()) - { - throw error::Internal( - "[ADIOS2 backend] Orphaned unique-ptr put operations found " - "when closing file."); - } // if write accessing, ensure that the engine is opened - if (!m_engine && writeOnly(m_mode)) + // and that all datasets are written + // (attributes and unique_ptr datasets are written upon closing a step + // or a file which users might never do) + bool needToWrite = !m_uniquePtrPuts.empty(); + if ((needToWrite || !m_engine) && writeOnly(m_mode)) { getEngine(); + for (auto &entry : m_uniquePtrPuts) + { + entry.run(*this); + } } if (m_engine) { @@ -250,6 +252,7 @@ void ADIOS2File::finalize() m_ADIOS.RemoveIO(m_IOName); } } + m_uniquePtrPuts.clear(); finalized = true; }