Merge pull request #76 from eboasson/builtintopics

Fixes for memory leaks, out-of-bounds reads and race conditions working with instances

src/CMakeLists.txt

@@ -92,12 +92,6 @@ endif()
 # Enable coloured ouput if Ninja is used for building
 if(${CMAKE_C_COMPILER_ID} STREQUAL "Clang" OR ${CMAKE_C_COMPILER_ID} STREQUAL "AppleClang")
   add_definitions(-Wall -Wextra -Wconversion -Wunused)
-  message("${CMAKE_GENERATOR}")
-  #if (CMAKE_BUILD_TYPE STREQUAL "Debug" AND NOT(${CMAKE_GENERATOR} STREQUAL "Xcode"))
-  #  set (CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -fno-omit-frame-pointer -fsanitize=address")
-  #  set (CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -fno-omit-frame-pointer -fsanitize=address")
-  #  set (CMAKE_LINKER_FLAGS_DEBUG "${CMAKE_LINKER_FLAGS_DEBUG} -fno-omit-frame-pointer -fsanitize=address")
-  #endif()
   if(${CMAKE_GENERATOR} STREQUAL "Ninja")
     add_definitions(-Xclang -fcolor-diagnostics)
   endif()
@@ -110,6 +104,27 @@ elseif(${CMAKE_C_COMPILER_ID} STREQUAL "MSVC")
   add_definitions(/W3)
 endif()
 
