From ab8fa6b9698ac56ab995c12704e0224e8627e5a8 Mon Sep 17 00:00:00 2001 From: rex-schilasky <49162693+rex-schilasky@users.noreply.github.com> Date: Mon, 18 Mar 2024 17:21:54 +0100 Subject: [PATCH] data race in reader, writer, client, server create/register/unregister logic --- ecal/core/src/readwrite/ecal_reader.cpp | 9 ++++++--- ecal/core/src/readwrite/ecal_writer.cpp | 10 ++++++---- ecal/core/src/readwrite/ecal_writer.h | 2 +- ecal/core/src/service/ecal_service_client_impl.cpp | 11 ++++++----- ecal/core/src/service/ecal_service_client_impl.h | 2 +- ecal/core/src/service/ecal_service_server_impl.cpp | 10 ++++++---- ecal/core/src/service/ecal_service_server_impl.h | 2 +- 7 files changed, 27 insertions(+), 19 deletions(-) diff --git a/ecal/core/src/readwrite/ecal_reader.cpp b/ecal/core/src/readwrite/ecal_reader.cpp index 5d1985f7bb..523740b488 100644 --- a/ecal/core/src/readwrite/ecal_reader.cpp +++ b/ecal/core/src/readwrite/ecal_reader.cpp @@ -125,7 +125,7 @@ namespace eCAL // register Register(false); - // mark as created + // and mark as created m_created = true; return(true); @@ -155,11 +155,13 @@ namespace eCAL m_event_callback_map.clear(); } - // unregister + // mark as no more created + m_created = false; + + // and unregister Unregister(); // reset defaults - m_created = false; m_clock = 0; m_message_drops = 0; @@ -230,6 +232,7 @@ namespace eCAL bool CDataReader::Register(const bool force_) { #if ECAL_CORE_REGISTRATION + if (!m_created) return(false); if(m_topic_name.empty()) return(false); // create command parameter diff --git a/ecal/core/src/readwrite/ecal_writer.cpp b/ecal/core/src/readwrite/ecal_writer.cpp index d47529e6c3..162a515d02 100644 --- a/ecal/core/src/readwrite/ecal_writer.cpp +++ b/ecal/core/src/readwrite/ecal_writer.cpp @@ -142,7 +142,7 @@ namespace eCAL // register Register(false); - // mark as created + // and mark as created m_created = true; // create udp multicast layer @@ -210,11 +210,12 @@ namespace eCAL m_event_callback_map.clear(); } - // unregister - Unregister(); - + // mark as no more created m_created = false; + // and unregister + Unregister(); + return(true); } @@ -771,6 +772,7 @@ namespace eCAL bool CDataWriter::Register(bool force_) { #if ECAL_CORE_REGISTRATION + if (!m_created) return(false); if (m_topic_name.empty()) return(false); //@Rex: why is the logic different in CDataReader??? diff --git a/ecal/core/src/readwrite/ecal_writer.h b/ecal/core/src/readwrite/ecal_writer.h index e0fd19afee..03893c33cf 100644 --- a/ecal/core/src/readwrite/ecal_writer.h +++ b/ecal/core/src/readwrite/ecal_writer.h @@ -217,6 +217,6 @@ namespace eCAL bool m_use_tdesc; int m_share_ttype; int m_share_tdesc; - bool m_created; + std::atomic m_created; }; } diff --git a/ecal/core/src/service/ecal_service_client_impl.cpp b/ecal/core/src/service/ecal_service_client_impl.cpp index ee7076c20b..7e8abb3be1 100644 --- a/ecal/core/src/service/ecal_service_client_impl.cpp +++ b/ecal/core/src/service/ecal_service_client_impl.cpp @@ -80,7 +80,7 @@ namespace eCAL // register this client Register(false); - // mark as created + // and mark as created m_created = true; return(true); @@ -108,7 +108,10 @@ namespace eCAL m_event_callback_map.clear(); } - // unregister this client + // mark as no more created + m_created = false; + + // and unregister this client Unregister(); // reset internals @@ -116,9 +119,6 @@ namespace eCAL m_service_id.clear(); m_host_name.clear(); - // mark as not created - m_created = false; - return(true); } @@ -616,6 +616,7 @@ namespace eCAL void CServiceClientImpl::Register(const bool force_) { + if (!m_created) return; if (m_service_name.empty()) return; Registration::Sample sample; diff --git a/ecal/core/src/service/ecal_service_client_impl.h b/ecal/core/src/service/ecal_service_client_impl.h index 4211c48ccb..d2bb9c60c1 100644 --- a/ecal/core/src/service/ecal_service_client_impl.h +++ b/ecal/core/src/service/ecal_service_client_impl.h @@ -123,6 +123,6 @@ namespace eCAL std::string m_service_id; std::string m_host_name; - bool m_created; + std::atomic m_created; }; } diff --git a/ecal/core/src/service/ecal_service_server_impl.cpp b/ecal/core/src/service/ecal_service_server_impl.cpp index d91e54bef5..daae9b5ac8 100644 --- a/ecal/core/src/service/ecal_service_server_impl.cpp +++ b/ecal/core/src/service/ecal_service_server_impl.cpp @@ -148,7 +148,7 @@ namespace eCAL // register this service Register(false); - // mark as created + // and mark as created m_created = true; return(true); @@ -176,7 +176,10 @@ namespace eCAL m_event_callback_map.clear(); } - // unregister this service + // mark as no more created + m_created = false; + + // and unregister this service Unregister(); // reset internals @@ -189,8 +192,6 @@ namespace eCAL m_connected_v1 = false; } - m_created = false; - return(true); } @@ -342,6 +343,7 @@ namespace eCAL void CServiceServerImpl::Register(const bool force_) { + if (!m_created) return; if (m_service_name.empty()) return; // might be zero in contruction phase diff --git a/ecal/core/src/service/ecal_service_server_impl.h b/ecal/core/src/service/ecal_service_server_impl.h index 5b35fcbef6..5d998bd599 100644 --- a/ecal/core/src/service/ecal_service_server_impl.h +++ b/ecal/core/src/service/ecal_service_server_impl.h @@ -121,7 +121,7 @@ namespace eCAL std::mutex m_event_callback_map_sync; EventCallbackMapT m_event_callback_map; - bool m_created = false; + std::atomic m_created = false; mutable std::mutex m_connected_mutex; //!< mutex protecting the m_connected_v0 and m_connected_v1 variable, as those are modified by the event callbacks in another thread. bool m_connected_v0 = false;