Explicitly check CLOSED flag when creating entity

All functions creating an entity pin the parent, which guarantees that
the CLOSED is not set yet, but as setting it occurs within the mutex, it
may be set by the time the parent has been locked.  One should not try
to add a child entity at that point, so check it again.

Signed-off-by: Erik Boasson <eb@ilities.com>

src/core/ddsc/src/dds__entity.h

@@ -36,7 +36,7 @@ dds_entity_register_child (
 DDS_EXPORT void
 dds_entity_add_ref_locked(dds_entity *e);
 
-#define DEFINE_ENTITY_LOCK_UNLOCK(qualifier_, type_, kind_) \
+#define DEFINE_ENTITY_LOCK_UNLOCK_ONLY(qualifier_, type_, kind_) \
   qualifier_ dds_return_t type_##_lock (dds_entity_t hdl, type_ **x) \
   { \
     dds_return_t rc; \
@@ -46,15 +46,29 @@ dds_entity_add_ref_locked(dds_entity *e);
     *x = (type_ *) e; \
     return DDS_RETCODE_OK; \
   } \
-  \
   qualifier_ void type_##_unlock (type_ *x) \
   { \
     dds_entity_unlock (&x->m_entity); \
   }
-#define DECL_ENTITY_LOCK_UNLOCK(qualifier_, type_) \
+#define DECL_ENTITY_LOCK_UNLOCK_ONLY(qualifier_, type_) \
   qualifier_ dds_return_t type_##_lock (dds_entity_t hdl, type_ **x); \
   qualifier_ void type_##_unlock (type_ *x);
 
+#define DEFINE_ENTITY_LOCK_UNLOCK(qualifier_, type_, kind_) \
+  DEFINE_ENTITY_LOCK_UNLOCK_ONLY (qualifier_, type_, kind_) \
+  qualifier_ dds_return_t type_##_lock_for_create (dds_entity_t hdl, type_ **x) \
+  { \
+    dds_return_t rc; \
+    dds_entity *e; \
+    if ((rc = dds_entity_lock_for_create (hdl, kind_, &e)) < 0) \
+      return rc; \
+    *x = (type_ *) e; \
+    return DDS_RETCODE_OK; \
+  }
+#define DECL_ENTITY_LOCK_UNLOCK(qualifier_, type_) \
+  DECL_ENTITY_LOCK_UNLOCK_ONLY (qualifier_, type_) \
+  qualifier_ dds_return_t type_##_lock_for_create (dds_entity_t hdl, type_ **x);
+
 DDS_EXPORT inline dds_entity *dds_entity_from_handle_link (struct dds_handle_link *hdllink) {
   return (dds_entity *) ((char *) hdllink - offsetof (struct dds_entity, m_hdllink));
 }
@@ -96,6 +110,12 @@ dds_entity_lock(
   dds_entity_kind_t kind,
   dds_entity **e);
 
+DDS_EXPORT dds_return_t
+dds_entity_lock_for_create(
+  dds_entity_t hdl,
+  dds_entity_kind_t kind,
+  dds_entity **e);
+
 DDS_EXPORT void
 dds_entity_unlock(dds_entity *e);
 

src/core/ddsc/src/dds_entity.c

@@ -1099,6 +1099,20 @@ dds_return_t dds_entity_lock (dds_entity_t hdl, dds_entity_kind_t kind, dds_enti
   }
 }
 
