diff --git a/Changelog.md b/Changelog.md index 19aad808b..35574e6fc 100644 --- a/Changelog.md +++ b/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) diff --git a/conf/CMakeLists.txt b/conf/CMakeLists.txt index 6f5089fca..94b36d3a8 100644 --- a/conf/CMakeLists.txt +++ b/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/) diff --git a/include/gz/transport/NodeShared.hh b/include/gz/transport/NodeShared.hh index f4d3d5e4f..56df051dc 100644 --- a/include/gz/transport/NodeShared.hh +++ b/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 SharedInstance(); + public: static NodeShared *Instance(); /// \brief Receive data and control messages. public: void RunReceptionTask(); diff --git a/log/src/Recorder.cc b/log/src/Recorder.cc index bb95fe86b..11b83040d 100644 --- a/log/src/Recorder.cc +++ b/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( Uuid().ToString(), shared->discoveryIP, shared->msgDiscPort); diff --git a/log/src/cmd/CMakeLists.txt b/log/src/cmd/CMakeLists.txt index f410bcc10..aeafdaaed 100644 --- a/log/src/cmd/CMakeLists.txt +++ b/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/) diff --git a/parameters/src/cmd/CMakeLists.txt b/parameters/src/cmd/CMakeLists.txt index aa4e8b10f..226bb09cf 100644 --- a/parameters/src/cmd/CMakeLists.txt +++ b/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/) diff --git a/src/Node.cc b/src/Node.cc index ec6116d2c..4dbc29598 100644 --- a/src/Node.cc +++ b/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 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; } ////////////////////////////////////////////////// diff --git a/src/NodePrivate.hh b/src/NodePrivate.hh index 9fd96eeca..181140552 100644 --- a/src/NodePrivate.hh +++ b/src/NodePrivate.hh @@ -18,7 +18,6 @@ #ifndef GZ_TRANSPORT_NODEPRIVATE_HH_ #define GZ_TRANSPORT_NODEPRIVATE_HH_ -#include #include #include @@ -68,7 +67,7 @@ namespace gz /// \brief Pointer to the object shared between all the nodes within the /// same process. - public: std::shared_ptr shared = NodeShared::SharedInstance(); + public: NodeShared *shared = NodeShared::Instance(); /// \brief Partition for this node. public: std::string partition = hostname() + ":" + username(); diff --git a/src/NodeShared.cc b/src/NodeShared.cc index 8c822a504..ce9e8a0c8 100644 --- a/src/NodeShared.cc +++ b/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::SharedInstance(); - return nodeShared.get(); -} -// LCOV_EXCL_STOP - -////////////////////////////////////////////////// -std::shared_ptr NodeShared::SharedInstance() { // Create an instance of NodeShared per process so the ZMQ context // is not shared between different processes. - static std::weak_ptr nodeSharedWeak; - static std::mutex mutex; + + static std::shared_mutex mutex; + static std::unordered_map nodeSharedMap; + + // Get current process ID. + auto pid = getProcessId(); // Check if there's already a NodeShared instance for this process. - std::shared_ptr 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(); - // 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; + } } ////////////////////////////////////////////////// diff --git a/src/cmd/CMakeLists.txt b/src/cmd/CMakeLists.txt index 2822d98df..3393a9cc0 100644 --- a/src/cmd/CMakeLists.txt +++ b/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)