image_pusher: reset writer when EndDataCollection finalization fails

HDF5FilePusher::EndDataCollection released the FileWriter only after
writer->WriteHDF5()/Finalize(), both of which throw at rename time when a
target file already exists and overwrite is off. The throw skipped
writer.reset(), leaving 'writer' non-null so the next StartDataCollection
died with "Image pusher is already writing images".

Tear the writer down on every exit path (try/catch + rethrow) so a failed
collection no longer blocks the next one. The error still propagates and the
orphaned temp files are intentionally left on disk.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
2026-07-09 14:43:42 +02:00
co-authored by Claude Fable 5
parent ec05a6c4d6
commit 36be60774f
2 changed files with 54 additions and 9 deletions
+17 -8
View File
@@ -39,16 +39,25 @@ void HDF5FilePusher::StartDataCollection(StartMessage &message) {
}
bool HDF5FilePusher::EndDataCollection(const EndMessage &message) {
if (writer_future.valid()) {
writer_queue.PutBlocking({.end = true});
writer_future.get();
} else
if (!writer_future.valid())
throw JFJochException(JFJochExceptionCategory::WrongDAQState, "Image pusher wasn't writing files");
if (!writer)
throw JFJochException(JFJochExceptionCategory::WrongDAQState, "Image pusher not ready for writing images");
writer->WriteHDF5(message);
writer->Finalize();
writer_queue.PutBlocking({.end = true});
// Release the writer on every exit path. If finalization throws (e.g. a target
// file already exists and overwrite is off) the writer must still be torn down,
// otherwise the next collection fails with "already writing images".
try {
writer_future.get();
if (!writer)
throw JFJochException(JFJochExceptionCategory::WrongDAQState, "Image pusher not ready for writing images");
writer->WriteHDF5(message);
writer->Finalize();
} catch (...) {
writer.reset();
throw;
}
writer.reset();
if (repub_socket) {
+37 -1
View File
@@ -3,10 +3,12 @@
#include <catch2/catch_all.hpp>
#include <iostream>
#include <fstream>
#include "../common/DiffractionExperiment.h"
#include "../writer/HDF5Objects.h"
#include "../writer/FileWriter.h"
#include "../image_pusher/HDF5FilePusher.h"
#include "../compression/JFJochCompressor.h"
#include "../common/AzimuthalIntegrationProfile.h"
#include <nlohmann/json.hpp>
@@ -1442,4 +1444,38 @@ TEST_CASE("HDF5Objects_VDS_reverse_strided", "[HDF5][Unit]") {
remove("scratch_vds_reverse_strided.h5");
REQUIRE(H5Fget_obj_count(H5F_OBJ_ALL, H5F_OBJ_ALL) == 0);
}
}
// Reproduces the "pusher stuck after overwrite failure" bug: if EndDataCollection
// throws while finalizing (here: master file already exists, overwrite off), the
// pusher must still tear down its writer so a subsequent collection can start.
TEST_CASE("HDF5FilePusher_overwrite_failure_recovers", "[HDF5FilePusher][Repro]") {
RegisterHDF5Filter();
DiffractionExperiment x(DetJF4M());
x.FilePrefix("pusher_overwrite_repro").ImagesPerTrigger(1)
.Compression(CompressionAlgorithm::NO_COMPRESSION)
.SetFileWriterFormat(FileWriterFormat::NXmxVDS)
.OverwriteExistingFiles(false);
StartMessage start_message;
x.FillMessage(start_message);
EndMessage end_message{};
// Make the master-file rename fail at EndDataCollection time.
{ std::ofstream(HDF5Metadata::MasterFileName(start_message)) << "blocker"; }
HDF5FilePusher pusher;
pusher.StartDataCollection(start_message);
// Finalization fails because the target master file already exists.
REQUIRE_THROWS_AS(pusher.EndDataCollection(end_message), JFJochException);
// The pusher must have released its writer despite the failure - otherwise the
// next collection dies with "Image pusher is already writing images".
REQUIRE_NOTHROW(pusher.StartDataCollection(start_message));
REQUIRE_THROWS_AS(pusher.EndDataCollection(end_message), JFJochException);
remove("pusher_overwrite_repro_master.h5");
remove(HDF5Metadata::DataFileName(start_message, 0).c_str());
}