JFJochServices: Protect detector with mutex to avoid problems while detector is being configured

broker/JFJochServices.cpp

@@ -18,9 +18,12 @@ void JFJochServices::Start(const DiffractionExperiment& experiment,
     if (receiver != nullptr) {
         logger.Info("   ... receiver start");
         receiver->Start(experiment, pixel_mask, calibration);
-        if (detector && !experiment.IsUsingInternalPacketGen()) {
-            logger.Info("   ... detector start");
-            detector->Start(experiment);
+        {
+            std::shared_lock ul(detector_mutex);
+            if (detector && !experiment.IsUsingInternalPacketGen()) {
+                logger.Info("   ... detector start");
+                detector->Start(experiment);
+            }
         }
     }
 
@@ -28,27 +31,38 @@ void JFJochServices::Start(const DiffractionExperiment& experiment,
 }
 
 void JFJochServices::Off() {
-    if (detector) {
-        detector->Deactivate();
-        detector.reset();
+    std::unique_ptr<DetectorWrapper> old_detector;
+    {
+        std::unique_lock ul(detector_mutex);
+        old_detector = std::move(detector);
+    }
+    if (old_detector) {
+        old_detector->Deactivate();
+        // destroyed here, outside lock
     }
 }
 
 void JFJochServices::On(DiffractionExperiment &x) {
     if (x.IsUsingInternalPacketGen() || (receiver == nullptr))  {
+        std::unique_lock ul(detector_mutex);
         detector.reset();
     } else {
         logger.Info("Detector on");
+        std::unique_ptr<DetectorWrapper> new_detector;
         switch (x.GetDetectorType()) {
             case DetectorType::EIGER:
             case DetectorType::JUNGFRAU:
-                detector = std::make_unique<SLSDetectorWrapper>();
+                new_detector = std::make_unique<SLSDetectorWrapper>();
                 break;
             case DetectorType::DECTRIS:
-                detector = std::make_unique<DectrisDetectorWrapper>();
+                new_detector = std::make_unique<DectrisDetectorWrapper>();
                 break;
         }
-        detector->Initialize(x, receiver->GetNetworkConfig());
+        new_detector->Initialize(x, receiver->GetNetworkConfig());
+        {
+            std::unique_lock ul(detector_mutex);
+            detector = std::move(new_detector);
+        }
         logger.Info("   ... done");
     }
 }
@@ -62,24 +76,28 @@ JFJochServicesOutput JFJochServices::Stop() {
 
     if (receiver != nullptr) {
         try {
-            if (detector) {
-                logger.Info("Wait for detector idle");
-                DetectorState state = detector->GetState();
-                while ((!cannot_stop_detector)
-                       && ((state == DetectorState::WAITING) || (state == DetectorState::BUSY))) {
-                    // check detector state every 5 ms
-                    std::this_thread::sleep_for(std::chrono::milliseconds(5));
-                    state = detector->GetState();
-                }
-                if (state == DetectorState::IDLE) {
-                    logger.Info("   ... detector idle");
-                    receiver->Cancel(true); // cancel silently
-                } else {
-                    logger.Error("   ... detector in error state");
-                    receiver->Cancel(false);
-                    detector_error = true;
+            {
+                std::shared_lock ul(detector_mutex);
+                if (detector) {
+                    logger.Info("Wait for detector idle");
+                    DetectorState state = detector->GetState();
+                    while ((!cannot_stop_detector)
+                           && ((state == DetectorState::WAITING) || (state == DetectorState::BUSY))) {
+                        // check detector state every 5 ms
+                        std::this_thread::sleep_for(std::chrono::milliseconds(5));
+                        state = detector->GetState();
+                    }
+                    if (state == DetectorState::IDLE) {
+                        logger.Info("   ... detector idle");
+                        receiver->Cancel(true); // cancel silently
+                    } else {
+                        logger.Error("   ... detector in error state");
+                        receiver->Cancel(false);
+                        detector_error = true;
+                    }
                 }
             }
+
             logger.Info("Wait for receiver done");
             ret.receiver_output = receiver->Stop();
 
@@ -117,6 +135,7 @@ JFJochServicesOutput JFJochServices::Stop() {
 }
 
 void JFJochServices::Cancel() {
+    std::shared_lock ul(detector_mutex);
     if (detector) {
         // Best effort - if detector cannot be stopped, this is OK, important to still stop receiver
         try {
@@ -163,15 +182,16 @@ void JFJochServices::SetSpotFindingSettings(const SpotFindingSettings &settings)
 }
 
 void JFJochServices::Trigger() {
+    std::shared_lock ul(detector_mutex);
     if (detector && (receiver != nullptr))
         detector->Trigger();
 }
 
 std::optional<DetectorStatus> JFJochServices::GetDetectorStatus() const {
-    if (detector)
+    std::shared_lock ul(detector_mutex, std::defer_lock);
+    if (ul.try_lock_for(std::chrono::milliseconds(500)) && detector)
         return detector->GetStatus();
-    else
-        return {};
+    return {};
 }
 
 std::string JFJochServices::GetPreviewJPEG(const PreviewImageSettings &settings, int64_t image_number) const {
@@ -189,6 +209,7 @@ std::string JFJochServices::GetPreviewTIFF(int64_t image_number) const {
 }
 
 void JFJochServices::ConfigureDetector(const DiffractionExperiment &experiment) {
+    std::unique_lock ul(detector_mutex); // While configuring detector ensure exclusive access (even though pointer is not modified here)
     if (detector)
         detector->Configure(experiment);
 }
@@ -262,6 +283,7 @@ void JFJochServices::ClearImageBuffer() const {
 }
 
 void JFJochServices::LoadDetectorPixelMask(PixelMask &mask) {
+    std::shared_lock ul(detector_mutex);
     if (detector)
         detector->LoadPixelMask(mask);
 }

broker/JFJochServices.h

@@ -4,6 +4,7 @@
 #ifndef JUNGFRAUJOCH_JFJOCHSERVICES_H
 #define JUNGFRAUJOCH_JFJOCHSERVICES_H
 
+#include <shared_mutex>
 #include "../common/DiffractionExperiment.h"
 #include "../jungfrau/JFCalibration.h"
 #include "../common/Logger.h"
@@ -15,6 +16,8 @@ struct JFJochServicesOutput {
 };
 
 class JFJochServices {
+    mutable std::shared_timed_mutex detector_mutex;
+
     JFJochReceiverService *receiver = nullptr;
     std::unique_ptr<DetectorWrapper> detector;