From e74e353010626e6de5a584a1aca42ba7f5d1d76f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 6 Aug 2025 14:44:20 +0200 Subject: [PATCH 1/4] Add failing test --- src/IO/AbstractIOHandlerImpl.cpp | 5 ++++- test/python/unittest/API/APITest.py | 17 +++++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/IO/AbstractIOHandlerImpl.cpp b/src/IO/AbstractIOHandlerImpl.cpp index faad2f49a9..b6add8a6d9 100644 --- a/src/IO/AbstractIOHandlerImpl.cpp +++ b/src/IO/AbstractIOHandlerImpl.cpp @@ -268,7 +268,10 @@ std::future AbstractIOHandlerImpl::flush() i.writable->parent, "->", i.writable, - "] WRITE_DATASET"); + "] WRITE_DATASET, offset=", + [¶meter]() { return vec_as_string(parameter.offset); }, + ", extent=", + [¶meter]() { return vec_as_string(parameter.extent); }); writeDataset(i.writable, parameter); break; } diff --git a/test/python/unittest/API/APITest.py b/test/python/unittest/API/APITest.py index 6337807f33..4084d64b20 100644 --- a/test/python/unittest/API/APITest.py +++ b/test/python/unittest/API/APITest.py @@ -82,6 +82,23 @@ def tearDown(self): del self.__particle_series del self.__series + def testRefCounting(self): + write = io.Series( + "../samples/refcounting.json", io.Access.create_linear) + iteration = write.snapshots()[0] + pos_x = iteration.particles["e"]["position"]["x"] + pos_x.reset_dataset(io.Dataset(np.dtype("float"), [100])) + pos_x[:] = np.arange(0, 100, dtype = np.dtype("float")) + write.close() + + read = io.Series("../samples/refcounting.json", io.Access.read_linear) + iteration = read.snapshots()[0] + pos_x = iteration.particles["e"]["position"]["x"] + loaded = pos_x[:] + read.flush() + self.assertTrue(np.allclose( + loaded, np.arange(0, 100, dtype = np.dtype("float")))) + def testFieldData(self): """ Testing serial IO on a pure field dataset. """ From 0759cd19b53f7ed7c97f9a729c885af1dbd9abdd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 6 Aug 2025 14:56:52 +0200 Subject: [PATCH 2/4] Move write part to own function this triggers the error --- test/python/unittest/API/APITest.py | 42 ++++++++++++++++++++++++----- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/test/python/unittest/API/APITest.py b/test/python/unittest/API/APITest.py index 4084d64b20..a81f25b133 100644 --- a/test/python/unittest/API/APITest.py +++ b/test/python/unittest/API/APITest.py @@ -82,14 +82,42 @@ def tearDown(self): del self.__particle_series del self.__series + # This function exhibits a bug in the old use of refcounting. + def refcountingCreateData(self): + series = io.Series( + "../samples/refcounting.json", + io.Access.create_linear, + ) + + for i in range(10): + current_iteration = series.snapshots()[i] + + # First, write an E mesh. + E = current_iteration.meshes["E"] + E.axis_labels = ["x", "y"] + for dim in ["x", "y"]: + component = E[dim] + component.reset_dataset(io.Dataset(np.dtype("float"), [10, 10])) + component[:, :] = np.reshape( + np.arange(i * 100, (i + 1) * 100, dtype=np.dtype("float")), + [10, 10], + ) + + # Now, write some e particles. + e = current_iteration.particles["e"] + for dim in ["x", "y"]: + # Do not bother with a positionOffset + position_offset = e["positionOffset"][dim] + position_offset.make_constant(0) + + position = e["position"][dim] + position.reset_dataset(io.Dataset(np.dtype("float"), [100])) + position[:] = np.arange( + i * 100, (i + 1) * 100, dtype=np.dtype("float") + ) + def testRefCounting(self): - write = io.Series( - "../samples/refcounting.json", io.Access.create_linear) - iteration = write.snapshots()[0] - pos_x = iteration.particles["e"]["position"]["x"] - pos_x.reset_dataset(io.Dataset(np.dtype("float"), [100])) - pos_x[:] = np.arange(0, 100, dtype = np.dtype("float")) - write.close() + self.refcountingCreateData() read = io.Series("../samples/refcounting.json", io.Access.read_linear) iteration = read.snapshots()[0] From 58e20ae1208c738911d1403ee05a179e7f8771a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 6 Aug 2025 14:21:00 +0200 Subject: [PATCH 3/4] Fix refcounting in Python bindings --- src/binding/python/RecordComponent.cpp | 31 ++++++++++++++++---------- 1 file changed, 19 insertions(+), 12 deletions(-) diff --git a/src/binding/python/RecordComponent.cpp b/src/binding/python/RecordComponent.cpp index 9fac7a8cfe..6774b5e182 100644 --- a/src/binding/python/RecordComponent.cpp +++ b/src/binding/python/RecordComponent.cpp @@ -22,6 +22,7 @@ #include #include #include +#include #include #include "openPMD/Dataset.hpp" @@ -322,13 +323,16 @@ struct StoreChunkFromPythonArray Offset const &offset, Extent const &extent) { - // here, we increase a reference on the user-passed data so that + a.inc_ref(); + void *data = a.mutable_data(); + // here, we store an owning handle in the lambda capture so that // temporary and lost-scope variables stay alive until we flush // note: this does not yet prevent the user, as in C++, to build // a race condition by manipulating the data that was passed - a.inc_ref(); - void *data = a.mutable_data(); - std::shared_ptr shared((T *)data, [a](T *) { a.dec_ref(); }); + std::shared_ptr shared( + (T *)data, [owning_handle = a.cast()](T *) { + // no-op + }); r.storeChunk(std::move(shared), offset, extent); } @@ -343,13 +347,15 @@ struct LoadChunkIntoPythonArray Offset const &offset, Extent const &extent) { - // here, we increase a reference on the user-passed data so that + void *data = a.mutable_data(); + // here, we store an owning handle in the lambda capture so that // temporary and lost-scope variables stay alive until we flush // note: this does not yet prevent the user, as in C++, to build // a race condition by manipulating the data that was passed - a.inc_ref(); - void *data = a.mutable_data(); - std::shared_ptr shared((T *)data, [a](T *) { a.dec_ref(); }); + std::shared_ptr shared( + (T *)data, [owning_handle = a.cast()](T *) { + // no-op + }); r.loadChunk(std::move(shared), offset, extent); } @@ -365,14 +371,15 @@ struct LoadChunkIntoPythonBuffer Offset const &offset, Extent const &extent) { - // here, we increase a reference on the user-passed data so that + void *data = buffer_info.ptr; + // here, we store an owning handle in the lambda capture so that // temporary and lost-scope variables stay alive until we flush // note: this does not yet prevent the user, as in C++, to build // a race condition by manipulating the data that was passed - buffer.inc_ref(); - void *data = buffer_info.ptr; std::shared_ptr shared( - (T *)data, [buffer](T *) { buffer.dec_ref(); }); + (T *)data, [owning_handle = buffer.cast()](T *) { + // no-op + }); r.loadChunk(std::move(shared), offset, extent); } From 88032a9c73e570f7f166844c13c7fef11bf0530a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Franz=20P=C3=B6schel?= Date: Wed, 6 Aug 2025 15:06:28 +0200 Subject: [PATCH 4/4] CI fixes --- test/python/unittest/API/APITest.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/test/python/unittest/API/APITest.py b/test/python/unittest/API/APITest.py index a81f25b133..b81cef596d 100644 --- a/test/python/unittest/API/APITest.py +++ b/test/python/unittest/API/APITest.py @@ -97,7 +97,8 @@ def refcountingCreateData(self): E.axis_labels = ["x", "y"] for dim in ["x", "y"]: component = E[dim] - component.reset_dataset(io.Dataset(np.dtype("float"), [10, 10])) + component.reset_dataset( + io.Dataset(np.dtype("float"), [10, 10])) component[:, :] = np.reshape( np.arange(i * 100, (i + 1) * 100, dtype=np.dtype("float")), [10, 10], @@ -125,7 +126,7 @@ def testRefCounting(self): loaded = pos_x[:] read.flush() self.assertTrue(np.allclose( - loaded, np.arange(0, 100, dtype = np.dtype("float")))) + loaded, np.arange(0, 100, dtype=np.dtype("float")))) def testFieldData(self): """ Testing serial IO on a pure field dataset. """