niklas/rmw_cyclonedds
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Address "Precondition not met" on rmw_create_node (#65) (#66)

Browse source 
Cyclone's original code for dds_create_domain() appears to create a
domain entity that is automatically deleted when the last attached
participant disappears.  In reality, it leaks a reference while
returning DDS_RETCODE_OK, making it appear as-if it is a regular entity
that remains into existence until explicitly deleted.

The RMW code assumed that it would be automatically deleted when the
last node was destroyed and that a subsequent call to rmw_create_node
could create the domain anew.  This then fails with "precondition not
met".

In an upcoming fixed version of dds_create_domain() the domain entities
will behave normally (return a handle, require an explicit delete).

This commit provides a workaround for the bug in the original
implementation: by recovering the handle from the first participant the
preceding commits that were intended to future-proof the code will
ensure that the entity now gets deleted explicitly.

Signed-off-by: Erik Boasson <eb@ilities.com>
This commit is contained in:
eboasson 2019-11-11 21:28:53 +01:00 • committed by Dirk Thomas
parent 096d4643c8
commit 0ca71446a9
1 changed files with 15 additions and 3 deletions
Download patch file Download diff file Expand all files Collapse all files

18
rmw_cyclonedds_cpp/src/rmw_node.cpp
View file

@ -548,9 +548,6 @@ static void unref_ppant()
#if SUPPORT_LOCALHOST
static void node_gone_from_domain_locked(dds_domainid_t did)
{
/* The initial support for dds_create_domain in Cyclone results in domains that get
automatically deleted when the last participant in it disappears. Later versions
return a handle and leave it in existence. */
CddsDomain & dom = gcdds.domains[did];
assert(dom.n_nodes > 0);
if (--dom.n_nodes == 0) {
@ -679,6 +676,21 @@ extern "C" rmw_node_t * rmw_create_node(
"rmw_create_node: failed to create DDS participant");
return nullptr;
}
#if SUPPORT_LOCALHOST
if (gcdds.domains[did].domain_handle == 0) {
/* Original implementation of dds_create_domain(): returned value is 0 (DDS_RETCODE_OK)
and a reference to the domain is leaked, necessitating an explicit call to
dds_delete(). For this, we must recover the handle from the participant. Yikes!
Pending fixes to Cyclone (https://github.com/eclipse-cyclonedds/cyclonedds/pull/304):
a handle is returned by dds_create_domain() and dds_delete() must be called (so
entirely in line with other entity types). In this case, we don't have to do this
(though there would be no harm in it either).
Note that "domains_lock" is held all this time. */
gcdds.domains[did].domain_handle = dds_get_parent(pp);
}
#endif
/* Since ROS2 doesn't require anything fancy from DDS Subscribers or Publishers, create a single
pair & reuse that */