Merge pull request #495 from azeey/13_to_14

Merge gz-transport13 ➡️  main

Changelog.md

@@ -4,7 +4,10 @@
 
 ## Gazebo Transport 13.X
 
-### Gazebo Transport 13.2.0 (2024-xx-xx)
+### Gazebo Transport 13.2.0 (2024-04-09)
+
+1. Use relative install path for gz tool data
+ * [Pull request #492](https://github.com/gazebosim/gz-transport/pull/492)
 
 1. No input service request from the command line
  * [Pull request #487](https://github.com/gazebosim/gz-transport/pull/487)

conf/CMakeLists.txt

@@ -24,4 +24,4 @@ configure_file(
 
 # Install the yaml configuration files in an unversioned location.
 install(FILES ${CMAKE_CURRENT_BINARY_DIR}/${GZ_DESIGNATION}${PROJECT_VERSION_MAJOR}.yaml
- DESTINATION ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_DATAROOTDIR}/gz/)
+ DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/gz/)

include/gz/transport/NodeShared.hh

@@ -68,14 +68,8 @@ namespace gz
  {
  /// \brief NodeShared is a singleton. This method gets the
  /// NodeShared instance shared between all the nodes.
- /// Note: This is deprecated. Please use \sa SharedInstance
  /// \return Pointer to the current NodeShared instance.
- public: static NodeShared GZ_DEPRECATED(13) *Instance();
-
- /// \brief NodeShared is a singleton. This method gets the
- /// a reference counted NodeShared instance shared between all the nodes.
- /// \return A shared_ptr to the current NodeShared instance.
- public: static std::shared_ptr<NodeShared> SharedInstance();
+ public: static NodeShared *Instance();
 
  /// \brief Receive data and control messages.
  public: void RunReceptionTask();

log/src/Recorder.cc

@@ -188,7 +188,7 @@ Recorder::Implementation::Implementation()
  this->OnMessageReceived(_data, _len, _info);
  };
 
- auto shared = NodeShared::SharedInstance();
+ auto shared = NodeShared::Instance();
 
  this->discovery = std::make_unique<MsgDiscovery>(
  Uuid().ToString(), shared->discoveryIP, shared->msgDiscPort);

log/src/cmd/CMakeLists.txt

@@ -57,4 +57,4 @@ configure_file(
  "transportlog.yaml.in"
  ${transportlog_configured})
 
-install(FILES ${transportlog_configured} DESTINATION ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_DATAROOTDIR}/gz/)
+install(FILES ${transportlog_configured} DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/gz/)

parameters/src/cmd/CMakeLists.txt

@@ -57,4 +57,4 @@ configure_file(
  "transportparam.yaml.in"
  ${transportparam_configured})
 
-install(FILES ${transportparam_configured} DESTINATION ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_DATAROOTDIR}/gz/)
+install(FILES ${transportparam_configured} DESTINATION ${CMAKE_INSTALL_DATAROOTDIR}/gz/)

src/Node.cc

@@ -77,13 +77,13 @@ namespace gz
  //////////////////////////////////////////////////
  int rcvHwm()
  {
- return NodeShared::SharedInstance()->RcvHwm();
+ return NodeShared::Instance()->RcvHwm();
  }
 
  //////////////////////////////////////////////////
  int sndHwm()
  {
- return NodeShared::SharedInstance()->SndHwm();
+ return NodeShared::Instance()->SndHwm();
  }
 
  //////////////////////////////////////////////////
