MediaChanagerFacade now instantiates each media changer proxy only once

mediachanger/AcsProxyZmqSingleton.cpp

+/*
+ * The CERN Tape Archive (CTA) project
+ * Copyright (C) 2015  CERN
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "common/exception/Exception.hpp"
+#include "common/utils/utils.hpp"
+#include "mediachanger/AcsProxyZmqSingleton.hpp"
+
+namespace cta {
+namespace mediachanger {
+
+//------------------------------------------------------------------------------
+// s_mutex
+//------------------------------------------------------------------------------
+std::mutex AcsProxyZmqSingleton::s_mutex;
+
+//------------------------------------------------------------------------------
+// s_instance
+//------------------------------------------------------------------------------
+std::unique_ptr<AcsProxyZmqSingleton> AcsProxyZmqSingleton::s_instance;
+
+//------------------------------------------------------------------------------
+// instance
+//------------------------------------------------------------------------------
+AcsProxyZmqSingleton &AcsProxyZmqSingleton::instance(void *const zmqContext) {
+  try {
+    std::lock_guard<std::mutex> lock(s_mutex);
+
+    if(nullptr == s_instance) {
+      s_instance.reset(new AcsProxyZmqSingleton(zmqContext));
+    }
+    return *s_instance;
+  } catch(exception::Exception &ex) {
+    throw exception::Exception(std::string(__FUNCTION__) + " failed: " + ex.getMessage().str());
+  }
+}
+
+//------------------------------------------------------------------------------
+// constructor
+//------------------------------------------------------------------------------
+AcsProxyZmqSingleton::AcsProxyZmqSingleton(void *const zmqContext, const unsigned short serverPort) throw():
+  AcsProxyZmq(zmqContext, serverPort) {
+}
+
+} // namespace mediachanger
+} // namespace cta

mediachanger/AcsProxyZmqSingleton.hpp

+/*
+ * The CERN Tape Archive(CTA) project
+ * Copyright(C) 2015  CERN
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ *(at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "mediachanger/Constants.hpp"
+#include "mediachanger/AcsProxyZmq.hpp"
+
+#include <memory>
+#include <mutex>
+
+namespace cta {
+namespace mediachanger {
+
+/**
+ * A singleton class.
+ */
+class AcsProxyZmqSingleton: public AcsProxyZmq {
+public:
+
+  /**
+   * Delete the default constructor.
+   */
+  AcsProxyZmqSingleton() = delete;
+
+  /**
+   * Delete the copy constructor.
+   */
+  AcsProxyZmqSingleton(const AcsProxyZmqSingleton&) = delete;
+
+  /**
+   * Delete the move constructor.
+   */
+  AcsProxyZmqSingleton(AcsProxyZmqSingleton&&) = delete;
+
+  /**
+   * Delete the copy assignment oprator.
+   */
+  AcsProxyZmqSingleton &operator=(const AcsProxyZmqSingleton&) = delete;
+
+  /**
+   * Delete the move assignment oprator.
+   */
+  AcsProxyZmqSingleton &operator=(AcsProxyZmqSingleton&&) = delete;
+
+  /**
+   * Returns the singleton instance.
+   *
+   * @param zmqContext The ZMQ context to be used to construct the instance if
+   * the instance does not already exist.
+   * @return the singleton instance.
+   */
+  static AcsProxyZmqSingleton &instance(void *const zmqContext);
+
+private:
+
+  /**
+   * Provate constructor to only be used by the instance() method.
+   *
+   * @param zmqContext The ZMQ context.
+   * @param serverPort The TCP/IP port on which the CASTOR ACS daemon is
+   * listening for ZMQ messages.
+   */
+  AcsProxyZmqSingleton(void *const zmqContext, const unsigned short serverPort = ACS_PORT) throw();
+
+  /**
+   * Mutex used to implement a critical region around the implementation of the
+   * instance() method.
+   */
+  static std::mutex s_mutex;
+
+  /**
+   * The single instance.
+   */
+  static std::unique_ptr<AcsProxyZmqSingleton> s_instance;
+
+}; // class AcsProxyZmqSingleton
+
+} // namespace mediachanger
+} // namespace cta

