From cda2a2abc6b4ae2567b286227f337d31fc8a3df2 Mon Sep 17 00:00:00 2001
From: P-A Loizeau <p.-a.loizeau@gsi.de>
Date: Tue, 21 Nov 2023 15:53:51 +0100
Subject: [PATCH] Fix UI cmds support in histserv NoFairMq + fix some threading
 risk

---
 services/histserv/app/Application.cxx         | 33 ++++++++++---------
 services/histserv/app/Application.h           |  9 ++---
 .../histserv/lib/CbmServicesHistServLinkDef.h |  2 ++
 services/histserv/lib/ui_callbacks.h          | 27 +++++++++++++--
 4 files changed, 49 insertions(+), 22 deletions(-)

diff --git a/services/histserv/app/Application.cxx b/services/histserv/app/Application.cxx
index ca6ab005ff..8a565c5811 100644
--- a/services/histserv/app/Application.cxx
+++ b/services/histserv/app/Application.cxx
@@ -30,7 +30,6 @@
 #include <zmq_addon.hpp>
 
 #include "Histo1D.h"
-#include "ui_callbacks.h"
 
 std::mutex mtx;
 
@@ -39,7 +38,6 @@ namespace b_ar = boost::archive;
 
 namespace cbm::services::histserv
 {
-
   // -----   Constructor   ---------------------------------------------------------------------------------------------
   Application::Application(ProgramOptions const& opt) : fOpt(opt)
   {
@@ -64,9 +62,13 @@ namespace cbm::services::histserv
     const char* jsrootsys = gSystem->Getenv("JSROOTSYS");
     if (!jsrootsys) jsrootsys = gEnv->GetValue("HttpServ.JSRootPath", jsrootsys);
 
-    fServer->RegisterCommand("/Reset_Hist", "bHistoServerResetHistos=true");
-    fServer->RegisterCommand("/Save_Hist", "bHistoServerSaveHistos=true");
-    fServer->RegisterCommand("/Stop_Server", "bHistoServerStop=true");
+    fUiCmdActor = std::make_unique<UiCmdActor>();
+    fServer->Register("/", fUiCmdActor.get());
+    fServer->Hide("/UiCmdActor");
+
+    fServer->RegisterCommand("/Reset_Hist", "/UiCmdActor/->SetResetHistos()");
+    fServer->RegisterCommand("/Save_Hist", "/UiCmdActor/->SetSaveHistos()");
+    fServer->RegisterCommand("/Stop_Server", "/UiCmdActor/->SetServerStop()");
 
     /*
     fServer->RegisterCommand("/Reset_Hist", "this->ResetHistograms()");
@@ -88,7 +90,7 @@ namespace cbm::services::histserv
   {
     fStopThread = false;
     fThread     = std::thread(&Application::UpdateHttpServer, this);
-    while (!bHistoServerStop) {  //
+    while (!(fUiCmdActor->GetServerStop())) {  //
       /// Infinite loop, this is a service which should survive until told otherwise after all
 
       /// FIXME: Start listening to <SOMETHING?!?> to receive histograms and configuration
@@ -98,6 +100,7 @@ namespace cbm::services::histserv
       const auto ret = zmq::recv_multipart(fZmqSocket, std::back_inserter(vMsg));
       if (!ret) continue;
 
+      std::lock_guard<std::mutex> lk(mtx);
       if (*ret > 3) {  //
         ReceiveConfigAndData(vMsg);
       }
@@ -133,17 +136,17 @@ namespace cbm::services::histserv
 
       /// TODO: control flags communication from histo server to histograms sources?
       /// Idea: 1 req channel (per process or not, mixup?), polling every N TS and/or M s
-      if (bHistoServerResetHistos) {
+      if (fUiCmdActor->GetResetHistos()) {
         LOG(info) << "Reset Monitor histos ";
         ResetHistograms();
-        bHistoServerResetHistos = false;
-      }  // if( bHistoServerResetHistos )
+        fUiCmdActor->SetResetHistos(false);
+      }  // if( fUiCmdActor->GetResetHistos() )
 
-      if (bHistoServerSaveHistos) {
+      if (fUiCmdActor->GetSaveHistos()) {
         LOG(info) << "Save All histos & canvases";
         SaveHistograms();
-        bHistoServerSaveHistos = false;
-      }  // if( bHistoServerSaveHistos )
+        fUiCmdActor->SetSaveHistos(false);
+      }  // if( fUiCmdActor->GetSaveHistos() )
     }
   }
   // -------------------------------------------------------------------------------------------------------------------
@@ -361,7 +364,7 @@ namespace cbm::services::histserv
       uOffsetHistoConfig = 1;
       if (0 < vMsg[uOffsetHistoConfig].size()) {
         fStopThread         = true;
-        bHistoServerStop    = true;
+        fUiCmdActor->SetServerStop();
         std::string err_msg = "Application::ReceiveConfigAndData => No histo config expected but corresponding message";
         err_msg += " is not empty: ";
         err_msg += vMsg[uOffsetHistoConfig].size();
@@ -374,7 +377,7 @@ namespace cbm::services::histserv
       uOffsetCanvasConfig = 1;
       if (0 < vMsg[uOffsetHistoConfig + uOffsetCanvasConfig].size()) {
         fStopThread         = true;
-        bHistoServerStop    = true;
+        fUiCmdActor->SetServerStop();
         std::string err_msg = "Application::ReceiveConfigAndData => No Canvas config expected but corresponding ";
         err_msg += " message is not empty: ";
         err_msg += vMsg[uOffsetHistoConfig + uOffsetCanvasConfig].size();
@@ -384,7 +387,7 @@ namespace cbm::services::histserv
 
     if ((1 + uOffsetHistoConfig + uOffsetCanvasConfig + 1) != vMsg.size()) {
       fStopThread         = true;
-      bHistoServerStop    = true;
+      fUiCmdActor->SetServerStop();
       std::string err_msg = "Application::ReceiveConfigAndData => Nb parts in message not matching configs numbers ";
       err_msg += " declared in header";
       err_msg += vMsg.size();
diff --git a/services/histserv/app/Application.h b/services/histserv/app/Application.h
index fb52caec36..a4f41f1009 100644
--- a/services/histserv/app/Application.h
+++ b/services/histserv/app/Application.h
@@ -8,21 +8,20 @@
 #include "THttpServer.h"
 #include "TObjArray.h"
 
+#include <memory>
 #include <string>
 #include <thread>
 #include <zmq.hpp>
 
 #include "ProgramOptions.h"
+#include "ui_callbacks.h"
 
-class TNamed;
 class TCanvas;
+class TNamed;
 
 namespace cbm::services::histserv
 {
-
   class Application {
-
-
   public:
     /** @brief Standard constructor, initialises the application
      ** @param opt  **/
@@ -64,6 +63,8 @@ namespace cbm::services::histserv
     std::thread fThread;
     bool fStopThread = false;
 
+    std::unique_ptr<UiCmdActor> fUiCmdActor;
+
     /// Interface
     zmq::context_t fZmqContext {1};
     zmq::socket_t fZmqSocket {fZmqContext, ZMQ_PULL};
diff --git a/services/histserv/lib/CbmServicesHistServLinkDef.h b/services/histserv/lib/CbmServicesHistServLinkDef.h
index 843a69aa13..0294bf2b1a 100644
--- a/services/histserv/lib/CbmServicesHistServLinkDef.h
+++ b/services/histserv/lib/CbmServicesHistServLinkDef.h
@@ -8,4 +8,6 @@
 #pragma link off all classes;
 #pragma link off all functions;
 
+#pragma link C++ class cbm::services::histserv::UiCmdActor + ;
+
 #endif
diff --git a/services/histserv/lib/ui_callbacks.h b/services/histserv/lib/ui_callbacks.h
index a658747a76..d951fc458a 100644
--- a/services/histserv/lib/ui_callbacks.h
+++ b/services/histserv/lib/ui_callbacks.h
@@ -5,8 +5,29 @@
 #ifndef CBM_SERVICES_HISTSERV_LIB_UI_CALLBACKS_H
 #define CBM_SERVICES_HISTSERV_LIB_UI_CALLBACKS_H 1
 
-static bool bHistoServerResetHistos = false;
-static bool bHistoServerSaveHistos  = false;
-static bool bHistoServerStop        = false;
+#include "TNamed.h"
 
+namespace cbm::services::histserv
+{
+
+  class UiCmdActor : public TNamed {
+  public:
+    UiCmdActor() : TNamed("UiCmdActor", "UiCmdActor") {};
+
+    void SetResetHistos(bool flag = true) { bHistoServerResetHistos = flag; }
+    void SetSaveHistos(bool flag = true) { bHistoServerSaveHistos = flag; }
+    void SetServerStop(bool flag = true) { bHistoServerStop = flag; }
+
+    bool GetResetHistos() { return bHistoServerResetHistos; }
+    bool GetSaveHistos() { return bHistoServerSaveHistos; }
+    bool GetServerStop() { return bHistoServerStop; }
+
+  private:
+    bool bHistoServerResetHistos = false;
+    bool bHistoServerSaveHistos  = false;
+    bool bHistoServerStop        = false;
+
+    ClassDef(UiCmdActor, 1);
+  };
+}  // namespace cbm::services::histserv
 #endif /* CBM_SERVICES_HISTSERV_LIB_UI_CALLBACKS_H */
-- 
GitLab