From 36be60774f40c33c848bb98323a4e3609171bca9 Mon Sep 17 00:00:00 2001
From: Filip Leonarski <filip.leonarski@psi.ch>
Date: Thu, 9 Jul 2026 14:43:42 +0200
Subject: [PATCH] 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>
---
 image_pusher/HDF5FilePusher.cpp | 25 +++++++++++++++-------
 tests/HDF5WritingTest.cpp       | 38 ++++++++++++++++++++++++++++++++-
 2 files changed, 54 insertions(+), 9 deletions(-)

diff --git a/image_pusher/HDF5FilePusher.cpp b/image_pusher/HDF5FilePusher.cpp
index 5c93f24e..31a1bc2a 100644
--- a/image_pusher/HDF5FilePusher.cpp
+++ b/image_pusher/HDF5FilePusher.cpp
@@ -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) {
diff --git a/tests/HDF5WritingTest.cpp b/tests/HDF5WritingTest.cpp
index a1ec0525..35af40a0 100644
--- a/tests/HDF5WritingTest.cpp
+++ b/tests/HDF5WritingTest.cpp
@@ -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);
-}
\ No newline at end of file
+}
+// 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());
+}