@@ -104,14 +104,14 @@ namespace gz
  {
  /// \brief Default constructor.
  public: PublisherPrivate()
- : shared(NodeShared::SharedInstance())
+ : shared(NodeShared::Instance())
  {
  }
 
  /// \brief Constructor
  /// \param[in] _publisher The message publisher.
  public: explicit PublisherPrivate(const MessagePublisher &_publisher)
- : shared(NodeShared::SharedInstance()),
+ : shared(NodeShared::Instance()),
  publisher(_publisher)
  {
  }
@@ -189,7 +189,7 @@ namespace gz
 
  /// \brief Pointer to the object shared between all the nodes within the
  /// same process.
- public: std::shared_ptr<NodeShared> shared = nullptr;
+ public: NodeShared *shared = nullptr;
 
  /// \brief The message publisher.
  public: MessagePublisher publisher;
@@ -864,7 +864,7 @@ bool Node::EnableStats(const std::string &_topic, bool _enable,
 //////////////////////////////////////////////////
 NodeShared *Node::Shared() const
 {
- return this->dataPtr->shared.get();
+ return this->dataPtr->shared;
 }
 
 //////////////////////////////////////////////////

src/NodePrivate.hh

@@ -18,7 +18,6 @@
 #ifndef GZ_TRANSPORT_NODEPRIVATE_HH_
 #define GZ_TRANSPORT_NODEPRIVATE_HH_
 
-#include <memory>
 #include <string>
 #include <unordered_set>
 
@@ -68,7 +67,7 @@ namespace gz
 
  /// \brief Pointer to the object shared between all the nodes within the
  /// same process.
- public: std::shared_ptr<NodeShared> shared = NodeShared::SharedInstance();
+ public: NodeShared *shared = NodeShared::Instance();
 
  /// \brief Partition for this node.
  public: std::string partition = hostname() + ":" + username();

src/NodeShared.cc

@@ -152,47 +152,47 @@ void sendAuthErrorHelper(zmq::socket_t &_socket, const std::string &_err)
 }
 
 //////////////////////////////////////////////////
-// LCOV_EXCL_START
 NodeShared *NodeShared::Instance()
-{
- // This is a deprecated function, but since it's public, the following ensures
- // backward compatibility by instantiating a shared_ptr that never gets
- // deleted.
- static std::shared_ptr<NodeShared> nodeShared = NodeShared::SharedInstance();
- return nodeShared.get();
-}
-// LCOV_EXCL_STOP
-
-//////////////////////////////////////////////////
-std::shared_ptr<NodeShared> NodeShared::SharedInstance()
 {
  // Create an instance of NodeShared per process so the ZMQ context
  // is not shared between different processes.
- static std::weak_ptr<NodeShared> nodeSharedWeak;
- static std::mutex mutex;
+
+ static std::shared_mutex mutex;
+ static std::unordered_map<unsigned int, NodeShared*> nodeSharedMap;
+
+ // Get current process ID.
+ auto pid = getProcessId();
 
  // Check if there's already a NodeShared instance for this process.
- std::shared_ptr<NodeShared> nodeShared = nodeSharedWeak.lock();
- if (nodeShared)
- return nodeShared;
-
- // Multiple threads from the same process could have arrived here
- // simultaneously, so after locking, we need to make sure that there's
- // not an already constructed NodeShared instance for this process.
- std::lock_guard lock(mutex);
- nodeShared = nodeSharedWeak.lock();
- if (nodeShared)
- return nodeShared;
-
- // Class used to enable use of std::shared_ptr. This is needed because the
- // constructor and destructor of NodeShared are protected.
- class MakeSharedEnabler : public NodeShared {};
- // No instance, construct a new one.
- nodeShared = std::make_shared<MakeSharedEnabler>();
- // Assign to weak_ptr so next time SharedInstance is called, we can return the
- // instance we just created.
- nodeSharedWeak = nodeShared;
- return nodeShared;
+ // Use a shared_lock so multiple threads can read simultaneously.
+ // This will only block if there's another thread locking exclusively
+ // for writing. Since most of the time threads will be reading,
+ // we make the read operation faster at the expense of making the write
+ // operation slower. Use exceptions for their zero-cost when successful.
+ try
+ {
+ std::shared_lock readLock(mutex);
+ return nodeSharedMap.at(pid);
+ }
+ catch (...)
+ {
+ // Multiple threads from the same process could have arrived here
+ // simultaneously, so after locking, we need to make sure that there's
+ // not an already constructed NodeShared instance for this process.
+ std::lock_guard writeLock(mutex);
+
+ auto iter = nodeSharedMap.find(pid);
+ if (iter != nodeSharedMap.end())
+ {
+ // There's already an instance for this process, return it.
+ return iter->second;
+ }
+
+ // No instance, construct a new one.
+ auto ret = nodeSharedMap.insert({pid, new NodeShared});
+ assert(ret.second); // Insert operation should be successful.
+ return ret.first->second;
+ }
 }
 
 //////////////////////////////////////////////////

src/cmd/CMakeLists.txt

@@ -127,4 +127,4 @@ install(
  FILES
  ${CMAKE_CURRENT_BINARY_DIR}/transport${PROJECT_VERSION_MAJOR}.bash_completion.sh
  DESTINATION
- ${CMAKE_INSTALL_PREFIX}/${CMAKE_INSTALL_DATAROOTDIR}/gz/gz${GZ_TOOLS_VER}.completion.d)
+ ${CMAKE_INSTALL_DATAROOTDIR}/gz/gz${GZ_TOOLS_VER}.completion.d)