CMAKE_PROJECT_NAME refers to the top-level project name, not the most recent project. So any CMake project that pulls this in as a dependency was in for a nasty surprise.
https://cmake.org/cmake/help/latest/variable/CMAKE_PROJECT_NAME.html
Signed-off-by: Dan Rose <dan@digilabs.io>
* 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>
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>
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>
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>
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>
Submit to the tyranny of the majority:
The DCPS specification makes no distinction between topics with and
topics without key fields: in the latter case, there is simply a single
instance but that instance obeys all the normal rules. In particular,
this implies dispose/unregister work regardless of whether a topic has
key fields.
The DDSI specification makes a distinction between WITH_KEY endpoints
and NO_KEY endpoints and does not support dispose/unregister operations
on NO_KEY endpoints. This implies that a DCPS implementation must limit
itself to the use of WITH_KEY endpoints.
Most implementations nonetheless map topics without key fields to the
NO_KEY type, and as the DDSI specification also states that a WITH_KEY
reader/writer does not match a NO_KEY writer/reader, Cyclone's correctly
mapping everything to WITH_KEY means there are interoperability problems
for topics without key fields.
This commit changes Cyclone to use NO_KEY like the others, but without
changing any other part of its behaviour: it continues to support
dispose/unregister operations regardless of whether a topic has key
fields or not. That is the lesser of the two evils.
Signed-off-by: Erik Boasson <eb@ilities.com>
Change the structure of the configuration file (in a backwards
compatible manner) to allow specifying configurations for multiple
domains in a file. (Listing multiple files in CYCLONEDDS_URI was
already supported.) A configuration specifies an id, with a default of
any, configurations for an incompatible id are ignored.
If the application specifies an id other than DDS_DOMAIN_DEFAULT in the
call to create_participant, then only configuration specifications for
Domain elements with that id or with id "any" will be used. If the
application does specify DDS_DOMAIN_DEFAULT, then the id will be taken
from the first Domain element that specifies an id. If none do, the
domain id defaults to 0. Each applicable domain specification is taken
as a separate source and may override settings made previously.
All settings moved from the top-level CycloneDDS element to the
CycloneDDS/Domain element. The CycloneDDS/Domain/Id element moved to
become the "id" attribute of CycloneDDS/Domain. The old locations still
work, with appropriate deprecation warnings.
Signed-off-by: Erik Boasson <eb@ilities.com>
The default participant QoS/plist that is used for defaulting received
QoS and for determining which QoS/plist entries to send in discovery
data was mixed up with the one that contains local process information
such as hostname and process id.
It moreover was modified after starting up the protocol stack, and hence
after discovery of remote participants. While unlikely, this could lead
to an assertion in plist_or_xqos_mergein_missing.
Signed-off-by: Erik Boasson <eb@ilities.com>
A QoS change can happen at the same time that a new reader for a
built-in topic is provisioned with historical data, and so cause reading
in inconsistent QoS, use-after-free or other fun things.
During QoS matching it is also necessary to guarantee the QoS doesn't
change (QoS changes affecting matching will be supported at some point,
and manipulating complex data structures where bitmasks determine which
parts are defined while reading the same data concurrently is a recipe
for disaster.
Signed-off-by: Erik Boasson <eb@ilities.com>
The big issue is the there is still only a single log output that gets
opened on creating a domain and closed on deleting one, but otherwise at
least this minimal test works.
The other issue is that the GC waits until threads in all domains have
made sufficient progress, rather than just the threads in its own
domain.
Signed-off-by: Erik Boasson <eb@ilities.com>
This commit moves all but a handful of the global variables into the
domain object, in particular including the DDSI configuration, globals
and all transport internal state.
The goal of this commit is not to produce the nicest code possible, but
to get a working version that can support multiple simultaneous domains.
Various choices are driven by this desire and it is expected that some
of the changes will have to be undone. (E.g., passing the DDSI globals
into address set operations and locator printing because there is no
other way to figure out what transport to use for a given locator;
storing the transport pointer inside the locator would solve that.)
Signed-off-by: Erik Boasson <eb@ilities.com>
Thread liveliness monitoring moves to dds_global and there is one
monitor running if there is at least one domain that requests it. The
synchronization over freeing the thread name when reaping the thread
state is gone by no longer dynamically allocating the thread name.
Signed-off-by: Erik Boasson <eb@ilities.com>
This moves DDSI stack initialisation and finalisation to the creating
and deleting of a domain, and modifies the related code to trigger all
that from creating/deleting participants.
Built-in topic generation is partially domain-dependent, so that moves
as well. The underlying ddsi_sertopics can be created are domain
independent and created without initialising DDSI, which necessitates
moving the IID generation (and thus init/fini) out of the DDSI stack and
to what will remain global data.
Signed-off-by: Erik Boasson <eb@ilities.com>
This makes it possible to use a different RHC implementations for
different readers and removes the need for the RHC interface to be part
of the global state.
Signed-off-by: Erik Boasson <eb@ilities.com>
The payload in a struct serdata_default is assumed to be at a 64-bit
offset for conversion to/from a dds_{i,o}stream_t and getting padding
calculations in the serialised representation correct. The definition
did not guarantee this and got it wrong on a 32-bit release build.
This commit computes the required padding at compile time and at
verifies the assumption holds where it matters.
Signed-off-by: Erik Boasson <eb@ilities.com>
Signed-off-by: Thijs Sassen <thijs.sassen@adlinktech.com>
Adjusted the close methode not to expand by the lwip close macro and added a check for DDSI_INCLUDE_SSM to match the correct pid table size.
Signed-off-by: Thijs Sassen <thijs.sassen@adlinktech.com>
Multiple writers for a single instance is pretty rare, so it makes sense
to lazily allocate the tables for keeping track of them. The more
elegant solution would be to have a single lock-free table.
Signed-off-by: Erik Boasson <eb@ilities.com>
Rather than allocate a HH_HOP_RANGE large array of buckets, allocate
just 1 if the initial size is 1, then jump to HH_HOP_RANGE as soon as a
second element is added to the table. There are quite a few cases where
hash tables are created where there never be more than 1 (or even 0)
elements in the table (e.g., a writer without readers, a reader for a
keyless topic).
Signed-off-by: Erik Boasson <eb@ilities.com>
There were inconsistencies in the order in which entity locks were taken
when multiple entities needed to be locked at the same time. In most
cases, the order was first locking entity X, then locking the parent
entity of X. However, in some cases the order was reversed, a likely
cause of deadlocks.
This commit sorts these problems, and in particular propagating
operations into children. The entity refcount is now part of the handle
administration so that it is no longer necessary to lock an entity to
determine whether it is still allowed to be used (previously it had to
check the CLOSED flag afterward). This allows recursing into the
children while holding handles and the underlying objects alive, but
without violating lock order.
Attendant changes that would warrant there own commits but are too hard
to split off:
* Children are now no longer in a singly linked list, but in an AVL
tree; this was necessary at some intermediate stage to allow unlocking
an entity and restarting iteration over all children at the "next"
child (all thanks to the eternally unique instance handle);
* Waitsets shifted to using arrays of attached entities instead of
linked lists; this was a consequence of dealing with some locking
issues in reading triggers and considering which operations on the
"triggered" and "observed" sets are actually needed.
* Entity status flags and waitset/condition trigger counts are now
handled using atomic operations. Entities are now classified as
having a "status" with a corresponding mask, or as having a "trigger
count" (conditions). As there are fewer than 16 status bits, the
status and its mask can squeeze into the same 32-bits as the trigger
count. These atomic updates avoid the need for a separate lock just
for the trigger/status values and results in a significant speedup
with waitsets.
* Create topic now has a more rational behaviour when multiple
participants attempt to create the same topic: each participant now
gets its own topic definition, but the underlying type representation
is shared.
Signed-off-by: Erik Boasson <eb@ilities.com>
Add the instance handle to the DDSC entity type, initialize it properly
for all types, and remove the per-type handling of
dds_get_instance_handle. Those entities that have a DDSI variant take
the instance handle from DDSI (which plays tricks to get the instance
handles of the entities matching the built-in topics). For those that
do not have a DDSI variant, just generate a unique identifier using the
same generate that DDSI uses.
Signed-off-by: Erik Boasson <eb@ilities.com>
Thread sanitizer warns about reads and writes of variables that are
meant to be read without holding a lock:
* Global "keep_going" is now a ddsrt_atomic_uint32_t
* Thread "vtime" is now a ddsrt_atomic_uint32_t
Previously the code relied on the assumption that a 32-bit int would be
treated as atomic, now that is all wrapped in ddsrt_atomic_{ld,st}32.
These being inline functions doing exactly the same thing, there is no
functional change, but it does allow annotating the loads and stores for
via function attributes on the ddsrt_atomic_{ld,st}X.
The concurrent hashtable implementation is replaced by a locked version
of the non-concurrent implementation if thread sanitizer is used. This
changes eliminates the scores of problems signalled by thread sanitizer
in the GUID-to-entity translation and the key-to-instance id lookups.
Other than that, this replaces a flag used in a waitset test case to be
a ddsrt_atomic_uint32_t.
Signed-off-by: Erik Boasson <eb@ilities.com>
* calling ddsrt_memdup, ddsrt_strdup with a null pointer (they handle it
gracefully but forbid it in the interface ...)
* replacement of all pre-C99 flexible arrays (i.e., declaring as
array[1], then mallocing and using as if array[N]) by C99 flexible
arrays.
* also add a missing null-pointer test in dds_dispose_ts, and fix the
test cases that pass a null pointer and a non-writer handle to it to
instead pass an invalid adress
Signed-off-by: Erik Boasson <eb@ilities.com>
* dds_set_allocator
* dds_set_aligned_allocator
The intent behind them is good, but the approach too primitive ... there
is far more work to be done for managing dynamic allocation in a
meaningful way.
Signed-off-by: Erik Boasson <eb@ilities.com>
