From 43560b9a094775a4c695ecd161cc800d8b8eb88a Mon Sep 17 00:00:00 2001 From: Erik Boasson Date: Wed, 11 Oct 2023 17:17:31 +0200 Subject: [PATCH] Fix get_matched_publication_data on builtin topic This crashed because the built-in topics are published by a "local orphan writer" that is not contained in a participant, causing dds_get_matched_publication_data to crash on dereferencing a null pointer. Signed-off-by: Erik Boasson --- src/core/ddsc/src/dds_matched.c | 22 ++++++++++---- .../include/dds/ddsi/ddsi_endpoint_match.h | 30 ++++++++++++++++--- src/core/ddsi/src/ddsi_endpoint_match.c | 10 ++++--- 3 files changed, 48 insertions(+), 14 deletions(-) diff --git a/src/core/ddsc/src/dds_matched.c b/src/core/ddsc/src/dds_matched.c index fe3adf453c..58679b71f3 100644 --- a/src/core/ddsc/src/dds_matched.c +++ b/src/core/ddsc/src/dds_matched.c @@ -76,6 +76,8 @@ static dds_builtintopic_endpoint_t *make_builtintopic_endpoint (const ddsi_guid_ return ep; } +static const struct ddsi_entity_common null_entity_common; + dds_builtintopic_endpoint_t *dds_get_matched_subscription_data (dds_entity_t writer, dds_instance_handle_t ih) { dds_writer *wr; @@ -83,15 +85,19 @@ dds_builtintopic_endpoint_t *dds_get_matched_subscription_data (dds_entity_t wri return NULL; dds_builtintopic_endpoint_t *ret = NULL; - struct ddsi_entity_common *rdc; - struct dds_qos *rdqos; - struct ddsi_entity_common *ppc; + const struct ddsi_entity_common *rdc; + const struct dds_qos *rdqos; + const struct ddsi_entity_common *ppc; // thread must be "awake" while pointers to DDSI entities are being used struct ddsi_domaingv * const gv = &wr->m_entity.m_domain->gv; ddsi_thread_state_awake (ddsi_lookup_thread_state (), gv); if (ddsi_writer_find_matched_reader (wr->m_wr, ih, &rdc, &rdqos, &ppc)) + { + if (ppc == NULL) + ppc = &null_entity_common; ret = make_builtintopic_endpoint (&rdc->guid, &ppc->guid, ppc->iid, rdqos); + } ddsi_thread_state_asleep (ddsi_lookup_thread_state ()); dds_writer_unlock (wr); @@ -105,15 +111,19 @@ dds_builtintopic_endpoint_t *dds_get_matched_publication_data (dds_entity_t read return NULL; dds_builtintopic_endpoint_t *ret = NULL; - struct ddsi_entity_common *wrc; - struct dds_qos *wrqos; - struct ddsi_entity_common *ppc; + const struct ddsi_entity_common *wrc; + const struct dds_qos *wrqos; + const struct ddsi_entity_common *ppc; // thread must be "awake" while pointers to DDSI entities are being used struct ddsi_domaingv * const gv = &rd->m_entity.m_domain->gv; ddsi_thread_state_awake (ddsi_lookup_thread_state (), gv); if (ddsi_reader_find_matched_writer (rd->m_rd, ih, &wrc, &wrqos, &ppc)) + { + if (ppc == NULL) + ppc = &null_entity_common; ret = make_builtintopic_endpoint (&wrc->guid, &ppc->guid, ppc->iid, wrqos); + } ddsi_thread_state_asleep (ddsi_lookup_thread_state ()); dds_reader_unlock (rd); diff --git a/src/core/ddsi/include/dds/ddsi/ddsi_endpoint_match.h b/src/core/ddsi/include/dds/ddsi/ddsi_endpoint_match.h index 1e576ad6c0..a3d7e1e7c6 100644 --- a/src/core/ddsi/include/dds/ddsi/ddsi_endpoint_match.h +++ b/src/core/ddsi/include/dds/ddsi/ddsi_endpoint_match.h @@ -32,11 +32,33 @@ dds_return_t ddsi_writer_get_matched_subscriptions (struct ddsi_writer *wr, uint /** @component endpoint_matching */ dds_return_t ddsi_reader_get_matched_publications (struct ddsi_reader *rd, uint64_t *wrs, size_t nwrs); -/** @component endpoint_matching */ -bool ddsi_writer_find_matched_reader (struct ddsi_writer *wr, uint64_t ih, struct ddsi_entity_common **rdc, struct dds_qos **rdqos, struct ddsi_entity_common **ppc); +/** @brief Lookup the instance handle of a matching reader and return it, it's qos and participant + * @component endpoint_matching + * + * @note Thread state must be "awake", returned pointers are aliases; lifetime ends on leaving the "awake" state; contents may not be modified. + * + * @param[in] wr writer for which to lookup ih in the setting of matching readers + * @param[in] ih instance handle of a reader + * @param[out] rdc pointer to the common entity data of the reader + * @param[out] rdqos pointer to the QoS of the reader + * @param[out] ppc participant, may be null for an orphan reader + * @return true iff ih corresponds to a matched reader, output parameters are undefined if not + */ +bool ddsi_writer_find_matched_reader (struct ddsi_writer *wr, uint64_t ih, const struct ddsi_entity_common **rdc, const struct dds_qos **rdqos, const struct ddsi_entity_common **ppc); -/** @component endpoint_matching */ -bool ddsi_reader_find_matched_writer (struct ddsi_reader *rd, uint64_t ih, struct ddsi_entity_common **wrc, struct dds_qos **wrqos, struct ddsi_entity_common **ppc); +/** @brief Lookup the instance handle of a matching writer and return it, it's qos and participant + * @component endpoint_matching + * + * @note Thread state must be "awake", returned pointers are aliases; lifetime ends on leaving the "awake" state; contents may not be modified. + * + * @param[in] rd reader for which to lookup ih in the setting of matching writers + * @param[in] ih instance handle of a writer + * @param[out] wrc pointer to the common entity data of the writer + * @param[out] wrqos pointer to the QoS of the writer + * @param[out] ppc participant, may be null for an orphan writer + * @return true iff ih corresponds to a matched writer, output parameters are undefined if not + */ +bool ddsi_reader_find_matched_writer (struct ddsi_reader *rd, uint64_t ih, const struct ddsi_entity_common **wrc, const struct dds_qos **wrqos, const struct ddsi_entity_common **ppc); #if defined (__cplusplus) } diff --git a/src/core/ddsi/src/ddsi_endpoint_match.c b/src/core/ddsi/src/ddsi_endpoint_match.c index 567fc0d5c0..4d20694997 100644 --- a/src/core/ddsi/src/ddsi_endpoint_match.c +++ b/src/core/ddsi/src/ddsi_endpoint_match.c @@ -1864,7 +1864,7 @@ dds_return_t ddsi_reader_get_matched_publications (struct ddsi_reader *rd, uint6 return (dds_return_t) nwrs_act; } -bool ddsi_writer_find_matched_reader (struct ddsi_writer *wr, uint64_t ih, struct ddsi_entity_common **rdc, struct dds_qos **rdqos, struct ddsi_entity_common **ppc) +bool ddsi_writer_find_matched_reader (struct ddsi_writer *wr, uint64_t ih, const struct ddsi_entity_common **rdc, const struct dds_qos **rdqos, const struct ddsi_entity_common **ppc) { /* FIXME: this ought not be so inefficient */ struct ddsi_domaingv *gv = wr->e.gv; @@ -1895,14 +1895,15 @@ bool ddsi_writer_find_matched_reader (struct ddsi_writer *wr, uint64_t ih, struc found = true; *rdc = &rd->e; *rdqos = rd->xqos; - *ppc = &rd->c.pp->e; + // The day orphan readers are introduced in addition to orphan writers, rd->c.pp may be a null pointer + *ppc = rd->c.pp ? &rd->c.pp->e : NULL; } } ddsrt_mutex_unlock (&wr->e.lock); return found; } -bool ddsi_reader_find_matched_writer (struct ddsi_reader *rd, uint64_t ih, struct ddsi_entity_common **wrc, struct dds_qos **wrqos, struct ddsi_entity_common **ppc) +bool ddsi_reader_find_matched_writer (struct ddsi_reader *rd, uint64_t ih, const struct ddsi_entity_common **wrc, const struct dds_qos **wrqos, const struct ddsi_entity_common **ppc) { /* FIXME: this ought not be so inefficient */ struct ddsi_domaingv *gv = rd->e.gv; @@ -1933,7 +1934,8 @@ bool ddsi_reader_find_matched_writer (struct ddsi_reader *rd, uint64_t ih, struc found = true; *wrc = &wr->e; *wrqos = wr->xqos; - *ppc = &wr->c.pp->e; + // Orphan writers have no participant + *ppc = wr->c.pp ? &wr->c.pp->e : NULL; } } ddsrt_mutex_unlock (&rd->e.lock);