From 15f2c71abfd8015cbc4d6631fec4049870c6229c Mon Sep 17 00:00:00 2001
From: Martyn Gigg <martyn.gigg@gmail.com>
Date: Mon, 16 Apr 2018 08:28:07 +0100
Subject: [PATCH] Clean up fake ISIS live data

Includes:
  - using stack allocation for tcp server
  - removal of unnecessary mutex
  - unused includes removed
---
 .../MantidLiveData/ISIS/FakeISISEventDAE.h    | 20 ++---------
 .../MantidLiveData/ISIS/FakeISISHistoDAE.h    | 29 ++++-----------
 .../LiveData/src/ISIS/FakeISISEventDAE.cpp    | 36 ++++---------------
 .../LiveData/src/ISIS/FakeISISHistoDAE.cpp    | 28 ++++-----------
 .../src/ISIS/ISISLiveEventDataListener.cpp    |  9 +++--
 5 files changed, 27 insertions(+), 95 deletions(-)

diff --git a/Framework/LiveData/inc/MantidLiveData/ISIS/FakeISISEventDAE.h b/Framework/LiveData/inc/MantidLiveData/ISIS/FakeISISEventDAE.h
index 2f648a9df8f..c9cb0496b4b 100644
--- a/Framework/LiveData/inc/MantidLiveData/ISIS/FakeISISEventDAE.h
+++ b/Framework/LiveData/inc/MantidLiveData/ISIS/FakeISISEventDAE.h
@@ -6,23 +6,14 @@
 //----------------------------------------------------------------------
 #include "MantidAPI/Algorithm.h"
 
