For compatibility with TwinOaks CoreDX, ignore an all-zero durability
service QoS received over SEDP for volatile and transient-local
endpoints.
Signed-off-by: Erik Boasson <eb@ilities.com>
The introduction of multiple receive threads could trigger the assertion
because a set of samples ready for delivery may have been received by
multiple threads (the problem manifests itself most easily with
fragmented samples). This is actually a non-issue:
* while synchronously processing a packet, there is a bias of 2**31
added to the refcount, to prevent any thread under any circumstance
from ever freeing the data;
* while data lives in the defragment buffers or reorder buffer of the
proxy writer, a bias of 2**20 is added to it until this particular
function is called, after delivery of the data to the readers, and
(if needed) after inserting the samples in the reorder buffer of
any readers that are out-of-sync with the proxy writer;
* the relevant refcount is updated atomically in such a manner that this
particular operation atomically removes the bias and performs the
delayed increment of the refcount to account for the data being stored
in any of the defragmenting or reorder buffers;
* the only ordinary decrementing of the refcount happens either
synchronously (if synchronous delivery is chosen), or asynchronously
in a delivery queue thread, and so the entire mechanism exists to
avoid premature freeing of the underlying data because the data is
delivered very quickly (possibly synchronously);
* as the biases are removed after all the delayed refcount increments
are taken into account and there are no increments following the call
to rmbias_and_adjust, the "ordinary" decrements can do no harm.
* the case of data from multiple writers being combined in a single
packet is dealt with by the 2**20 bias, and so there is potentially a
problem if there are more than 2**20 out-of-sync readers attached to
a single proxy writer, or data submessages from more than 2**11
writers in a single packet. The minimum possible data message is 32
bytes (headers, encoding, data, padding), so packets up to 64kB are
safe.
None of this is in any way related to which threads originally accepted
the packets, and therefore I see no argument for the existence of the
assertion.
That said, it is a rather complicated mechanism of unknown benefit, and
a major simplification is definitely something to be considered. In UDP
mode I see no chance of abuse, but there may be network protocols (TCP,
for sure) where there might be packets larger than 64kB and those could,
under worst-case assumptions, cause trouble. That, too, is a reason to
rethink it.
The call to rmbias_and_adjust was sometimes called with the proxy writer
locked, and sometimes after unlocking it. This commit changes it to
consistently call it with the lock held.
Signed-off-by: Erik Boasson <eb@ilities.com>
Sizing/ReceiveBufferSize must be >= Sizing/ReceiveBufferChunkSize + N
for some small N, and if it is not, Cyclone will crash reading beyond
allocated memory in a nasty way. Ordinarily this should be handled by
the configuration validation, but that would put the burden of knowing
the details of computing N upon the user, an unreasonable requirement.
The old state of an assertion presupposes a check, and brings us back
that same requirement.
Thus, a change to ensure that ReceiveBufferSize will be taken as the
minimum of the configured value and the actual minimal value as
determined by ChunkSize and whatever N happens to be.
Signed-off-by: Erik Boasson <eb@ilities.com>
During shutdown, the optional "debmon" thread for getting some
information about internal state of the DDSI stack had a tendency to run
into errors from calling write on a connection that had already been
closed immediately after connecting successfully to wake the thread.
Instead of blindly writing into the connection, it now checks whether it
is supposed to shutdown before doing anything, avoiding this particular
problem.
Signed-off-by: Erik Boasson <eb@ilities.com>
The rewrite of the abstraction layer changed some details in thread ids
used in tracing and functions to get those ids, with a result of always
printing the parent thread's id in create_thread rather than the newly
create thread's id. As all supported platforms use thread names in the
trace, it is a rather insignificant matter, and so this provides the
trivial fix by letting the new thread log the message.
Signed-off-by: Erik Boasson <eb@ilities.com>
The rtps_init function used to initialize all data structures and start
all threads used by the protocol stack, allowing discovery of remote
entities before the built-in topic data structures were initialized.
(Very) early discovery of a remote participant thus led to a crash.
This commit splits the initialisation, providing a separate function for
starting, in particular, the threads receiving data from the network.
In terms of threads created, it matches exactly with the rtps_stop /
rtps_fini split that already existed to address the exact same problem
on termination.
Signed-off-by: Erik Boasson <eb@ilities.com>
This adds an Internal/EnableExpensiveChecks setting for enabling some or all expensive run-time checks to avoid a massive slowdown when assertions are enabled at compile-time. Currently these cover only the writer and reader-history cache checking.
Signed-off-by: Erik Boasson <eb@ilities.com>
The configuration handling already allowed specifying multiple files in CYCLONEDDS_URI to be read in-order, this extends the behaviour to also allow the contents of these files to be embedded. This makes it possible to set a configuration without requiring a file system, or to add some ad-hoc options.
Signed-off-by: Erik Boasson <eb@ilities.com>
- Replace os_result by dds_retcode_t and move DDS return code defines down.
Eliminates the need to convert between different return code types.
- Move dds_time_t down and remove os_time.
Eliminates the need to convert between different time representations and
reduces code duplication.
- Remove use of Microsoft source-code annotation language (SAL).
SAL annotations are Microsoft specific and not very well documented. This
makes it very difficult for contributers to write.
- Rearrange the abstraction layer to be feature-based. The previous layout
falsely assumed that the operating system dictates which implementation is
best suited. For general purpose operating systems this is mostly true, but
embedded targets require a slightly different approach and may not even offer
all features. The new layout makes it possible to mix-and-match feature
implementations and allows for features to not be implemented at all.
- Replace the os prefix by ddsrt to avoid name collisions.
- Remove various portions of unused and unwanted code.
- Export thread names on all supported platforms.
- Return native thread identifier on POSIX compatible platforms.
- Add timed wait for condition variables that takes an absolute time.
- Remove system abstraction for errno. The os_getErrno and os_setErrno were
incorrect. Functions that might fail now simply return a DDS return code
instead.
- Remove thread-specific memory abstraction. os_threadMemGet and accompanying
functions were a mess and their use has been eliminated by other changes in
this commit.
- Replace attribute (re)defines by ddsrt_ prefixed equivalents to avoid name
collisions and problems with faulty __nonnull__ attributes.
Signed-off-by: Jeroen Koekkoek <jeroen@koekkoek.nl>
Internally time stamps and durations are all in nanoseconds, but the
platform abstraction uses {sec,nsec} (essentially a struct timespec) and
Windows uses milliseconds. The conversion to milliseconds with upwards
rounding was broken, adding ~1s to each timeout. In most of the handful
of uses the effect is minor in practice, but it does matter a lot in the
scheduling of Heartbeat and AckNack messages, e.g., by causing a simple
throughput test to exhibit periodic drops in throughput.
Signed-off-by: Erik Boasson <eb@ilities.com>
A writer blocking on a full WHC will still send out whatever it has
buffered but not sent yet. For this, the writer lock must be released,
but that means an ACK can sneak in between sending out the packet and
relocking the writer (not likely if there's a real network in between,
but over a loopback interface it is definitely possible).
Therefore, the amount of unacknowledged data that controls the blocking
and triggering of it must be refreshed before deciding to block,
otherwise it may hang indefinitely.
Signed-off-by: Erik Boasson <eb@ilities.com>
The writer tracks whether it is throttled because of a full WHC, but
does so by treating it as a simple flag. This is fine if there is at
most one thread blocked on any single writer at any time, but if there
are multiple threads using the same writer it would be possible for one
thread to be woken up, clear the flag, and so affect the wakeup of other
threads.
Turning it from a flag to a counter avoids that problem.
Signed-off-by: Erik Boasson <eb@ilities.com>
The "rhc" test runs a random sequence of operations (writes, reads, &c.)
through an RHC with conditions attached to it. All possible state masks
are used, and query conditions are tried with a condition that only
tests the key value, and one that tests attribute values. It depends on
the internal checking logic of the RHC, which is currently enabled only
in Debug builds because of the associated run-time overhead.
Signed-off-by: Erik Boasson <eb@ilities.com>
Various details:
- index replaced throughout by bitmask
- caching a single sample in RHC for deserialising samples when query conditions are used
- short-circuiting the trivial case of an instance matched neither before nor after a change to its state
- combining inc/dec counters into a single delta in condition evaluation
- major speed-up of rhc_check_counts by not checking the condition bit masks every single time
Signed-off-by: Erik Boasson <eb@ilities.com>
The specification says the data in an invalid sample must not be inspected, but that means retrieving the key values from a disposed/unregistered instance after a take is impossible. So, the elegant thing to do is to always provide the key values in the data. The remaining question is then whether the non-key fields should be left in whatever state they happened to be in, or be set to zero. The latter is more consistent in the interface, and that is almost invariably a good thing.
Signed-off-by: Erik Boasson <eb@ilities.com>
Whenever a sample or an instance is added, check it against the attached query conditions and indicate which ones match
Signed-off-by: Erik Boasson <eb@ilities.com>
This index can then be used as an index into a bitmap to keep track
which query conditions are matched by a sample or an instance.
Signed-off-by: Erik Boasson <eb@ilities.com>
The read condition and the query condition are represented by the same data type internally, and a read condition therefore has a "m_filter" attribute. It makes more sense to initialise this properly as part of the read condition, instead of initialisation-by-memset in the dds_create_readcond, then overwriting it in dds_create_querycond.
Signed-off-by: Erik Boasson <eb@ilities.com>
The RHC tracing produces so much junk that is hardly ever useful that a
normal trace should definitely not include it.
Signed-off-by: Erik Boasson <eb@ilities.com>
A read restricted to samples in "read" state would not enter the condition update code on the false assumption that no read conditions could become triggered if the number of read samples remained the same, but it is nonetheless possible that the instance was transitions from "new" to "old" as a consequence, at least in my interpretation of the spec and the current implementation of read() in Cyclone. This commit brings consistency to the implementation without the intention of confirming the current behaviour as being desirable.
Signed-off-by: Erik Boasson <eb@ilities.com>
The name change missed the uses of the macro, with the result that
datagram truncation on reception does not result in warning (but in
the default configuration, truncation cannot occur); and that the
message flags are undefined on sending datagrams, but judging by the
man page, the likelihood of this causing problems is also small in
practice.
Signed-off-by: Erik Boasson <eb@ilities.com>