mediachanger/CMakeLists.txt

@@ -25,6 +25,7 @@ PROTOBUF_GENERATE_CPP(ProtoSources ProtoHeaders ${ProtoFiles})
 set (MEDIACHANGER_LIB_SRC_FILES
   AcsLibrarySlot.cpp
   AcsProxyZmq.cpp
+  AcsProxyZmqSingleton.cpp
   CmdLine.cpp
   CmdLineTool.cpp
   CommonMarshal.cpp

mediachanger/MediaChangerFacade.cpp

@@ -17,12 +17,8 @@
  */
 
 #include "common/exception/Exception.hpp"
-#include "common/make_unique.hpp"
-#include "mediachanger/AcsProxyZmq.hpp"
-#include "mediachanger/Constants.hpp"
+#include "mediachanger/AcsProxyZmqSingleton.hpp"
 #include "mediachanger/MediaChangerFacade.hpp"
-#include "mediachanger/MmcProxyLog.hpp"
-#include "mediachanger/RmcProxyTcpIp.hpp"
 
 namespace cta {
 namespace mediachanger {
@@ -31,8 +27,8 @@ namespace mediachanger {
 // constructor
 //------------------------------------------------------------------------------
 MediaChangerFacade::MediaChangerFacade(log::Logger &log, void *const zmqContext) throw():
-  m_log(log),
-  m_zmqContext(zmqContext) {
+  m_zmqContext(zmqContext),
+  m_mmcProxy(log) {
 }
 
 //------------------------------------------------------------------------------
@@ -40,7 +36,7 @@ MediaChangerFacade::MediaChangerFacade(log::Logger &log, void *const zmqContext)
 //------------------------------------------------------------------------------
 void MediaChangerFacade::mountTapeReadOnly(const std::string &vid, const LibrarySlot &slot) {
   try {
-    return createMediaChangerProxy(slot.getLibraryType())->mountTapeReadOnly(vid, slot);
+    return getProxy(slot.getLibraryType()).mountTapeReadOnly(vid, slot);
   } catch(cta::exception::Exception &ne) {
     cta::exception::Exception ex;
     ex.getMessage() << "Failed to mount tape for read-only access: vid=" << vid << " slot=" << slot.str() << ": " <<
@@ -54,7 +50,7 @@ void MediaChangerFacade::mountTapeReadOnly(const std::string &vid, const Library
 //------------------------------------------------------------------------------
 void MediaChangerFacade::mountTapeReadWrite(const std::string &vid, const LibrarySlot &slot) {
   try {
-    return createMediaChangerProxy(slot.getLibraryType())->mountTapeReadWrite(vid, slot);
+    return getProxy(slot.getLibraryType()).mountTapeReadWrite(vid, slot);
   } catch(cta::exception::Exception &ne) {
     cta::exception::Exception ex;
     ex.getMessage() << "Failed to mount tape for read/write access: vid=" << vid << " slot=" << slot.str() << ": " <<
@@ -68,7 +64,7 @@ void MediaChangerFacade::mountTapeReadWrite(const std::string &vid, const Librar
 //------------------------------------------------------------------------------
 void MediaChangerFacade::dismountTape(const std::string &vid, const LibrarySlot &slot) {
   try {
-    return createMediaChangerProxy(slot.getLibraryType())->dismountTape(vid, slot);
+    return getProxy(slot.getLibraryType()).dismountTape(vid, slot);
   } catch(cta::exception::Exception &ne) {
     cta::exception::Exception ex;
     ex.getMessage() << "Failed to dismount tape: vid=" << vid << " slot=" << slot.str() << ": " <<
@@ -82,7 +78,7 @@ void MediaChangerFacade::dismountTape(const std::string &vid, const LibrarySlot
 //------------------------------------------------------------------------------
 void MediaChangerFacade::forceDismountTape(const std::string &vid, const LibrarySlot &slot) {
   try {
-    return createMediaChangerProxy(slot.getLibraryType())->forceDismountTape(vid, slot);
+    return getProxy(slot.getLibraryType()).forceDismountTape(vid, slot);
   } catch(cta::exception::Exception &ne) {
     cta::exception::Exception ex;
     ex.getMessage() << "Failed to force dismount tape: vid=" << vid << " slot=" << slot.str() << ": " <<
@@ -92,17 +88,22 @@ void MediaChangerFacade::forceDismountTape(const std::string &vid, const Library
 }
 
 //------------------------------------------------------------------------------
-// createMediaChangerProxy
+// getProxy
 //------------------------------------------------------------------------------
-std::unique_ptr<MediaChangerProxy> MediaChangerFacade::createMediaChangerProxy(const TapeLibraryType libraryType) {
+MediaChangerProxy &MediaChangerFacade::getProxy(const TapeLibraryType libraryType) {
   try {
     switch(libraryType) {
     case TAPE_LIBRARY_TYPE_ACS:
-      return make_unique<AcsProxyZmq>(m_zmqContext);
+      // Using AcsProxyZmqSingleton instead of simply having a AcsProxyZmq
+      // member variable (e.g. m_acsProxy) in order to only instantiate an
+      // AcsProxyZmq object if necessary.  Instantiating such as object
+      // results in a one off memory leak related to ZMQ that would cause
+      // valgrind to fail on simple CTA unit-tests.
+      return AcsProxyZmqSingleton::instance(m_zmqContext);
     case TAPE_LIBRARY_TYPE_MANUAL:
-      return make_unique<MmcProxyLog>(m_log);
+      return m_mmcProxy;
     case TAPE_LIBRARY_TYPE_SCSI:
-      return make_unique<RmcProxyTcpIp>();
+      return m_rmcProxy;
     default:
       // Should never get here
       throw exception::Exception("Library slot has an unexpected library type");

mediachanger/MediaChangerFacade.hpp

@@ -21,6 +21,8 @@
 #include "common/log/Logger.hpp"
 #include "mediachanger/LibrarySlot.hpp"
 #include "mediachanger/MediaChangerProxy.hpp"
+#include "mediachanger/MmcProxyLog.hpp"
+#include "mediachanger/RmcProxyTcpIp.hpp"
 #include "mediachanger/ZmqContextSingleton.hpp"
 
 #include <memory>
@@ -38,10 +40,13 @@ public:
   /**
    * Constructor.
    *
-   * @param log Object representing the API to the CTA logging system.
+   * @param log Object representing the API to the CTA logging system.  This log
+   * object will be used by the manual media changer to communicate with the
+   * tape operator.
    * @param zmqContext The ZMQ context.  There is usually one ZMQ context within
    * a program.  Set this parameter in order for the MediaChangerFacade to share
-   * an already existing ZMQ context.
+   * an already existing ZMQ context.  If this parameter is not set then the
+   * ZmqContextSingleton of the mediachanger namespace will be used.
    */
   MediaChangerFacade(log::Logger &log, void *const zmqContext = ZmqContextSingleton::instance()) throw();
 
@@ -91,22 +96,24 @@ public:
 private:
 
   /**
-   * Object representing the API to the CTA logging system.
+   * The ZMQ context.
    */
-  log::Logger &m_log;
+  void *m_zmqContext;
 
   /**
-   * The ZMQ context.
+   * Manual media changer proxy.
    */
-  void *m_zmqContext;
+  MmcProxyLog m_mmcProxy;
 
   /**
-   * Factory method that creates a media changer proxy object based on the
-   * specified tape library type.
-   *
-   * @param libraryType The type of tape library.
+   * SCSI media changer proxy.
+   */
+  RmcProxyTcpIp m_rmcProxy;
+
+  /**
+   * Returns the media changer proxy for the specified library type.
    */
-  std::unique_ptr<MediaChangerProxy> createMediaChangerProxy(const TapeLibraryType libraryType);
+  MediaChangerProxy &getProxy(const TapeLibraryType libraryType);
 
 }; // class MediaChangerFacade