From d05fcede4c102a80b8d06600b171593d977048ce Mon Sep 17 00:00:00 2001
From: Dan Rose <dan@digilabs.io>
Date: Thu, 19 Mar 2020 19:15:12 -0500
Subject: [PATCH 1/3] Remove suspicious wait set locking code

Since inuse is not volatile, it cannot be used instead of a lock.
---
 rmw_cyclonedds_cpp/src/rmw_node.cpp | 22 +++-------------------
 1 file changed, 3 insertions(+), 19 deletions(-)

diff --git a/rmw_cyclonedds_cpp/src/rmw_node.cpp b/rmw_cyclonedds_cpp/src/rmw_node.cpp
index 464148582..094f7e2c3 100644
--- a/rmw_cyclonedds_cpp/src/rmw_node.cpp
+++ b/rmw_cyclonedds_cpp/src/rmw_node.cpp
@@ -207,7 +207,6 @@ struct CddsWaitset
   size_t nelems;
 
   std::mutex lock;
-  bool inuse;
   std::vector<CddsSubscription *> subs;
   std::vector<CddsGuardCondition *> gcs;
   std::vector<CddsClient *> cls;
@@ -2078,7 +2077,6 @@ extern "C" rmw_wait_set_t * rmw_create_wait_set(rmw_context_t * context, size_t
     RMW_SET_ERROR_MSG("failed to construct wait set info struct");
     goto fail_ws;
   }
-  ws->inuse = false;
   ws->nelems = 0;
 #if MULTIDOMAIN
   owner = DDS_CYCLONEDDS_HANDLE;
@@ -2223,10 +2221,8 @@ static void clean_waitset_caches()
      used ... */
   std::lock_guard<std::mutex> lock(gcdds.lock);
   for (auto && ws : gcdds.waitsets) {
-    std::lock_guard<std::mutex> lock(ws->lock);
-    if (!ws->inuse) {
-      waitset_detach(ws);
-    }
+    std::lock_guard<std::mutex> lock2(ws->lock);
+    waitset_detach(ws);
   }
 }
 
@@ -2294,14 +2290,7 @@ extern "C" rmw_ret_t rmw_wait(
   CddsWaitset * ws = static_cast<CddsWaitset *>(wait_set->data);
   RET_NULL(ws);
 
-  {
-    std::lock_guard<std::mutex> lock(ws->lock);
-    if (ws->inuse) {
-      RMW_SET_ERROR_MSG("concurrent calls to rmw_wait on a single waitset is not supported");
-      return RMW_RET_ERROR;
-    }
-    ws->inuse = true;
-  }
+  std::lock_guard<std::mutex> lock(ws->lock);
 
   if (require_reattach(
       ws->subs, subs ? subs->subscriber_count : 0,
@@ -2405,11 +2394,6 @@ extern "C" rmw_ret_t rmw_wait(
   }
 #endif
 
-  {
-    std::lock_guard<std::mutex> lock(ws->lock);
-    ws->inuse = false;
-  }
-
   return (ws->trigs.size() == 1) ? RMW_RET_TIMEOUT : RMW_RET_OK;
 }
 

From 5f0023a6178e740e7ad46dcc85b69c7e007ab643 Mon Sep 17 00:00:00 2001
From: Dan Rose <dan@digilabs.io>
Date: Fri, 20 Mar 2020 09:54:37 -0500
Subject: [PATCH 2/3] If a waitset can't be immediately cleaned up, skip it
 instead of waiting

---
 rmw_cyclonedds_cpp/src/rmw_node.cpp | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/rmw_cyclonedds_cpp/src/rmw_node.cpp b/rmw_cyclonedds_cpp/src/rmw_node.cpp
index 094f7e2c3..fc232850b 100644
--- a/rmw_cyclonedds_cpp/src/rmw_node.cpp
+++ b/rmw_cyclonedds_cpp/src/rmw_node.cpp
@@ -2221,8 +2221,10 @@ static void clean_waitset_caches()
      used ... */
   std::lock_guard<std::mutex> lock(gcdds.lock);
   for (auto && ws : gcdds.waitsets) {
-    std::lock_guard<std::mutex> lock2(ws->lock);
-    waitset_detach(ws);
+    std::unique_lock<std::mutex> lock2(ws->lock, std::try_to_lock);
+    if (lock2.owns_lock()) {
+      waitset_detach(ws);
+    }
   }
 }
 

From deccdf727644ec370e22abe59131832f84419856 Mon Sep 17 00:00:00 2001
From: Dan Rose <dan@digilabs.io>
Date: Wed, 25 Mar 2020 12:29:48 -0500
Subject: [PATCH 3/3] use braces for constructor

---
 rmw_cyclonedds_cpp/src/rmw_node.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/rmw_cyclonedds_cpp/src/rmw_node.cpp b/rmw_cyclonedds_cpp/src/rmw_node.cpp
index fc232850b..445a9699d 100644
--- a/rmw_cyclonedds_cpp/src/rmw_node.cpp
+++ b/rmw_cyclonedds_cpp/src/rmw_node.cpp
@@ -2219,9 +2219,9 @@ static void clean_waitset_caches()
      have been cached in a waitset), and drops all cached entities from all waitsets (just to keep
      life simple). I'm assuming one is not allowed to delete an entity while it is still being
      used ... */
-  std::lock_guard<std::mutex> lock(gcdds.lock);
+  std::lock_guard<std::mutex> lock{gcdds.lock};
   for (auto && ws : gcdds.waitsets) {
-    std::unique_lock<std::mutex> lock2(ws->lock, std::try_to_lock);
+    std::unique_lock<std::mutex> lock2{ws->lock, std::try_to_lock};
     if (lock2.owns_lock()) {
       waitset_detach(ws);
     }