Fix domain confusion in garbage collector caused by cdds_psmx plugin

Signed-off-by: Dennis Potman <[email protected]>

src/core/ddsc/tests/psmx_cdds_impl.c

@@ -18,6 +18,7 @@
 #include "dds/ddsrt/strtol.h"
 #include "dds/ddsi/ddsi_locator.h"
 #include "dds/ddsi/ddsi_protocol.h"
+#include "dds/ddsi/ddsi_thread.h"
 #include "dds/ddsc/dds_psmx.h"
 #include "psmx_cdds_impl.h"
 #include "psmx_cdds_data.h"
@@ -507,6 +508,8 @@ dds_return_t cdds_create_psmx (dds_psmx_t **psmx_out, dds_psmx_instance_id_t ins
 {
   assert (psmx_out);
 
+  ddsrt_atomic_st32 (&dds_nested_gv_allowed, 1);
+
   struct cdds_psmx *psmx = dds_alloc (sizeof (*psmx));
   psmx->c.instance_name = dds_string_dup ("cdds-psmx");
   psmx->c.instance_id = instance_id;

src/core/ddsi/include/dds/ddsi/ddsi_thread.h

@@ -73,10 +73,24 @@ void ddsi_thread_vtime_trace (struct ddsi_thread_state *thrst);
 #define ddsi_thread_vtime_trace(thrst) do { } while (0)
 #endif /* DDSI_THREAD_DEBUG */
 
+/**
+ * When DDS_ALLOW_NESTED_DOMAIN is set, the thread state can store an additional
+ * gv pointer, so that in case of domain nesting in a single thread, the gv
+ * pointer of the nested domain won't overwrite the top-level domain's gv pointer.
+ * Nesting domains has several limitations, and is used for PSMX testing using the
+ * Cyclone DDS PSMX implementation.
+ */
+#ifdef DDS_ALLOW_NESTED_DOMAIN
+#define THREAD_BASE_NESTEDGV ddsrt_atomic_voidp_t nested_gv;
+#else
+#define THREAD_BASE_NESTEDGV
+#endif
+
 #define THREAD_BASE                             \
   ddsrt_atomic_uint32_t vtime;                  \
   enum ddsi_thread_state_kind state;            \
   ddsrt_atomic_voidp_t gv;                      \
+  THREAD_BASE_NESTEDGV                          \
   ddsrt_thread_t tid;                           \
   uint32_t (*f) (void *arg);                    \
   void *f_arg;                                  \
@@ -115,6 +129,10 @@ extern ddsrt_thread_local struct ddsi_thread_state *tsd_thread_state;
 DDS_EXPORT extern ddsrt_thread_local struct ddsi_thread_state *tsd_thread_state;
 #endif
 
+#ifdef DDS_ALLOW_NESTED_DOMAIN
+extern ddsrt_atomic_uint32_t dds_nested_gv_allowed;
+#endif
+
 /** @component thread_support */
 void ddsi_thread_states_init (void);
 
@@ -190,7 +208,19 @@ inline void ddsi_thread_state_awake (struct ddsi_thread_state *thrst, const stru
   assert (gv != NULL);
   assert (thrst->state != DDSI_THREAD_STATE_ALIVE || gv == ddsrt_atomic_ldvoidp (&thrst->gv));
   ddsi_thread_vtime_trace (thrst);
+#ifdef DDS_ALLOW_NESTED_DOMAIN
+  assert ((vt & DDSI_VTIME_NEST_MASK) == 0 || gv == ddsrt_atomic_ldvoidp (&thrst->gv) || gv == ddsrt_atomic_ldvoidp (&thrst->nested_gv) || 0 == ddsrt_atomic_ldvoidp (&thrst->nested_gv));
+  if ((vt & DDSI_VTIME_NEST_MASK) == 0)
+    ddsrt_atomic_stvoidp (&thrst->gv, (struct ddsi_domaingv *) gv);
+  else if (gv != ddsrt_atomic_ldvoidp (&thrst->gv))
+  {
+    assert (ddsrt_atomic_ld32 (&dds_nested_gv_allowed));
+    ddsrt_atomic_stvoidp (&thrst->nested_gv, (struct ddsi_domaingv *) gv);
+  }
+#else
+  assert ((vt & DDSI_VTIME_NEST_MASK) == 0 || gv == ddsrt_atomic_ldvoidp (&thrst->gv));
   ddsrt_atomic_stvoidp (&thrst->gv, (struct ddsi_domaingv *) gv);
+#endif
   ddsrt_atomic_fence_stst ();
   ddsrt_atomic_st32 (&thrst->vtime, vt + 1u);
   /* nested calls a rare and an extra fence doesn't break things */

src/core/ddsi/src/ddsi_gc.c

@@ -55,7 +55,12 @@ static void threads_vtime_gather_for_wait (const struct ddsi_domaingv *gv, uint3
            correct; if instead it has gone through another cycle since loading thrst[i].vtime, then
            the thread will be dropped from the live threads on the next check.  So it won't ever wait
            with unknown duration for progres of threads stuck in another domain */
-        if (gv == ddsrt_atomic_ldvoidp (&cur->thrst[i].gv))
+#ifdef DDS_ALLOW_NESTED_DOMAIN
+        const bool this_domain = gv == ddsrt_atomic_ldvoidp (&cur->thrst[i].gv) || gv == ddsrt_atomic_ldvoidp (&cur->thrst[i].nested_gv);
+#else
+        const bool this_domain = gv == ddsrt_atomic_ldvoidp (&cur->thrst[i].gv);
+#endif
+        if (this_domain)
         {
           assert (dstidx < nthreads);
           ivs[dstidx].thrst = &cur->thrst[i];
@@ -76,7 +81,12 @@ static int threads_vtime_check (const struct ddsi_domaingv *gv, uint32_t *nivs,
   {
     ddsi_vtime_t vtime = ddsrt_atomic_ld32 (&ivs[i].thrst->vtime);
     assert (ddsi_vtime_awake_p (ivs[i].vtime));
-    if (!ddsi_vtime_gt (vtime, ivs[i].vtime) && ddsrt_atomic_ldvoidp (&ivs[i].thrst->gv) == gv)
+#ifdef DDS_ALLOW_NESTED_DOMAIN
+    const bool this_domain = ddsrt_atomic_ldvoidp (&ivs[i].thrst->gv) == gv || ddsrt_atomic_ldvoidp (&ivs[i].thrst->nested_gv) == gv;
+#else
+    const bool this_domain = ddsrt_atomic_ldvoidp (&ivs[i].thrst->gv) == gv;
+#endif
+    if (!ddsi_vtime_gt (vtime, ivs[i].vtime) && this_domain)
       ++i;
     else
     {

src/core/ddsi/src/ddsi_thread.c

@@ -27,6 +27,9 @@
 
 struct ddsi_thread_states thread_states;
 ddsrt_thread_local struct ddsi_thread_state *tsd_thread_state;
+#ifdef DDS_ALLOW_NESTED_DOMAIN
+ddsrt_atomic_uint32_t dds_nested_gv_allowed;
+#endif
 
 extern inline bool ddsi_vtime_awake_p (ddsi_vtime_t vtime);
 extern inline bool ddsi_vtime_asleep_p (ddsi_vtime_t vtime);
@@ -114,6 +117,9 @@ void ddsi_thread_states_init (void)
   struct ddsi_thread_state * const thrst = ddsi_lookup_thread_state_real ();
   assert (ts0 == NULL || ts0 == thrst);
   (void) thrst;
+#ifdef DDS_ALLOW_NESTED_DOMAIN
+  ddsrt_atomic_st32 (&dds_nested_gv_allowed, 0);
+#endif
 }
 
 bool ddsi_thread_states_fini (void)
@@ -300,6 +306,9 @@ static struct ddsi_thread_state *init_thread_state (const char *tname, const str
 
   assert (ddsi_vtime_asleep_p (ddsrt_atomic_ld32 (&thrst->vtime)));
   ddsrt_atomic_stvoidp (&thrst->gv, (struct ddsi_domaingv *) gv);
+#ifdef DDS_ALLOW_NESTED_DOMAIN
+  ddsrt_atomic_stvoidp (&thrst->nested_gv, NULL);
+#endif
   (void) ddsrt_strlcpy (thrst->name, tname, sizeof (thrst->name));
   thrst->state = state;
   return thrst;

src/ddsrt/CMakeLists.txt

@@ -314,6 +314,10 @@ foreach(feature SSL SECURITY LIFESPAN DEADLINE_MISSED NETWORK_PARTITIONS
   set(DDS_HAS_${feature} ${ENABLE_${feature}})
 endforeach()
 
+if(BUILD_TESTING)
+  set(DDS_ALLOW_NESTED_DOMAIN 1)
+endif()
+
 configure_file(include/dds/config.h.in include/dds/config.h)
 configure_file(include/dds/features.h.in include/dds/features.h)
 configure_file(include/dds/version.h.in include/dds/version.h)

src/ddsrt/include/dds/features.h.in

@@ -38,4 +38,7 @@
 /* Whether or not support for topic discovery is included */
 #cmakedefine DDS_HAS_TOPIC_DISCOVERY 1
 
+/* Not for general use, specificly for testing psmx Cyclone DDS plugin */
+#cmakedefine DDS_ALLOW_NESTED_DOMAIN 1
+
 #endif