Skip to content

Commit

Permalink
[core] register, unregister logic adapted to prevent data races (#1481)
Browse files Browse the repository at this point in the history
  • Loading branch information
rex-schilasky authored Mar 21, 2024
1 parent f63238c commit 0c282f2
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 32 deletions.
17 changes: 9 additions & 8 deletions ecal/core/src/readwrite/ecal_reader.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,12 +118,12 @@ namespace eCAL
// start transport layers
SubscribeToLayers();

// register
Register(false);

// mark as created
m_created = true;

// register
Register(false);

return(true);
}

Expand Down Expand Up @@ -151,11 +151,13 @@ namespace eCAL
m_event_callback_map.clear();
}

// mark as no more created (and prevent reregistering)
m_created = false;

// unregister
Unregister();

// reset defaults
m_created = false;
m_clock = 0;
m_message_drops = 0;

Expand Down Expand Up @@ -214,7 +216,8 @@ namespace eCAL

bool CDataReader::Register(const bool force_)
{
if(m_topic_name.empty()) return(false);
if (!m_created) return(false);
if (m_topic_name.empty()) return(false);

// create command parameter
eCAL::pb::Sample ecal_reg_sample;
Expand Down Expand Up @@ -914,11 +917,9 @@ namespace eCAL

void CDataReader::RefreshRegistration()
{
// this function will be called finally from registration provider every second (by default settings)
if(!m_created) return;

// ensure that registration is not called within zero nanoseconds
// normally it will be called from registration logic every second

// register without send
Register(false);

Expand Down
1 change: 1 addition & 0 deletions ecal/core/src/readwrite/ecal_reader.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@

#pragma once

#include <atomic>
#include <chrono>
#include <cstddef>
#include <deque>
Expand Down
13 changes: 8 additions & 5 deletions ecal/core/src/readwrite/ecal_writer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -137,12 +137,12 @@ namespace eCAL
// allow to share topic description
m_use_tdesc = Config::IsTopicDescriptionSharingEnabled();

// register
Register(false);

// mark as created
m_created = true;

// register
Register(false);

// create udp multicast layer
SetUseUdpMC(m_writer.udp_mc_mode.requested);

Expand Down Expand Up @@ -206,11 +206,12 @@ namespace eCAL
m_event_callback_map.clear();
}

// mark as no more created (and prevent reregistering)
m_created = false;

// unregister
Unregister();

m_created = false;

return(true);
}

Expand Down Expand Up @@ -739,6 +740,7 @@ namespace eCAL

void CDataWriter::RefreshRegistration()
{
// this function will be called finally from registration provider every second (by default settings)
if (!m_created) return;

// register without send
Expand Down Expand Up @@ -798,6 +800,7 @@ namespace eCAL

bool CDataWriter::Register(bool force_)
{
if (!m_created) return(false);
if (m_topic_name.empty()) return(false);

//@Rex: why is the logic different in CDataReader???
Expand Down
2 changes: 1 addition & 1 deletion ecal/core/src/readwrite/ecal_writer.h
Original file line number Diff line number Diff line change
Expand Up @@ -217,6 +217,6 @@ namespace eCAL
bool m_use_tdesc;
int m_share_ttype;
int m_share_tdesc;
bool m_created;
std::atomic<bool> m_created;
};
}
17 changes: 10 additions & 7 deletions ecal/core/src/service/ecal_service_client_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,12 @@ namespace eCAL
counter << std::chrono::steady_clock::now().time_since_epoch().count();
m_service_id = counter.str();

// register this client
Register(false);

// mark as created
m_created = true;

// register this client
Register(false);

return(true);
}

Expand All @@ -111,6 +111,9 @@ namespace eCAL
m_event_callback_map.clear();
}

// mark as no more created (and prevent reregistering)
m_created = false;

// unregister this client
Unregister();

Expand All @@ -119,9 +122,6 @@ namespace eCAL
m_service_id.clear();
m_host_name.clear();

// mark as not created
m_created = false;

return(true);
}

Expand Down Expand Up @@ -613,10 +613,12 @@ namespace eCAL
}
}

// called by eCAL:CClientGate every second to update registration layer
void CServiceClientImpl::RefreshRegistration()
{
// this function will be called finally from registration provider every second (by default settings)
if (!m_created) return;

// register without send
Register(false);
}

Expand Down Expand Up @@ -661,6 +663,7 @@ namespace eCAL

void CServiceClientImpl::Register(const bool force_)
{
if (!m_created) return;
if (m_service_name.empty()) return;

eCAL::pb::Sample sample;
Expand Down
3 changes: 2 additions & 1 deletion ecal/core/src/service/ecal_service_client_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

#include <ecal/service/client_session.h>

#include <atomic>
#include <map>
#include <mutex>
#include <memory>
Expand Down Expand Up @@ -128,6 +129,6 @@ namespace eCAL
std::string m_service_id;
std::string m_host_name;

bool m_created;
std::atomic<bool> m_created;
};
}
22 changes: 14 additions & 8 deletions ecal/core/src/service/ecal_service_server_impl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,10 @@ namespace eCAL
return instance;
}

CServiceServerImpl::CServiceServerImpl()
{}
CServiceServerImpl::CServiceServerImpl() :
m_created(false)
{
}

CServiceServerImpl::~CServiceServerImpl()
{
Expand Down Expand Up @@ -126,12 +128,12 @@ namespace eCAL
m_tcp_server_v1 = server_manager->create_server(1, 0, service_callback, true, event_callback);
}

// register this service
Register(false);

// mark as created
m_created = true;

// register this service
Register(false);

return(true);
}

Expand All @@ -157,6 +159,9 @@ namespace eCAL
m_event_callback_map.clear();
}

// mark as no more created (and prevent reregistering)
m_created = false;

// unregister this service
Unregister();

Expand All @@ -170,8 +175,6 @@ namespace eCAL
m_connected_v1 = false;
}

m_created = false;

return(true);
}

Expand Down Expand Up @@ -312,15 +315,18 @@ namespace eCAL
{
}

// called by eCAL:CServiceGate every second to update registration layer
void CServiceServerImpl::RefreshRegistration()
{
// this function will be called finally from registration provider every second (by default settings)
if (!m_created) return;

// register without send
Register(false);
}

void CServiceServerImpl::Register(const bool force_)
{
if (!m_created) return;
if (m_service_name.empty()) return;

// might be zero in contruction phase
Expand Down
6 changes: 4 additions & 2 deletions ecal/core/src/service/ecal_service_server_impl.h
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@

#include <ecal/ecal.h>
#include <ecal/ecal_callback.h>

#include <atomic>
#include <memory>
#include <string>

Expand Down Expand Up @@ -130,10 +132,10 @@ namespace eCAL
using EventCallbackMapT = std::map<eCAL_Server_Event, ServerEventCallbackT>;
EventCallbackMapT m_event_callback_map;

bool 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;
bool m_connected_v1 = false;

std::atomic<bool> m_created;
};
}

0 comments on commit 0c282f2

Please sign in to comment.