From 8fe9ae94fddf059461fb008d201dbc08bdd6f22a Mon Sep 17 00:00:00 2001 From: Sumanth Nirmal Date: Fri, 2 Jul 2021 14:35:09 -0700 Subject: [PATCH 1/2] Modify updating of the entity closed status to be atomic Signed-off-by: Sumanth Nirmal --- .../eclipse/cyclonedds/core/ObjectDelegate.hpp | 3 ++- .../eclipse/cyclonedds/core/ObjectDelegate.cpp | 18 ++++-------------- 2 files changed, 6 insertions(+), 15 deletions(-) diff --git a/src/ddscxx/include/org/eclipse/cyclonedds/core/ObjectDelegate.hpp b/src/ddscxx/include/org/eclipse/cyclonedds/core/ObjectDelegate.hpp index d9b03b8e..b279a8ec 100644 --- a/src/ddscxx/include/org/eclipse/cyclonedds/core/ObjectDelegate.hpp +++ b/src/ddscxx/include/org/eclipse/cyclonedds/core/ObjectDelegate.hpp @@ -18,6 +18,7 @@ #ifndef CYCLONEDDS_CORE_OBJECT_DELEGATE_HPP_ #define CYCLONEDDS_CORE_OBJECT_DELEGATE_HPP_ +#include #include "dds/core/macros.hpp" #include "dds/core/refmacros.hpp" #include "org/eclipse/cyclonedds/core/Mutex.hpp" @@ -57,7 +58,7 @@ class OMG_DDS_API ObjectDelegate void set_weak_ref (ObjectDelegate::weak_ref_type weak_ref); Mutex mutex; - bool closed; + std::atomic_bool closed {false}; ObjectDelegate::weak_ref_type myself; }; diff --git a/src/ddscxx/src/org/eclipse/cyclonedds/core/ObjectDelegate.cpp b/src/ddscxx/src/org/eclipse/cyclonedds/core/ObjectDelegate.cpp index 27c4eaa7..d740ca64 100644 --- a/src/ddscxx/src/org/eclipse/cyclonedds/core/ObjectDelegate.cpp +++ b/src/ddscxx/src/org/eclipse/cyclonedds/core/ObjectDelegate.cpp @@ -16,8 +16,7 @@ #include "org/eclipse/cyclonedds/core/ObjectDelegate.hpp" #include -org::eclipse::cyclonedds::core::ObjectDelegate::ObjectDelegate () : - closed (false) +org::eclipse::cyclonedds::core::ObjectDelegate::ObjectDelegate () { } @@ -27,24 +26,15 @@ org::eclipse::cyclonedds::core::ObjectDelegate::~ObjectDelegate () void org::eclipse::cyclonedds::core::ObjectDelegate::check () const { - /* This method is not-thread-safe, and should only be used with a lock. */ - if (closed) { + if (closed.load()) { ISOCPP_THROW_EXCEPTION (ISOCPP_ALREADY_CLOSED_ERROR, "Trying to invoke an oparation on an object that was already closed"); } } void org::eclipse::cyclonedds::core::ObjectDelegate::lock () const { + check(); this->mutex.lock (); - try - { - check(); - } - catch (...) - { - this->mutex.unlock (); - throw; - } } void org::eclipse::cyclonedds::core::ObjectDelegate::unlock () const @@ -54,7 +44,7 @@ void org::eclipse::cyclonedds::core::ObjectDelegate::unlock () const void org::eclipse::cyclonedds::core::ObjectDelegate::close () { - this->closed = true; + this->closed.store(true); } void org::eclipse::cyclonedds::core::ObjectDelegate::set_weak_ref (ObjectDelegate::weak_ref_type weak_ref) From 8a00aa9d8804c3f28b93bb8a7166c5122e1c73ba Mon Sep 17 00:00:00 2001 From: Sumanth Nirmal Date: Fri, 2 Jul 2021 14:36:21 -0700 Subject: [PATCH 2/2] Add is_valid API get the status of DDScObjectDelegate Signed-off-by: Sumanth Nirmal --- .../org/eclipse/cyclonedds/core/DDScObjectDelegate.hpp | 1 + .../include/org/eclipse/cyclonedds/core/ObjectDelegate.hpp | 1 + .../src/org/eclipse/cyclonedds/core/DDScObjectDelegate.cpp | 5 +++++ .../src/org/eclipse/cyclonedds/core/ObjectDelegate.cpp | 6 ++++++ 4 files changed, 13 insertions(+) diff --git a/src/ddscxx/include/org/eclipse/cyclonedds/core/DDScObjectDelegate.hpp b/src/ddscxx/include/org/eclipse/cyclonedds/core/DDScObjectDelegate.hpp index 11198367..f44bd371 100644 --- a/src/ddscxx/include/org/eclipse/cyclonedds/core/DDScObjectDelegate.hpp +++ b/src/ddscxx/include/org/eclipse/cyclonedds/core/DDScObjectDelegate.hpp @@ -51,6 +51,7 @@ class OMG_DDS_API DDScObjectDelegate : public virtual org::eclipse::cyclonedds:: dds_entity_t get_ddsc_entity (); void set_ddsc_entity (dds_entity_t e); void add_to_entity_map (org::eclipse::cyclonedds::core::ObjectDelegate::weak_ref_type weak_ref); + bool is_valid () const; public: static ObjectDelegate::ref_type extract_strong_ref(dds_entity_t e); diff --git a/src/ddscxx/include/org/eclipse/cyclonedds/core/ObjectDelegate.hpp b/src/ddscxx/include/org/eclipse/cyclonedds/core/ObjectDelegate.hpp index b279a8ec..e643593d 100644 --- a/src/ddscxx/include/org/eclipse/cyclonedds/core/ObjectDelegate.hpp +++ b/src/ddscxx/include/org/eclipse/cyclonedds/core/ObjectDelegate.hpp @@ -47,6 +47,7 @@ class OMG_DDS_API ObjectDelegate virtual void close (); void lock() const; void unlock() const; + virtual bool is_valid() const; virtual void init (ObjectDelegate::weak_ref_type weak_ref) = 0; ObjectDelegate::weak_ref_type get_weak_ref () const; diff --git a/src/ddscxx/src/org/eclipse/cyclonedds/core/DDScObjectDelegate.cpp b/src/ddscxx/src/org/eclipse/cyclonedds/core/DDScObjectDelegate.cpp index 69f8e3ff..e1187793 100644 --- a/src/ddscxx/src/org/eclipse/cyclonedds/core/DDScObjectDelegate.cpp +++ b/src/ddscxx/src/org/eclipse/cyclonedds/core/DDScObjectDelegate.cpp @@ -84,6 +84,11 @@ org::eclipse::cyclonedds::core::DDScObjectDelegate::add_to_entity_map(org::eclip DDScObjectDelegate::entity_map_mutex.unlock (); } +bool org::eclipse::cyclonedds::core::DDScObjectDelegate::is_valid() const +{ + return org::eclipse::cyclonedds::core::ObjectDelegate::is_valid(); +} + void org::eclipse::cyclonedds::core::DDScObjectDelegate::delete_from_entity_map() { diff --git a/src/ddscxx/src/org/eclipse/cyclonedds/core/ObjectDelegate.cpp b/src/ddscxx/src/org/eclipse/cyclonedds/core/ObjectDelegate.cpp index d740ca64..a6a7a18f 100644 --- a/src/ddscxx/src/org/eclipse/cyclonedds/core/ObjectDelegate.cpp +++ b/src/ddscxx/src/org/eclipse/cyclonedds/core/ObjectDelegate.cpp @@ -42,6 +42,12 @@ void org::eclipse::cyclonedds::core::ObjectDelegate::unlock () const this->mutex.unlock (); } +bool org::eclipse::cyclonedds::core::ObjectDelegate::is_valid() const +{ + bool is_closed = this->closed.load(std::memory_order_acquire); + return !is_closed; +} + void org::eclipse::cyclonedds::core::ObjectDelegate::close () { this->closed.store(true);