+dds_return_t dds_entity_lock_for_create (dds_entity_t hdl, dds_entity_kind_t kind, dds_entity **eptr)
+{
+  dds_return_t res;
+  if ((res = dds_entity_lock (hdl, kind, eptr)) < 0)
+    return res;
+  else if (!dds_handle_is_closed (&(*eptr)->m_hdllink))
+    return DDS_RETCODE_OK;
+  else
+  {
+    ddsrt_mutex_unlock (&(*eptr)->m_mutex);
+    return DDS_RETCODE_BAD_PARAMETER;
+  }
+}
+
 void dds_entity_unlock (dds_entity *e)
 {
   ddsrt_mutex_unlock (&e->m_mutex);

src/core/ddsc/src/dds_guardcond.c

@@ -41,7 +41,7 @@ dds_entity_t dds_create_guardcondition (dds_entity_t owner)
   if ((rc = dds_init ()) < 0)
     return rc;
 
-  if ((rc = dds_entity_lock (owner, DDS_KIND_DONTCARE, &e)) != DDS_RETCODE_OK)
+  if ((rc = dds_entity_lock_for_create (owner, DDS_KIND_DONTCARE, &e)) != DDS_RETCODE_OK)
     goto err_entity_lock;
 
   switch (dds_entity_kind (e))

src/core/ddsc/src/dds_publisher.c

@@ -52,7 +52,7 @@ dds_entity_t dds_create_publisher (dds_entity_t participant, const dds_qos_t *qo
   dds_qos_t *new_qos;
   dds_return_t ret;
 
-  if ((ret = dds_participant_lock (participant, &par)) != DDS_RETCODE_OK)
+  if ((ret = dds_participant_lock_for_create (participant, &par)) != DDS_RETCODE_OK)
     return ret;
 
   new_qos = dds_create_qos ();

src/core/ddsc/src/dds_querycond.c

@@ -26,7 +26,7 @@ dds_entity_t dds_create_querycondition (dds_entity_t reader, uint32_t mask, dds_
   dds_return_t rc;
   dds_reader *r;
 
-  if ((rc = dds_reader_lock (reader, &r)) != DDS_RETCODE_OK)
+  if ((rc = dds_reader_lock_for_create (reader, &r)) != DDS_RETCODE_OK)
     return rc;
   else
   {

src/core/ddsc/src/dds_readcond.c

@@ -64,7 +64,7 @@ dds_entity_t dds_create_readcondition (dds_entity_t reader, uint32_t mask)
 {
   dds_reader *rd;
   dds_return_t rc;
-  if ((rc = dds_reader_lock (reader, &rd)) != DDS_RETCODE_OK)
+  if ((rc = dds_reader_lock_for_create (reader, &rd)) != DDS_RETCODE_OK)
     return rc;
   else
   {

src/core/ddsc/src/dds_reader.c

@@ -323,7 +323,7 @@ dds_entity_t dds_create_reader (dds_entity_t participant_or_subscriber, dds_enti
     }
   }
 
-  if ((ret = dds_subscriber_lock (subscriber, &sub)) != DDS_RETCODE_OK)
+  if ((ret = dds_subscriber_lock_for_create (subscriber, &sub)) != DDS_RETCODE_OK)
   {
     reader = ret;
     goto err_sub_lock;
@@ -335,7 +335,7 @@ dds_entity_t dds_create_reader (dds_entity_t participant_or_subscriber, dds_enti
     sub->m_entity.m_flags |= DDS_ENTITY_IMPLICIT;
   }
 
-  if ((ret = dds_topic_lock (t, &tp)) != DDS_RETCODE_OK)
+  if ((ret = dds_topic_lock_for_create (t, &tp)) != DDS_RETCODE_OK)
   {
     reader = ret;
     goto err_tp_lock;
@@ -397,7 +397,7 @@ dds_entity_t dds_create_reader (dds_entity_t participant_or_subscriber, dds_enti
   ddsrt_mutex_lock (&tp->m_entity.m_mutex);
   assert (ret == DDS_RETCODE_OK); /* FIXME: can be out-of-resources at the very least */
   thread_state_asleep (lookup_thread_state ());
-  
+
   rd->m_entity.m_iid = get_entity_instance_id (&rd->m_entity.m_domain->gv, &rd->m_entity.m_guid);
   dds_entity_register_child (&sub->m_entity, &rd->m_entity);
 

src/core/ddsc/src/dds_subscriber.c

@@ -74,7 +74,7 @@ dds_entity_t dds_create_subscriber (dds_entity_t participant, const dds_qos_t *q
   dds_participant *par;
   dds_entity_t hdl;
   dds_return_t ret;
-  if ((ret = dds_participant_lock (participant, &par)) != DDS_RETCODE_OK)
+  if ((ret = dds_participant_lock_for_create (participant, &par)) != DDS_RETCODE_OK)
     return ret;
   hdl = dds__create_subscriber_l (par, qos, listener);
   dds_participant_unlock (par);

src/core/ddsc/src/dds_topic.c

@@ -324,7 +324,7 @@ dds_entity_t dds_create_topic_arbitrary (dds_entity_t participant, struct ddsi_s
 
   /* FIXME: just mutex_lock ought to be good enough, but there is the
      pesky "closed" check still ... */
-  if ((rc = dds_participant_lock (participant, &par)) != DDS_RETCODE_OK)
+  if ((rc = dds_participant_lock_for_create (participant, &par)) != DDS_RETCODE_OK)
     goto err_lock_participant;
 
   bool retry_lookup;
@@ -384,7 +384,7 @@ dds_entity_t dds_create_topic_arbitrary (dds_entity_t participant, struct ddsi_s
           abort ();
       }
 
-      if ((rc = dds_participant_lock (participant, &par)) != DDS_RETCODE_OK)
+      if ((rc = dds_participant_lock_for_create (participant, &par)) != DDS_RETCODE_OK)
         goto err_lock_participant;
     }
   } while (retry_lookup);

src/core/ddsc/src/dds_waitset.c

@@ -21,7 +21,7 @@
 #include "dds__rhc.h"
 #include "dds/ddsi/ddsi_iid.h"
 
-DEFINE_ENTITY_LOCK_UNLOCK (static, dds_waitset, DDS_KIND_WAITSET)
+DEFINE_ENTITY_LOCK_UNLOCK_ONLY (static, dds_waitset, DDS_KIND_WAITSET)
 
 static bool is_triggered (struct dds_entity *e)
 {
@@ -125,7 +125,7 @@ dds_entity_t dds_create_waitset (dds_entity_t owner)
   if ((rc = dds_init ()) < 0)
     return rc;
 
-  if ((rc = dds_entity_lock (owner, DDS_KIND_DONTCARE, &e)) != DDS_RETCODE_OK)
+  if ((rc = dds_entity_lock_for_create (owner, DDS_KIND_DONTCARE, &e)) != DDS_RETCODE_OK)
     goto err_entity_lock;
 
   switch (dds_entity_kind (e))

src/core/ddsc/src/dds_writer.c

@@ -261,14 +261,14 @@ dds_entity_t dds_create_writer (dds_entity_t participant_or_publisher, dds_entit
     dds_entity_unpin (p_or_p);
   }
 
-  if ((rc = dds_publisher_lock (publisher, &pub)) != DDS_RETCODE_OK)
+  if ((rc = dds_publisher_lock_for_create (publisher, &pub)) != DDS_RETCODE_OK)
     return rc;
 
   ddsi_tran_conn_t conn = pub->m_entity.m_domain->gv.data_conn_uc;
   if (publisher != participant_or_publisher)
     pub->m_entity.m_flags |= DDS_ENTITY_IMPLICIT;
 
-  if ((rc = dds_topic_lock (topic, &tp)) != DDS_RETCODE_OK)
+  if ((rc = dds_topic_lock_for_create (topic, &tp)) != DDS_RETCODE_OK)
     goto err_tp_lock;
   assert (tp->m_stopic);