+# Make it easy to enable one of Clang's/gcc's analyzers, and default to using
+# the address sanitizer for ordinary debug builds; gcc is giving some grief on
+# Travis, so don't enable it for gcc by default
+if(NOT USE_SANITIZER)
+  if(${CMAKE_BUILD_TYPE} STREQUAL "Debug" AND
+      NOT (${CMAKE_GENERATOR} STREQUAL "Xcode") AND
+      (${CMAKE_C_COMPILER_ID} STREQUAL "Clang"
+        OR ${CMAKE_C_COMPILER_ID} STREQUAL "AppleClang"))
+    message(STATUS "Enabling address sanitizer; set USE_SANITIZER=none to prevent this")
+    set(USE_SANITIZER address)
+  else()
+    set(USE_SANITIZER none)
+  endif()
+endif()
+if(NOT (${USE_SANITIZER} STREQUAL "none"))
+  message(STATUS "Sanitizer set to ${USE_SANITIZER}")
+  set (CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fno-omit-frame-pointer -fsanitize=${USE_SANITIZER}")
+  set (CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer -fsanitize=${USE_SANITIZER}")
+  set (CMAKE_LINKER_FLAGS "${CMAKE_LINKER_FLAGS} -fno-omit-frame-pointer -fsanitize=${USE_SANITIZER}")
+endif()
+
 include(FileIDs)
 include(GNUInstallDirs)
 include(AnalyzeBuild)

src/core/ddsc/src/dds_init.c

@@ -88,7 +88,7 @@ dds_init(dds_domainid_t domain)
     {
       DDS_ERROR("requested domain id %d is out of range\n", domain);
       ret = DDS_ERRNO(DDS_RETCODE_ERROR);
-      goto fail_config;
+      goto fail_config_domainid;
     }
     else if (config.domainId.isdefault)
     {
@@ -98,7 +98,7 @@ dds_init(dds_domainid_t domain)
     {
       DDS_ERROR("requested domain id %d is inconsistent with configured value %d\n", domain, config.domainId.value);
       ret = DDS_ERRNO(DDS_RETCODE_ERROR);
-      goto fail_config;
+      goto fail_config_domainid;
     }
   }
 
@@ -190,6 +190,7 @@ fail_servicelease_new:
   thread_states_fini();
 fail_rtps_config:
   dds__builtin_fini();
+fail_config_domainid:
   dds_global.m_default_domain = DDS_DOMAIN_DEFAULT;
   config_fini (dds_cfgst);
   dds_cfgst = NULL;

src/core/ddsc/src/dds_instance.c

@@ -76,17 +76,8 @@ dds_instance_remove(
         assert (data);
         inst = dds_instance_find (topic, data, false);
     }
-
     if (inst) {
-        struct thread_state1 * const thr = lookup_thread_state();
-        const bool asleep = thr ? !vtime_awake_p(thr->vtime) : false;
-        if (asleep) {
-            thread_state_awake(thr);
-        }
         ddsi_tkmap_instance_unref (inst);
-        if (asleep) {
-            thread_state_asleep(thr);
-        }
     }
 }
 
@@ -133,6 +124,8 @@ dds_register_instance(
         _Out_ dds_instance_handle_t *handle,
         _In_ const void *data)
 {
+    struct thread_state1 * const thr = lookup_thread_state();
+    const bool asleep = !vtime_awake_p(thr->vtime);
     struct ddsi_tkmap_instance * inst;
     dds_entity *wr;
     dds_return_t ret;
@@ -154,14 +147,20 @@ dds_register_instance(
         ret = DDS_ERRNO(rc);
         goto err;
     }
+    if (asleep) {
+        thread_state_awake(thr);
+    }
     inst = dds_instance_find (((dds_writer*) wr)->m_topic, data, true);
     if(inst != NULL){
         *handle = inst->m_iid;
         ret = DDS_RETCODE_OK;
-    } else{
+    } else {
         DDS_ERROR("Unable to create instance\n");
         ret = DDS_ERRNO(DDS_RETCODE_ERROR);
     }
+    if (asleep) {
+        thread_state_asleep(thr);
+    }
     dds_entity_unlock(wr);
 err:
     return ret;
@@ -192,6 +191,8 @@ dds_unregister_instance_ts(
        _In_opt_ const void *data,
        _In_ dds_time_t timestamp)
 {
+    struct thread_state1 * const thr = lookup_thread_state();
+    const bool asleep = !vtime_awake_p(thr->vtime);
     dds_return_t ret = DDS_RETCODE_OK;
     dds__retcode_t rc;
     bool autodispose = true;
@@ -219,11 +220,17 @@ dds_unregister_instance_ts(
     if (wr->m_qos) {
         dds_qget_writer_data_lifecycle (wr->m_qos, &autodispose);
     }
+    if (asleep) {
+        thread_state_awake(thr);
+    }
     if (autodispose) {
         dds_instance_remove (((dds_writer*) wr)->m_topic, data, DDS_HANDLE_NIL);
         action |= DDS_WR_DISPOSE_BIT;
     }
     ret = dds_write_impl ((dds_writer*)wr, sample, timestamp, action);
+    if (asleep) {
+        thread_state_asleep(thr);
+    }
     dds_entity_unlock(wr);
 err:
     return ret;
@@ -236,14 +243,14 @@ dds_unregister_instance_ih_ts(
        _In_opt_ dds_instance_handle_t handle,
        _In_ dds_time_t timestamp)
 {
+    struct thread_state1 * const thr = lookup_thread_state();
+    const bool asleep = !vtime_awake_p(thr->vtime);
     dds_return_t ret = DDS_RETCODE_OK;
     dds__retcode_t rc;
     bool autodispose = true;
     dds_write_action action = DDS_WR_ACTION_UNREGISTER;
     dds_entity *wr;
-    struct ddsi_tkmap *map;
+    struct ddsi_tkmap_instance *tk;
-    const dds_topic *topic;
-    void *sample;
 
     rc = dds_entity_lock(writer, DDS_KIND_WRITER, &wr);
     if (rc != DDS_RETCODE_OK) {
@@ -260,16 +267,24 @@ dds_unregister_instance_ih_ts(
         action |= DDS_WR_DISPOSE_BIT;
     }
 
-    map = gv.m_tkmap;
+    if (asleep) {
-    topic = dds_instance_info((dds_entity*)wr);
+        thread_state_awake(thr);
-    sample = ddsi_sertopic_alloc_sample (topic->m_stopic);
+    }
-    if (ddsi_tkmap_get_key (map, topic->m_stopic, handle, sample)) {
+    tk = ddsi_tkmap_find_by_id (gv.m_tkmap, handle);
+    if (tk) {
+        struct ddsi_sertopic *tp = ((dds_writer*) wr)->m_topic->m_stopic;
+        void *sample = ddsi_sertopic_alloc_sample (tp);
+        ddsi_serdata_topicless_to_sample (tp, tk->m_sample, sample, NULL, NULL);
+        ddsi_tkmap_instance_unref (tk);
         ret = dds_write_impl ((dds_writer*)wr, sample, timestamp, action);
-    } else{
+        ddsi_sertopic_free_sample (tp, sample, DDS_FREE_ALL);
+    } else {
         DDS_ERROR("No instance related with the provided handle is found\n");
         ret = DDS_ERRNO(DDS_RETCODE_PRECONDITION_NOT_MET);
     }
-    ddsi_sertopic_free_sample (topic->m_stopic, sample, DDS_FREE_ALL);
+    if (asleep) {
+        thread_state_asleep(thr);
+    }
     dds_entity_unlock(wr);
 err:
     return ret;
@@ -288,10 +303,18 @@ dds_writedispose_ts(
 
     rc = dds_writer_lock(writer, &wr);
     if (rc == DDS_RETCODE_OK) {
+        struct thread_state1 * const thr = lookup_thread_state();
+        const bool asleep = !vtime_awake_p(thr->vtime);
+        if (asleep) {
+            thread_state_awake(thr);
+        }
         ret = dds_write_impl (wr, data, timestamp, DDS_WR_ACTION_WRITE_DISPOSE);
         if (ret == DDS_RETCODE_OK) {
             dds_instance_remove (wr->m_topic, data, DDS_HANDLE_NIL);
         }
+        if (asleep) {
+            thread_state_asleep(thr);
+        }
         dds_writer_unlock(wr);
     } else {
         DDS_ERROR("Error occurred on locking writer\n");
@@ -309,6 +332,7 @@ dds_dispose_impl(
        _In_ dds_time_t timestamp)
 {
     dds_return_t ret;
+    assert(vtime_awake_p(lookup_thread_state()->vtime));
     assert(wr);
     ret = dds_write_impl(wr, data, timestamp, DDS_WR_ACTION_DISPOSE);
     if (ret == DDS_RETCODE_OK) {
@@ -330,7 +354,15 @@ dds_dispose_ts(
 
     rc = dds_writer_lock(writer, &wr);
     if (rc == DDS_RETCODE_OK) {
+        struct thread_state1 * const thr = lookup_thread_state();
+        const bool asleep = !vtime_awake_p(thr->vtime);
+        if (asleep) {
+            thread_state_awake(thr);
+        }
         ret = dds_dispose_impl(wr, data, DDS_HANDLE_NIL, timestamp);
+        if (asleep) {
+            thread_state_asleep(thr);
+        }
         dds_writer_unlock(wr);
     } else {
         DDS_ERROR("Error occurred on locking writer\n");
@@ -353,16 +385,26 @@ dds_dispose_ih_ts(
 
     rc = dds_writer_lock(writer, &wr);
     if (rc == DDS_RETCODE_OK) {
-        struct ddsi_tkmap *map = gv.m_tkmap;
+        struct thread_state1 * const thr = lookup_thread_state();
-        const dds_topic *topic = dds_instance_info((dds_entity*)wr);
+        const bool asleep = !vtime_awake_p(thr->vtime);
-        void *sample = ddsi_sertopic_alloc_sample (topic->m_stopic);
+        struct ddsi_tkmap_instance *tk;
-        if (ddsi_tkmap_get_key (map, topic->m_stopic, handle, sample)) {
+        if (asleep) {
-            ret = dds_dispose_impl(wr, sample, handle, timestamp);
+            thread_state_awake(thr);
+        }
+        if ((tk = ddsi_tkmap_find_by_id (gv.m_tkmap, handle)) != NULL) {
+            struct ddsi_sertopic *tp = ((dds_writer*) wr)->m_topic->m_stopic;
+            void *sample = ddsi_sertopic_alloc_sample (tp);
+            ddsi_serdata_topicless_to_sample (tp, tk->m_sample, sample, NULL, NULL);
+            ddsi_tkmap_instance_unref (tk);
+            ret = dds_dispose_impl (wr, sample, handle, timestamp);
+            ddsi_sertopic_free_sample (tp, sample, DDS_FREE_ALL);
         } else {
             DDS_ERROR("No instance related with the provided handle is found\n");
             ret = DDS_ERRNO(DDS_RETCODE_PRECONDITION_NOT_MET);
         }
-        ddsi_sertopic_free_sample (topic->m_stopic, sample, DDS_FREE_ALL);
+        if (asleep) {
+            thread_state_asleep(thr);
+        }
         dds_writer_unlock(wr);
     } else {
         DDS_ERROR("Error occurred on locking writer\n");
@@ -390,9 +432,17 @@ dds_lookup_instance(
 
     topic = dds_instance_info_by_hdl (entity);
     if (topic) {
+        struct thread_state1 * const thr = lookup_thread_state();
+        const bool asleep = !vtime_awake_p(thr->vtime);
+        if (asleep) {
+            thread_state_awake(thr);
+        }
         sd = ddsi_serdata_from_sample (topic->m_stopic, SDK_KEY, data);
         ih = ddsi_tkmap_lookup (map, sd);
         ddsi_serdata_unref (sd);
+        if (asleep) {
+            thread_state_asleep(thr);
+        }
     } else {
         DDS_ERROR("Acquired topic is NULL\n");
     }
@@ -413,12 +463,14 @@ _Pre_satisfies_(entity & DDS_ENTITY_KIND_MASK)
 dds_return_t
 dds_instance_get_key(
         dds_entity_t entity,
-        dds_instance_handle_t inst,
+        dds_instance_handle_t ih,
         void *data)
 {
+    struct thread_state1 * const thr = lookup_thread_state();
+    const bool asleep = !vtime_awake_p(thr->vtime);
     dds_return_t ret;
     const dds_topic * topic;
-    struct ddsi_tkmap * map = gv.m_tkmap;
+    struct ddsi_tkmap_instance * tk;
 
     if(data == NULL){
         DDS_ERROR("Argument data is NULL\n");
@@ -432,14 +484,21 @@ dds_instance_get_key(
         ret = DDS_ERRNO(DDS_RETCODE_BAD_PARAMETER);
         goto err;
     }
-    ddsi_sertopic_zero_sample (topic->m_stopic, data);
+    if (asleep) {
-    if (ddsi_tkmap_get_key (map, topic->m_stopic, inst, data)) {
+        thread_state_awake(thr);
+    }
+    if ((tk = ddsi_tkmap_find_by_id(gv.m_tkmap, ih)) != NULL) {
+        ddsi_sertopic_zero_sample (topic->m_stopic, data);
+        ddsi_serdata_topicless_to_sample (topic->m_stopic, tk->m_sample, data, NULL, NULL);
+        ddsi_tkmap_instance_unref (tk);
         ret = DDS_RETCODE_OK;
-    } else{
+    } else {
         DDS_ERROR("No instance related with the provided entity is found\n");
         ret = DDS_ERRNO(DDS_RETCODE_BAD_PARAMETER);
     }
-
+    if (asleep) {
+        thread_state_asleep(thr);
+    }
 err:
     return ret;
 }

src/core/ddsc/src/dds_rhc.c

@@ -1276,6 +1276,12 @@ bool dds_rhc_store
         {
           DDS_TRACE("(reject)");
           stored = RHC_REJECTED;
+
+          /* FIXME: fix the bad rejection handling, probably put back in a proper rollback, until then a band-aid like this will have to do: */
+          inst->isnew = old_isnew;
+          inst->isdisposed = old_isdisposed;
+          if (old_isdisposed)
+            inst->disposed_gen--;
           goto error_or_nochange;
         }
       }

src/core/ddsc/tests/qos.c

@@ -274,6 +274,10 @@ CU_Test(ddsc_qos, copy_with_partition, .init=qos_init, .fini=qos_fini)
             CU_ASSERT_STRING_EQUAL_FATAL(p.ps[cnt], g_pol_partition.ps[cnt]);
         }
 
+        for (uint32_t cnt = 0; cnt < p.n; cnt++) {
+            dds_free (p.ps[cnt]);
+        }
+        dds_free (p.ps);
         dds_delete_qos(qos);
 }
 

src/core/ddsc/tests/waitset.c

@@ -265,7 +265,7 @@ CU_Theory((dds_entity_t *par), ddsc_waitset_create, non_participants, .init=ddsc
 /*************************************************************************************************/
 CU_TheoryDataPoints(ddsc_waitset_attach, invalid_params) = {
         CU_DataPoints(dds_entity_t, -2, -1, 0, 1, 100, INT_MAX, INT_MIN),
-        CU_DataPoints(dds_attach_t,  (dds_attach_t)NULL, (dds_attach_t)&reader, (dds_attach_t)3),
+        CU_DataPoints(dds_attach_t,  (dds_attach_t)NULL, (dds_attach_t)&reader, (dds_attach_t)3, (dds_attach_t)0, (dds_attach_t)0, (dds_attach_t)0, (dds_attach_t)0),
 };
 CU_Theory((dds_entity_t e, dds_attach_t a), ddsc_waitset_attach, invalid_params, .init=ddsc_waitset_basic_init, .fini=ddsc_waitset_basic_fini)
 {
@@ -278,7 +278,7 @@ CU_Theory((dds_entity_t e, dds_attach_t a), ddsc_waitset_attach, invalid_params,
 /*************************************************************************************************/
 CU_TheoryDataPoints(ddsc_waitset_attach, invalid_waitsets) = {
         CU_DataPoints(dds_entity_t, -2, -1, 0, 1, 100, INT_MAX, INT_MIN),
-        CU_DataPoints(dds_attach_t,  (dds_attach_t)NULL, (dds_attach_t)&reader, (dds_attach_t)3),
+        CU_DataPoints(dds_attach_t,  (dds_attach_t)NULL, (dds_attach_t)&reader, (dds_attach_t)3, (dds_attach_t)0, (dds_attach_t)0, (dds_attach_t)0, (dds_attach_t)0),
 };
 CU_Theory((dds_entity_t ws, dds_attach_t a), ddsc_waitset_attach, invalid_waitsets, .init=ddsc_waitset_basic_init, .fini=ddsc_waitset_basic_fini)
 {
@@ -294,7 +294,7 @@ CU_Theory((dds_entity_t ws, dds_attach_t a), ddsc_waitset_attach, invalid_waitse
 CU_TheoryDataPoints(ddsc_waitset_attach, non_waitsets) = {
         CU_DataPoints(dds_entity_t*, &participant, &topic, &writer, &reader,           &publisher, &subscriber, &readcond),
         CU_DataPoints(dds_entity_t*, &participant, &topic, &writer, &reader, &waitset, &publisher, &subscriber, &readcond),
-        CU_DataPoints(dds_attach_t,  (dds_attach_t)NULL, (dds_attach_t)&reader, (dds_attach_t)3),
+        CU_DataPoints(dds_attach_t,  (dds_attach_t)NULL, (dds_attach_t)&reader, (dds_attach_t)3, (dds_attach_t)0, (dds_attach_t)0, (dds_attach_t)0, (dds_attach_t)0),
 };
 CU_Theory((dds_entity_t *ws, dds_entity_t *e, dds_attach_t a), ddsc_waitset_attach, non_waitsets, .init=ddsc_waitset_init, .fini=ddsc_waitset_fini)
 {
@@ -325,7 +325,7 @@ CU_Test(ddsc_waitset_attach, deleted_waitset, .init=ddsc_waitset_basic_init, .fi
 CU_TheoryDataPoints(ddsc_waitset_attach_detach, valid_entities) = {
         CU_DataPoints(dds_entity_t*, &participant, &topic, &writer, &reader, &waitset, &publisher, &subscriber, &readcond),
         CU_DataPoints(dds_entity_t*, &participant, &topic, &writer, &reader, &waitset, &publisher, &subscriber, &readcond),
-        CU_DataPoints(dds_attach_t,  (dds_attach_t)NULL, (dds_attach_t)&reader, (dds_attach_t)3),
+        CU_DataPoints(dds_attach_t,  (dds_attach_t)NULL, (dds_attach_t)&reader, (dds_attach_t)3, (dds_attach_t)3, (dds_attach_t)3, (dds_attach_t)3, (dds_attach_t)3, (dds_attach_t)3),
 };
 CU_Theory((dds_entity_t *ws, dds_entity_t *e, dds_attach_t a), ddsc_waitset_attach_detach, valid_entities, .init=ddsc_waitset_init, .fini=ddsc_waitset_fini)
 {

src/core/ddsi/include/ddsi/ddsi_tkmap.h

@@ -36,7 +36,6 @@ struct ddsi_tkmap * ddsi_tkmap_new (void);
 void ddsi_tkmap_free (_Inout_ _Post_invalid_ struct ddsi_tkmap *tkmap);
 void ddsi_tkmap_instance_ref (_In_ struct ddsi_tkmap_instance *tk);
 uint64_t ddsi_tkmap_lookup (_In_ struct ddsi_tkmap *tkmap, _In_ const struct ddsi_serdata *serdata);
-_Check_return_ bool ddsi_tkmap_get_key (_In_ struct ddsi_tkmap * map, const struct ddsi_sertopic *topic, _In_ uint64_t iid, _Out_ void * sample);
 _Check_return_ struct ddsi_tkmap_instance * ddsi_tkmap_find(
         _In_ struct ddsi_serdata * sd,
         _In_ const bool rd,

src/core/ddsi/src/ddsi_tkmap.c

@@ -82,7 +82,7 @@ static int dds_tk_equals_void (const void *a, const void *b)
   return dds_tk_equals (a, b);
 }
 
-struct ddsi_tkmap * ddsi_tkmap_new (void)
+struct ddsi_tkmap *ddsi_tkmap_new (void)
 {
   struct ddsi_tkmap *tkmap = dds_alloc (sizeof (*tkmap));
   tkmap->m_hh = ut_chhNew (1, dds_tk_hash_void, dds_tk_equals_void, gc_buckets);
@@ -111,64 +111,32 @@ uint64_t ddsi_tkmap_lookup (_In_ struct ddsi_tkmap * map, _In_ const struct ddsi
 {
   struct ddsi_tkmap_instance dummy;
   struct ddsi_tkmap_instance * tk;
+  assert (vtime_awake_p(lookup_thread_state()->vtime));
   dummy.m_sample = (struct ddsi_serdata *) sd;
   tk = ut_chhLookup (map->m_hh, &dummy);
   return (tk) ? tk->m_iid : DDS_HANDLE_NIL;
 }
 
-typedef struct
-{
-  const struct ddsi_sertopic *topic;
-  uint64_t m_iid;
-  void * m_sample;
-  bool m_ret;
-}
-tkmap_get_key_arg;
-
-static void dds_tkmap_get_key_fn (void * vtk, void * varg)
-{
-  struct ddsi_tkmap_instance * tk = vtk;
-  tkmap_get_key_arg * arg = (tkmap_get_key_arg*) varg;
-  if (tk->m_iid == arg->m_iid)
-  {
-    ddsi_serdata_topicless_to_sample (arg->topic, tk->m_sample, arg->m_sample, 0, 0);
-    arg->m_ret = true;
-  }
-}
-
 _Check_return_
-bool ddsi_tkmap_get_key (_In_ struct ddsi_tkmap * map, const struct ddsi_sertopic *topic, _In_ uint64_t iid, _Out_ void * sample)
+struct ddsi_tkmap_instance *ddsi_tkmap_find_by_id (_In_ struct ddsi_tkmap *map, _In_ uint64_t iid)
 {
-  tkmap_get_key_arg arg = { topic, iid, sample, false };
+  /* This is not a function that should be used liberally, as it linearly scans the key-to-iid map. */
-  os_mutexLock (&map->m_lock);
+  struct ut_chhIter it;
-  ut_chhEnumUnsafe (map->m_hh, dds_tkmap_get_key_fn, &arg);
+  struct ddsi_tkmap_instance *tk;
-  os_mutexUnlock (&map->m_lock);
+  uint32_t refc;
-  return arg.m_ret;
+  assert (vtime_awake_p(lookup_thread_state()->vtime));
-}
+  for (tk = ut_chhIterFirst (map->m_hh, &it); tk; tk = ut_chhIterNext (&it))
-
+    if (tk->m_iid == iid)
-typedef struct
+      break;
-{
+  if (tk == NULL)
-  uint64_t m_iid;
+    /* Common case of it not existing at all */
-  struct ddsi_tkmap_instance * m_inst;
+    return NULL;
-}
+  else if (!((refc = os_atomic_ld32 (&tk->m_refc)) & REFC_DELETE) && os_atomic_cas32 (&tk->m_refc, refc, refc+1))
-tkmap_get_inst_arg;
+    /* Common case of it existing, not in the process of being deleted and no one else concurrently modifying the refcount */
-
+    return tk;
-static void dds_tkmap_get_inst_fn (void * vtk, void * varg)
+  else
-{
+    /* Let key value lookup handle the possible CAS loop and the complicated cases */
-  struct ddsi_tkmap_instance * tk = vtk;
+    return ddsi_tkmap_find (tk->m_sample, false, false);
-  tkmap_get_inst_arg * arg = (tkmap_get_inst_arg*) varg;
-  if (tk->m_iid == arg->m_iid)
-  {
-    arg->m_inst = tk;
-  }
-}
-
-_Check_return_
-struct ddsi_tkmap_instance * ddsi_tkmap_find_by_id (_In_ struct ddsi_tkmap * map, _In_ uint64_t iid)
-{
-  tkmap_get_inst_arg arg = { iid, NULL };
-  ut_chhEnumUnsafe (map->m_hh, dds_tkmap_get_inst_fn, &arg);
-  return arg.m_inst;
 }
 
 /* Debug keyhash generation for debug and coverage builds */
@@ -193,6 +161,7 @@ struct ddsi_tkmap_instance * ddsi_tkmap_find(
   struct ddsi_tkmap_instance * tk;
   struct ddsi_tkmap * map = gv.m_tkmap;
 
+  assert (vtime_awake_p(lookup_thread_state()->vtime));
   dummy.m_sample = sd;
 retry:
   if ((tk = ut_chhLookup(map->m_hh, &dummy)) != NULL)
@@ -242,7 +211,6 @@ retry:
 _Check_return_
 struct ddsi_tkmap_instance * ddsi_tkmap_lookup_instance_ref (_In_ struct ddsi_serdata * sd)
 {
-  assert (vtime_awake_p (lookup_thread_state ()->vtime));
   return ddsi_tkmap_find (sd, true, true);
 }