-#include <mutex>
-
-namespace Poco {
-namespace Net {
-class TCPServer;
-}
-}
-
 namespace Mantid {
 namespace LiveData {
 /**
     Simulates ISIS histogram DAE. It runs continuously until canceled and
-   listens to port 6789 for
-    ISIS DAE commands.
+    listens to port 6789 for ISIS DAE commands.
 
     Copyright &copy; 2008-9 ISIS Rutherford Appleton Laboratory, NScD Oak Ridge
-   National Laboratory & European Spallation Source
+    National Laboratory & European Spallation Source
 
     This file is part of Mantid.
 
@@ -44,9 +35,6 @@ namespace LiveData {
 */
 class FakeISISEventDAE : public API::Algorithm {
 public:
-  FakeISISEventDAE();
-  ~FakeISISEventDAE() override;
-
   /// Algorithm's name for identification overriding a virtual method
   const std::string name() const override { return "FakeISISEventDAE"; }
   /// Algorithm's version for identification overriding a virtual method
@@ -67,10 +55,6 @@ public:
 private:
   void init() override;
   void exec() override;
-  /// Poco TCP server
-  Poco::Net::TCPServer *m_server;
-  /// Mutex
-  std::mutex m_mutex;
 };
 
 } // namespace LiveData
diff --git a/Framework/LiveData/inc/MantidLiveData/ISIS/FakeISISHistoDAE.h b/Framework/LiveData/inc/MantidLiveData/ISIS/FakeISISHistoDAE.h
index 87a7dab447a..84c187739b3 100644
--- a/Framework/LiveData/inc/MantidLiveData/ISIS/FakeISISHistoDAE.h
+++ b/Framework/LiveData/inc/MantidLiveData/ISIS/FakeISISHistoDAE.h
@@ -5,30 +5,22 @@
 // Includes
 //----------------------------------------------------------------------
 #include "MantidAPI/Algorithm.h"
-#include <mutex>
-
-namespace Poco {
-namespace Net {
-class TCPServer;
-}
-}
 
 namespace Mantid {
 namespace LiveData {
 /**
     Simulates ISIS histogram DAE. It runs continuously until canceled and
-   listens to port 6789 for
-    ISIS DAE commands.
+    listens to port 6789 for ISIS DAE commands.
 
     Data is generated starting at 10000 microseconds Time of flight, and each
-   bin requested covers 100 microseconds.
+    bin requested covers 100 microseconds.
     The algorithm silently defines three additional spectra with numbers
-   NSpectra+1, NSpectra+2 and NSpectra+3 in a
+    NSpectra+1, NSpectra+2 and NSpectra+3 in a
     different time regime (they have different binning to the rest of the
-   spectra).
+    spectra).
 
     Copyright &copy; 2008-9 ISIS Rutherford Appleton Laboratory, NScD Oak Ridge
-   National Laboratory & European Spallation Source
+    National Laboratory & European Spallation Source
 
     This file is part of Mantid.
 
@@ -50,13 +42,10 @@ namespace LiveData {
 */
 class DLLExport FakeISISHistoDAE : public API::Algorithm {
 public:
-  FakeISISHistoDAE();
-  ~FakeISISHistoDAE() override;
-
   /// Algorithm's name for identification overriding a virtual method
-  const std::string name() const override { return "FakeISISHistoDAE"; };
+  const std::string name() const override { return "FakeISISHistoDAE"; }
   /// Algorithm's version for identification overriding a virtual method
-  int version() const override { return 1; };
+  int version() const override { return 1; }
   /// Algorithm's category for identification overriding a virtual method
   const std::string category() const override {
     return "DataHandling\\DataAcquisition";
@@ -73,10 +62,6 @@ private:
   // Implement abstract Algorithm methods
   void init() override;
   void exec() override;
-  /// Poco TCP server
-  Poco::Net::TCPServer *m_server;
-  /// Mutex
-  std::mutex m_mutex;
 };
 
 } // namespace LiveData
diff --git a/Framework/LiveData/src/ISIS/FakeISISEventDAE.cpp b/Framework/LiveData/src/ISIS/FakeISISEventDAE.cpp
index 4e22d55816a..b16acedc7e3 100644
--- a/Framework/LiveData/src/ISIS/FakeISISEventDAE.cpp
+++ b/Framework/LiveData/src/ISIS/FakeISISEventDAE.cpp
@@ -7,15 +7,12 @@
 #include "MantidKernel/MersenneTwister.h"
 #include "MantidKernel/Timer.h"
 
+#include <Poco/ActiveResult.h>
 #include <Poco/Net/TCPServer.h>
 #include <Poco/Net/StreamSocket.h>
-#include <Poco/ActiveResult.h>
-#include <Poco/Thread.h>
 
 #include <boost/random/uniform_int.hpp>
 
-#include <numeric>
-
 namespace Mantid {
 namespace LiveData {
 // Register the algorithm into the algorithm factory
@@ -147,17 +144,6 @@ public:
 };
 } // end anonymous
 
-/// (Empty) Constructor
-FakeISISEventDAE::FakeISISEventDAE() : m_server(nullptr) {}
-
-/// Destructor
-FakeISISEventDAE::~FakeISISEventDAE() {
-  if (m_server) {
-    m_server->stop();
-    delete m_server;
-  }
-}
-
 /**
 * Declare the algorithm properties
 */
@@ -198,19 +184,18 @@ void FakeISISEventDAE::exec() {
   histoDAE->setProperty("NPeriods", nper);
   histoDAE->setProperty("NSpectra", nspec);
   histoDAE->setProperty("Port", port + 1);
-  Poco::ActiveResult<bool> histoDAEHandle = histoDAE->executeAsync();
+  auto histoDAEHandle = histoDAE->executeAsync();
 
   auto prog = boost::make_shared<Progress>(this, 0.0, 1.0, 100);
   prog->setNotifyStep(0);
   prog->report(0, "Waiting for client");
-  std::lock_guard<std::mutex> lock(m_mutex);
   Poco::Net::ServerSocket socket(static_cast<Poco::UInt16>(port));
   socket.listen();
-  m_server = new Poco::Net::TCPServer(
+  Poco::Net::TCPServer server(
       TestServerConnectionFactory::Ptr(
           new TestServerConnectionFactory(nper, nspec, rate, nevents, prog)),
       socket);
-  m_server->start();
+  server.start();
   // Keep going until you get cancelled
   while (true) {
     try {
@@ -223,19 +208,12 @@ void FakeISISEventDAE::exec() {
     // Sleep for 50 msec
     Poco::Thread::sleep(50);
   }
+  // It's most likely that we got here from a cancel request
+  // so calling prog->report after this point
+  // will generate another CancelException
   histoDAE->cancel();
   histoDAEHandle.wait();
-  if (m_server) {
-    m_server->stop();
-    m_server = nullptr;
-  }
   socket.close();
-
-  prog->report(90, "Closing ISIS event DAE");
-  histoDAE->setLogging(false); // hide the final closedown message to the log it
-                               // is confusing as it is a child alg.
-  histoDAE->cancel();
-  histoDAEHandle.wait();
 }
 
 } // namespace LiveData
diff --git a/Framework/LiveData/src/ISIS/FakeISISHistoDAE.cpp b/Framework/LiveData/src/ISIS/FakeISISHistoDAE.cpp
index bb9b11d2eea..bea46be21c0 100644
--- a/Framework/LiveData/src/ISIS/FakeISISHistoDAE.cpp
+++ b/Framework/LiveData/src/ISIS/FakeISISHistoDAE.cpp
@@ -2,11 +2,9 @@
 // Includes
 //----------------------------------------------------------------------
 #include "MantidLiveData/ISIS/FakeISISHistoDAE.h"
-#include <numeric>
 
 #include <Poco/Net/TCPServer.h>
 #include <Poco/Net/StreamSocket.h>
-#include <Poco/Thread.h>
 
 namespace Mantid {
 namespace LiveData {
@@ -314,17 +312,6 @@ public:
 using namespace Kernel;
 using namespace API;
 
-/// (Empty) Constructor
-FakeISISHistoDAE::FakeISISHistoDAE() : m_server(nullptr) {}
-
-/// Destructor
-FakeISISHistoDAE::~FakeISISHistoDAE() {
-  if (m_server) {
-    m_server->stop();
-    delete m_server;
-  }
-}
-
 /**
  * Declare the algorithm properties
  */
@@ -352,16 +339,15 @@ void FakeISISHistoDAE::exec() {
   int nbins = getProperty("NBins");
   int port = getProperty("Port");
 
-  std::lock_guard<std::mutex> lock(m_mutex);
   Poco::Net::ServerSocket socket(static_cast<Poco::UInt16>(port));
   socket.listen();
 
-  m_server = new Poco::Net::TCPServer(
+  Poco::Net::TCPServer server(
       TestServerConnectionFactory::Ptr(
           new TestServerConnectionFactory(nper, nspec, nbins)),
       socket);
-  m_server->start();
-  // Keep going until you get cancelled
+  server.start();
+  // Keep going until you get cancelled or an error occurs
   while (true) {
     try {
       // Exit if the user presses cancel
@@ -374,10 +360,10 @@ void FakeISISHistoDAE::exec() {
     // Sleep for 50 msec
     Poco::Thread::sleep(50);
   }
-  if (m_server) {
-    m_server->stop();
-    m_server = nullptr;
-  }
+  // It's most likely that we got here from a cancel request
+  // so calling prog->report after this point
+  // will generate another CancelException
+  server.stop();
   socket.close();
 }
 
diff --git a/Framework/LiveData/src/ISIS/ISISLiveEventDataListener.cpp b/Framework/LiveData/src/ISIS/ISISLiveEventDataListener.cpp
index 1d1610f83b6..53c620436f3 100644
--- a/Framework/LiveData/src/ISIS/ISISLiveEventDataListener.cpp
+++ b/Framework/LiveData/src/ISIS/ISISLiveEventDataListener.cpp
@@ -286,8 +286,7 @@ void ISISLiveEventDataListener::run() {
       saveEvents(events.data, pulseTime, events.head_n.period);
     }
 
-  } catch (std::runtime_error &
-               e) { // exception handler for generic runtime exceptions
+  } catch (std::runtime_error &e) {
 
     g_log.error() << "Caught a runtime exception.\nException message: "
                   << e.what() << '\n';
@@ -295,8 +294,8 @@ void ISISLiveEventDataListener::run() {
 
     m_backgroundException = boost::make_shared<std::runtime_error>(e);
 
-  } catch (std::invalid_argument &
-               e) { // TimeSeriesProperty (and possibly some other things) can
+  } catch (std::invalid_argument &e) {
+    // TimeSeriesProperty (and possibly some other things) can
     // can throw these errors
     g_log.error()
         << "Caught an invalid argument exception.\nException message: "
@@ -307,7 +306,7 @@ void ISISLiveEventDataListener::run() {
     newMsg += e.what();
     m_backgroundException = boost::make_shared<std::runtime_error>(newMsg);
 
-  } catch (...) { // Default exception handler
+  } catch (...) {
     g_log.error() << "Uncaught exception in ISISLiveEventDataListener network "
                      "read thread.\n";
     m_isConnected = false;
-- 
GitLab