From 77e53ac71439f52ffb943a01727fd159d2879059 Mon Sep 17 00:00:00 2001
From: Filip Leonarski <filip.leonarski@psi.ch>
Date: Wed, 8 Jul 2026 14:36:25 +0200
Subject: [PATCH] Fix exception handling in acquisition failure paths

Follow-ups to the Idle-vs-Error acquisition failure split:

- JFJochServices::Stop: raise the critical detector fault BEFORE the ordinary
  receiver/writer exception. Both can be set at once (detector not idle, then
  receiver->Stop() also throws); throwing the ordinary one first masked the
  detector fault and returned the broker to Idle instead of Error, skipping the
  required re-initialisation.
- SLSDetectorWrapper::Stop/Deactivate: rethrow a JFJochException as-is (as Start
  already does) so InternalStop's JFJochCriticalException is not downgraded to an
  ordinary JFJochException by the generic catch.
- JFJochServices::Start: wrap the whole best-effort receiver cleanup (Cancel +
  Stop) in the catch so a throwing Cancel cannot replace the original detector
  exception before it is re-raised.

Also fix the "cannot create indexing pool" failure seen when retrying
initialize after a failed acquisition start: a failed start leaves the receiver
(and its GPU resources) alive until the next Start, but the retried initialize
rebuilds the indexer pool first, so the fresh GPU indexer had to coexist with
the stale receiver and its init failed. JFJochReceiverService::Indexing now
releases the previous receiver before (re)building the pool (safe: only runs
when the receiver is idle), which also removes a dangling pool pointer held by
that receiver.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---
 broker/JFJochServices.cpp               | 13 +++++++++----
 detector_control/SLSDetectorWrapper.cpp |  4 ++++
 receiver/JFJochReceiverService.cpp      |  8 ++++++++
 3 files changed, 21 insertions(+), 4 deletions(-)

diff --git a/broker/JFJochServices.cpp b/broker/JFJochServices.cpp
index 9f82919b..2fec0ff4 100644
--- a/broker/JFJochServices.cpp
+++ b/broker/JFJochServices.cpp
@@ -37,8 +37,10 @@ void JFJochServices::Start(const DiffractionExperiment& experiment,
         }
     } catch (const std::exception &e) {
         logger.Error("   ... detector failed to start ({}) - stopping receiver", e.what());
-        receiver->Cancel(false);
+        // Best-effort receiver cleanup - it must not replace the original detector exception (which
+        // may be critical), so swallow anything it throws and let the throw below re-raise e.
         try {
+            receiver->Cancel(false);
             receiver->Stop();
         } catch (const std::exception &stop_error) {
             logger.Warning("Receiver stop after failed start reported: {}", stop_error.what());
@@ -156,12 +158,15 @@ JFJochServicesOutput JFJochServices::Stop() {
     }
 
 
-    if (exception)
-        throw JFJochException(*exception);
-
+    // A detector fault must win over an ordinary receiver/writer error: it needs re-initialisation,
+    // so raise the critical exception first. If only the receiver failed, the run can be retried from
+    // Idle, so that ordinary error is raised second.
     if (detector_error)
         throw JFJochCriticalException("Error in detector operation");
 
+    if (exception)
+        throw JFJochException(*exception);
+
     return ret;
 }
 
diff --git a/detector_control/SLSDetectorWrapper.cpp b/detector_control/SLSDetectorWrapper.cpp
index 3d8e0bd5..b0fdf9cf 100644
--- a/detector_control/SLSDetectorWrapper.cpp
+++ b/detector_control/SLSDetectorWrapper.cpp
@@ -259,6 +259,8 @@ void SLSDetectorWrapper::Deactivate() {
             det.setMaster(false, 0);
             det.setSynchronization(false);
         }
+    } catch (const JFJochException &) {
+        throw; // already categorised (incl. critical detector error) - keep its type
     } catch (std::exception &e) {
         logger.ErrorException(e);
         throw JFJochException(JFJochExceptionCategory::Detector, e.what());
@@ -270,6 +272,8 @@ void SLSDetectorWrapper::Stop() {
     logger.Info("Stop");
     try {
         InternalStop();
+    } catch (const JFJochException &) {
+        throw; // already categorised (incl. critical detector error) - keep its type
     } catch (std::exception &e) {
         logger.ErrorException(e);
         throw JFJochException(JFJochExceptionCategory::Detector, e.what());
diff --git a/receiver/JFJochReceiverService.cpp b/receiver/JFJochReceiverService.cpp
index 63b7b5b4..e2ec17c2 100644
--- a/receiver/JFJochReceiverService.cpp
+++ b/receiver/JFJochReceiverService.cpp
@@ -312,6 +312,14 @@ JFJochReceiverService &JFJochReceiverService::Indexing(const IndexingSettings &i
     // Clearing image buffer during data collection could be catastrophic, so better protect here, even if redundant
     // with JFJochStateMachine
     if (state == ReceiverState::Idle) {
+        // Release the previous run's receiver first. It holds a raw pointer to the indexer pool and
+        // its own GPU resources; keeping it alive while we rebuild the pool means a fresh GPU indexer
+        // has to coexist with a stale receiver (e.g. after a failed acquisition start, where the
+        // receiver is stopped but not destroyed until the next Start), which can make the GPU indexer
+        // initialisation fail. Destroying the receiver before the pool also avoids the dangling
+        // pointer. Safe here: state is Idle, so no measurement is using it.
+        receiver.reset();
+
         logger.Info("Resetting indexing thread pool");
         indexer_thread_pool.reset();