* Fix type of num reliable readers (int to int32_t)
* Conversion codes in debug monitor printf formats
* Dead code elimination
* Skipping a test case where SIZE_MAX is assumed > INT32_MAX if
assumption is false on target platform
* Error handling in os_sockWaitsetNew
* Stick to unsigned in fragment size calculations
This check is actually guarded by valid_DataFrag and was safe for
datagrams up to 2GB, but the unintended and implicit conversion to is
still best eliminated.
* A "server" connection never has an invalid socket in TCP wrapper
* Handle error return from gethostname in SPDP write (CID 248183)
* Handle extended retcodes in dds_strretcode
CID 248131, introduced by 19aec98b8a
* Remove dead code in ddsrt logging test (CID 248195)
* Validate command-line argument in process test (CID 248117)
* Allow for extremely delayed store in test
Test is constructed to have the events trigger only at the appropriate
times, but it does assume that the store to cb_called becomes visible
prior to the listener callback. I'm pretty sure that will always be
the case in practice, but I'm also pretty sure there is no formal
guarantee without a memory barrier, which mutex_unlock provides.
CID 248088, 248136, 248177, 253590, 253591, 253593
* Check unsetenv return value in test (CID 248099)
Signed-off-by: Erik Boasson <eb@ilities.com>
Initialisation of ddsrt layer uses a hand-rolled CAS/sleep device not
unlike a spin lock. This so initialisation doesn't depend on, e.g.,
ddsrt_once.
Checking or changing thread states between "awake" and "asleep" can end
up in ddsrt_init if the thread is unknown at the time of the call.
Once really only ends up in those cases when the library is initialised
already, in which case no sleeping occurs.
In any case, the sleep is just a friendly yielding of the CPU. Coverity
will still see the loop, just not the sleep.
Signed-off-by: Erik Boasson <eb@ilities.com>
In the case of when a DATA_ON_READERS listener is set, but with the
corresponding status mask is set to suppress the event, the reader lock
would not be in locked, resulting in a unlocked access of status flags
and a double unlock.
Signed-off-by: Erik Boasson <eb@ilities.com>
For targets that do not support ddsrt_setenv and ddsrt_getenv, an alternative
method is needed to supply an application specific configuration. One way to
implement this, is to add a function for creating a domain with a string
arguments, which needs to be called before any call to dds_create_participant
for given domain identifier.
The function dds_create_domain has been added, which has as arguments a domain
identifier and a configuration string. The string is treated in the same way
as the string that is retrieved from the environment variable, in that it may
containt a comma separated list of file names and/or XML fragments for the
configuration.
Two tests have been added. One limits the number of participants to two and
verifies that creating a third participant fails. The other tests checks
incorrect calls to dds_create_domain.
An assert in dds_handle_delete has been weakened.
Signed-off-by: Frans Faase <frans.faase@adlinktech.com>
Those fail with a small probability, but it is still annoying. The code
has been vetted and tested; and by disabling the test only when run in
the CI infrastructure, anyone changing the code would still have the
test run locally.
Signed-off-by: Erik Boasson <eb@ilities.com>
Rare intermittent failure appears to be timing. Increasing the timeout
doesn't affect the duration of a successful run and will still signal an
missing trigger.
Signed-off-by: Erik Boasson <eb@ilities.com>
Deleting participant does: add to "deleted participants", remove from
GUID hash table; so SPDP processing must first check for an existing
participant and check deleted participants if nothing found.
Signed-off-by: Erik Boasson <eb@ilities.com>
So dds_read/dds_take should pass it the address of the first pointer,
rather than the first pointer itself, or the freeing of memory allocated
for samples because of an outstanding loan will crash. Add a test that
reliable detects this case when no other participants are around.
Signed-off-by: Erik Boasson <eb@ilities.com>
Deleting entities concurrently with operating on them or creating child
entities should work properly, even if it is essentially abuse. This
commit fixes (most?) of them, with exception of some nastiness when
deleting the last entity, and thus deinitialising the library, in
parallel to attempting to operate on a (by definition invalid) handle.
* Interrupting a blocked operation at the beginning of "delete" is now a
separate operation. E.g., a wait call on a waitset must be interrupted,
but the data structures can't be touched yet because other threads may
be doing an attach/detach in parallel.
* DDSI writer can now be switched to an intermediate state,
"INTERRUPTED", to indicate that it should unblock any waiting threads
and refuse to transmit any further data, but without actually
embarking on the path of deleting data structures.
* The extra "pinning" of readers and writers is now gone, they remain
fully functional until the no other threads can still access the
entity.
* Future listener invocations are prevented as part of deleting the
entity, but now it also guarantees the application can no longer
re-enable them. It furthermore waits until there are no further
current or pending listener invocations, rather than simply no current
ones.
* The internal state of the waitset now has its own lock, otherwise
attaching the parent entity of the waitset can require locking the
waitset after having locking the parent, which violates locking order.
* Handles are created in a pending state, where they are not included in
a dds_get_children operation and refuse to be pinned. This makes it
possible (in a future commit) to undo deletion of complex entities.
* There is a test (ddsc_waitset_torture) that exercises some of these
corner cases.
Signed-off-by: Erik Boasson <eb@ilities.com>
This fixes a possible deadlock when detaching an entity at the same time
it is triggering: a triggering entity holds its m_observers_lock while
trying to acquire waiset::m_mutex, and so attach and detach must not do
the opposite. The deadlock had excellent reproducibility in a seemingly
unrelated ROS2 application; this changes fixes it.
Signed-off-by: Erik Boasson <eb@ilities.com>
The default RHC implementation is not always ideal and rather than
trying to squeeze everything in a fixed interface it makes more sense to
allow the caller to provide an arbitrary implementation of the
interface.
This is not yet a stable interface.
Signed-off-by: Erik Boasson <eb@ilities.com>
This makes it possible to write one's own RHC implementation. This is
not a stable interface. It shuffles a few things around and renames
some types used throughout the code to stick to having a "dds" prefix
for all the external things.
Signed-off-by: Erik Boasson <eb@ilities.com>
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>
Leak a small amount of memory when there are still application threads
around, that's slightly less problematic than the old code. It really
needs an overhaul.
Signed-off-by: Erik Boasson <eb@ilities.com>
Require that all entities attached to waitset are in the same
participant (if the waitset's parent is a participant) or in the same
domain (if the waitset's parent is a domain). There used to be no check
on this, but it always seemed unhygenic and now that domains and the
entire library instantiation can be used as parents, it seems much
better to enforce this scoping rule.
Signed-off-by: Erik Boasson <eb@ilities.com>
This commit adds two entity types: a "domain", which is the parent of
participants and which is instantiated for each domain that has at least
one participant in it; and "cyclonedds", which is a representation of
the (initialized) Cyclone DDS library in the process and that is the
parent of all domain entities. The handle of the latter is a
compile-constant, DDS_CYCLONEDDS_HANDLE.
This changes the return value from dds_get_parent when executed on a
participant: it now returns the handle of the entity representing the
domain the participant is attached to. Two participants in the same
domain self-evidently return the same domain entity.
This allows deleting all participants in a domain by calling dds_delete
on the domain entity, or tearing down everything and deinitializing the
library by calling dds_delete on the top-level entity.
Signed-off-by: Erik Boasson <eb@ilities.com>
Things go really badly wrong when topics from one participant are used
to create a reader/writer in another participant. This returns an error
if they are not.
Signed-off-by: Erik Boasson <eb@ilities.com>
* Fix compilation errors for RMW build on Windows
This commit fixes the compilation errors that occur when building
the ROS2 rmw_cyclonedds_cpp module on Windows with the msvc C++
compiler. The error are fixed by adding explicit casts when calling
operations on atomics and a different syntax is used for compound
literals (the c99 syntax is not supported in msvc in C++ mode).
In additionally some warnings related to emtpy array usage
in structs are suppressed for msvc.
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
* Restructured macros for vendor id cast and casts for atomic operations
To improve readability I've restructured the macros that are used
for vendor id casts on msvc in c++ mode and macros for type-casting
arguments in operations on atomics (as suggested in the review of
my previous commit)
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
* Moved atomic function suffix to macro
Moved the suffix for (msvc) 64 bit atomic functions to the
DDSRT_ATOMIC_OP64 macro
Signed-off-by: Dennis Potman <dennis.potman@adlinktech.com>
This commit adds a test that checks all data published by (reliable)
volatile writers is delivered to readers that existed at the time of the
writer creation, while continuously creating and deleting these writers
and relying on the writer lingering long enough for the data to be
actually delivered. It also verifies that the match counts end up at 0.
By also semi-continuously creating and deleting an additional reader in
the subscribing process, all three delivery paths (steady state, proxy
writer deletion or matching in progress, and delivery of data to
out-of-sync readers) are hit, as verified by a coverage test.
Signed-off-by: Erik Boasson <eb@ilities.com>
While commit a652ecb78e addressed most of
the issues with a new reader possibly not accepting all samples a proxy
writer published after it matched that reader, it failed to do so
properly for 2nd and later readers. The cause was, once again,
prematurely considering the reader to be "in sync" with the proxy
writer and not requesting any old samples.
The desire to consider them "in sync" from the very beginning is that it
avoids a lot of network traffic. With this commit, all new readers are
treated equally, despite the additional traffic that generates.
Signed-off-by: Erik Boasson <eb@ilities.com>
Dropping a match from a data reader generated unregisters and signalled
a SUBSCRIPTION_MATCHED event even when another thread raced it and did
so before. The consequence is possible undercounting of the number of
matched writers.
Signed-off-by: Erik Boasson <eb@ilities.com>
It currently uses process id and timestamp, but on a low-resolution
clock that might result in two subsequent initialisations in the same
process yielding the same seed, and that wasn't the intent.
Signed-off-by: Erik Boasson <eb@ilities.com>
The only place it was used for more than printing the name (which is
available via the QoS object) was in deserializing the sample, but there
is never a need to deserialize a sample if there is no local reader. So
instead of caching the topic object in the proxy endpoint, take it from
a reader to which it is to be delivered and avoid having to keep things
in sync.
Signed-off-by: Erik Boasson <eb@ilities.com>
* Add a colcon.pkg file to override the CMake package name
Without this file, colcon will take the project name from the
CMakeLists.txt file, which is `CycloneDDS`. This name does not conform
to the standard practices for a ROS package name, and should be
lowercase (i.e. `cyclonedds`).
This `colcon.pkg` will make colcon detect the package dependencies using
this name.
Signed-off-by: Scott K Logan <logans@cottsay.net>
* Drop unneeded package type
This isn't needed as colcon can detect the package type as `cmake`
without it.
Signed-off-by: Scott K Logan <logans@cottsay.net>
Setting BUILD_IDLC=OFF makes it possible to build Cyclone without
its (Java-based) IDL preprocessor. In that case there is no
IdlcGenerate.cmake, and therefore the generated CycloneDDSConfig.cmake
must not try to include it either.
Signed-off-by: Erik Boasson <eb@ilities.com>
