Improve comments on condition variables and clocks

Signed-off-by: Erik Boasson <[email protected]>

Author: eboasson

src/core/ddsc/src/dds__types.h

@@ -474,6 +474,8 @@ typedef struct dds_waitset {
      acquired while holding an ancestor's lock, but a waitset must be capable of triggering on
      events on its parent */
   ddsrt_mutex_t wait_lock;
+
+  /* etime: dds_waitset_wait timeout */
   ddsrt_cond_etime_t wait_cond;
   size_t nentities;         /* [wait_lock] */
   size_t ntriggered;        /* [wait_lock] */

src/core/ddsi/include/dds/ddsi/ddsi_domaingv.h

@@ -309,15 +309,15 @@ struct ddsi_domaingv {
   ddsrt_avl_tree_t typelib;
   ddsrt_avl_tree_t typedeps;
   ddsrt_avl_tree_t typedeps_reverse;
-  ddsrt_cond_etime_t typelib_resolved_cond;
+  ddsrt_cond_etime_t typelib_resolved_cond; // etime: create_topic_descriptor timeout
 #endif
 #ifdef DDS_HAS_TOPIC_DISCOVERY
   ddsrt_mutex_t topic_defs_lock;
   struct ddsrt_hh *topic_defs;
 #endif
 
   ddsrt_mutex_t new_topic_lock;
-  ddsrt_cond_etime_t new_topic_cond;
+  ddsrt_cond_etime_t new_topic_cond; // etime: find_topic timeout
   uint32_t new_topic_version;
 
   /* security globals */

src/core/ddsi/include/dds/ddsi/ddsi_endpoint.h

@@ -72,7 +72,7 @@ struct ddsi_writer
   struct ddsi_endpoint_common c;
   ddsi_status_cb_t status_cb;
   void * status_cb_entity;
-  ddsrt_cond_mtime_t throttle_cond; /* used to trigger a transmit thread blocked in throttle_writer() or wait_for_acks() */
+  ddsrt_cond_mtime_t throttle_cond; /* FIXME: mtime is historical, etime should be better because of max_blocking_time; used to trigger a transmit thread blocked in throttle_writer() or wait_for_acks() */
   ddsi_seqno_t seq; /* last sequence number (transmitted seqs are 1 ... seq, 0 when nothing published yet) */
   seq_xmit_t seq_xmit; /* last sequence number actually transmitted */
   ddsi_seqno_t min_local_readers_reject_seq; /* mimum of local_readers->last_deliv_seq */

src/core/ddsi/src/ddsi_threadmon.c

@@ -45,7 +45,7 @@ struct ddsi_threadmon {
   bool noprogress_log_stacktraces;
 
   ddsrt_mutex_t lock;
-  ddsrt_cond_mtime_t cond;
+  ddsrt_cond_mtime_t cond; // mtime: thread progress is absent when sleeping
   struct ddsi_thread_state *thrst;
   struct ddsrt_hh *domains;
 };

src/core/ddsi/src/ddsi_xevent.c

@@ -103,6 +103,11 @@ struct ddsi_xeventq {
   struct ddsi_thread_state *thrst;
   struct ddsi_domaingv *gv;
   ddsrt_mutex_t lock;
+  
+  // Use monotonic clock to be independent of time jumps and because all timings was
+  // using the monotonic clock already. Using etime would cause the least delay for
+  // protocol messages that should have been sent while sleeping, but at the time scale
+  // at which the protocol operates, it probably doesn't matter
   ddsrt_cond_mtime_t cond;
 
   size_t cum_rexmit_bytes;

src/ddsrt/include/dds/ddsrt/sync.h

@@ -105,7 +105,7 @@ ddsrt_cond_destroy(
 ddsrt_nonnull_all;
 
 /**
- * @brief Initialize a condition variable.
+ * @brief Initialize a condition variable bound to the wall clock.
  *
  * @param[in]  cond  Condition variable to initialize.
  */
@@ -115,7 +115,7 @@ ddsrt_cond_wctime_init(
 ddsrt_nonnull_all;
 
 /**
- * @brief Destroy a condition variable.
+ * @brief Destroy a condition variable bound to the wall clock.
  *
  * @param[in]  cond  Condition variable to destroy.
  */
@@ -125,7 +125,7 @@ ddsrt_cond_wctime_destroy(
 ddsrt_nonnull_all;
 
 /**
- * @brief Initialize a condition variable.
+ * @brief Initialize a condition variable bound to the monotonic clock.
  *
  * @param[in]  cond  Condition variable to initialize.
  */
@@ -135,7 +135,7 @@ ddsrt_cond_mtime_init(
 ddsrt_nonnull_all;
 
 /**
- * @brief Destroy a condition variable.
+ * @brief Destroy a condition variable bound to the monotonic clock.
  *
  * @param[in]  cond  Condition variable to destroy.
  */
@@ -145,7 +145,7 @@ ddsrt_cond_mtime_destroy(
 ddsrt_nonnull_all;
 
 /**
- * @brief Initialize a condition variable.
+ * @brief Initialize a condition variable bound to the elasped clock.
  *
  * @param[in]  cond  Condition variable to initialize.
  */
@@ -155,7 +155,7 @@ ddsrt_cond_etime_init(
 ddsrt_nonnull_all;
 
 /**
- * @brief Destroy a condition variable.
+ * @brief Destroy a condition variable bound to the elapsed clock.
  *
  * @param[in]  cond  Condition variable to destroy.
  */
@@ -181,7 +181,7 @@ ddsrt_cond_wait(
 ddsrt_nonnull_all;
 
 /**
- * @brief Wait for a condition variable to be signalled.
+ * @brief Wait for a condition variable bound to the wall clock to be signalled.
  *
  * @param[in]  cond   Condition variable to block on.
  * @param[in]  mutex  Mutex to associate with condition variable.
@@ -197,7 +197,7 @@ ddsrt_cond_wctime_wait(
 ddsrt_nonnull_all;
 
 /**
- * @brief Wait for a condition variable to be signalled.
+ * @brief Wait for a condition variable bound to the monotonic clock to be signalled.
  *
  * @param[in]  cond   Condition variable to block on.
  * @param[in]  mutex  Mutex to associate with condition variable.
@@ -213,7 +213,7 @@ ddsrt_cond_mtime_wait(
 ddsrt_nonnull_all;
 
 /**
- * @brief Wait for a condition variable to be signalled.
+ * @brief Wait for a condition variable bound to the elapsed clock to be signalled.
  *
  * @param[in]  cond   Condition variable to block on.
  * @param[in]  mutex  Mutex to associate with condition variable.
@@ -229,7 +229,10 @@ ddsrt_cond_etime_wait(
 ddsrt_nonnull_all;
 
 /**
- * @brief Wait until @abstime for a condition variable to be signalled.
+ * @brief Wait until @abstime for a condition variable bound to the wall clock to be signalled.
+ *
+ * For platforms that only provide relative timeouts, the function will convert the absolute timeout
+ * to a relative one by subtracting the current time according to the wall clock.
  *
  * @param[in]  cond     Condition variable to block on.
  * @param[in]  mutex    Mutex to associate with condition variable.
@@ -250,7 +253,13 @@ ddsrt_cond_wctime_waituntil(
 ddsrt_nonnull((1,2));
 
 /**
- * @brief Wait until @abstime for a condition variable to be signalled.
+ * @brief Wait until @abstime for a condition variable bound to the monotonic clock to be signalled.
+ *
+ * For platforms that only provide relative timeouts, the function will convert the absolute timeout
+ * to a relative one by subtracting the current time according to the monotonic clock. For platforms
+ * that do not support binding a condition variable to a specific clock and require an absolute timeout
+ * the timeout is calculated by converting the relative timeout to an absolute timeout on the wall clock
+ * by adding the current time according to the wall clock.
  *
  * @param[in]  cond     Condition variable to block on.
  * @param[in]  mutex    Mutex to associate with condition variable.
@@ -271,7 +280,13 @@ ddsrt_cond_mtime_waituntil(
 ddsrt_nonnull((1,2));
 
 /**
- * @brief Wait until @abstime for a condition variable to be signalled.
+ * @brief Wait until @abstime for a condition variable bound to the elapsed clock to be signalled.
+ *
+ * For platforms that only provide relative timeouts, the function will convert the absolute timeout
+ * to a relative one by subtracting the current time according to the elapsed clock. For platforms
+ * that do not support binding a condition variable to a specific clock and require an absolute timeout
+ * the timeout is calculated by converting the relative timeout to an absolute timeout on the wall clock
+ * by adding the current time according to the wall clock.
  *
  * @param[in]  cond     Condition variable to block on.
  * @param[in]  mutex    Mutex to associate with condition variable.
@@ -306,7 +321,7 @@ ddsrt_cond_signal(
 ddsrt_nonnull_all;
 
 /**
- * @brief Signal a condition variable and unblock at least one thread.
+ * @brief Signal a condition variable bound to the wall clock and unblock at least one thread.
  *
  * @param[in]  cond  Condition variable to signal.
  *
@@ -320,7 +335,7 @@ ddsrt_cond_wctime_signal(
 ddsrt_nonnull_all;
 
 /**
- * @brief Signal a condition variable and unblock at least one thread.
+ * @brief Signal a condition variable bound to the monotonic clock and unblock at least one thread.
  *
  * @param[in]  cond  Condition variable to signal.
  *
@@ -335,7 +350,7 @@ ddsrt_cond_mtime_signal(
 ddsrt_nonnull_all;
 
 /**
- * @brief Signal a condition variable and unblock at least one thread.
+ * @brief Signal a condition variable bound to the elapsed clock and unblock at least one thread.
  *
  * @param[in]  cond  Condition variable to signal.
  *
@@ -363,7 +378,7 @@ ddsrt_cond_broadcast(
 ddsrt_nonnull_all;
 
 /**
- * @brief Signal a condition variable and unblock all threads.
+ * @brief Signal a condition variable bound to the wall clock and unblock all threads.
  *
  * @param[in]  cond  Condition variable to signal.
  *
@@ -377,7 +392,7 @@ ddsrt_cond_wctime_broadcast(
 ddsrt_nonnull_all;
 
 /**
- * @brief Signal a condition variable and unblock all threads.
+ * @brief Signal a condition variable bound to the monotonic clock and unblock all threads.
  *
  * @param[in]  cond  Condition variable to signal.
  *
@@ -391,7 +406,7 @@ ddsrt_cond_mtime_broadcast(
 ddsrt_nonnull_all;
 
 /**
- * @brief Signal a condition variable and unblock all threads.
+ * @brief Signal a condition variable bound to the elapsed clock and unblock all threads.
  *
  * @param[in]  cond  Condition variable to signal.
  *

src/ddsrt/include/dds/ddsrt/time.h

@@ -116,12 +116,12 @@ DDS_EXPORT void dds_sleepfor (dds_duration_t reltime);
  * @brief Get the current time in nanoseconds since the UNIX Epoch.  Identical
  * to (ddsrt_wctime_t){dds_time()}
  *
- * @returns Curren time.
+ * @returns Current time.
  */
 DDS_EXPORT ddsrt_wctime_t ddsrt_time_wallclock(void);
 
 /**
- * @brief Get high resolution, monotonic time.
+ * @brief Get monotonic time.
  *
  * The monotonic clock is a clock with near real-time progression and can be
  * used when a high-resolution time is needed without the need for it to be
@@ -136,7 +136,7 @@ DDS_EXPORT ddsrt_wctime_t ddsrt_time_wallclock(void);
 DDS_EXPORT ddsrt_mtime_t ddsrt_time_monotonic(void);
 
 /**
- * @brief Get high resolution, elapsed (and thus monotonic) time since some
+ * @brief Get elapsed (and thus monotonic) time since some
  * fixed unspecified past time.
  *
  * The elapsed time clock is a clock with near real-time progression and can be
@@ -150,16 +150,14 @@ DDS_EXPORT ddsrt_mtime_t ddsrt_time_monotonic(void);
 DDS_EXPORT ddsrt_etime_t ddsrt_time_elapsed(void);
 
 /**
- * @brief Get high resolution, elapsed (and thus monotonic) time since some
- * fixed unspecified past time.
+ * @brief Get a high resolution, monotonic time suitable for measuring time differences.
  *
- * The elapsed time clock is a clock with near real-time progression and can be
- * used when a high-resolution suspend-aware monotonic clock is needed, without
- * having to deal with the complications of discontinuities if for example the
- * time is changed. The fixed point from which the elapsed time is returned is
- * not guaranteed to be fixed over reboots of the system.
+ * On most systems, the other time functions return a high resolution time stamp
+ * anyway and this is just an alias for a monotonic timestamp. On some systems
+ * this function provides a higher resolution. It should not be used for anything other
+ * than measuring (short) time intervals.
  *
- * @returns Elapsed time if available, otherwise return monotonic time.
+ * @returns High resolution time stamp.
  */
 DDS_EXPORT ddsrt_hrtime_t ddsrt_time_highres(void);
 

src/security/core/src/dds_security_fsm.c

@@ -63,7 +63,7 @@ struct dds_security_fsm
 struct dds_security_fsm_control
 {
   ddsrt_mutex_t lock;
-  ddsrt_cond_etime_t cond;
+  ddsrt_cond_etime_t cond; // etime: timeouts in protocol machine (but see comment for xevent)
   struct ddsi_thread_state *thrst;
   struct ddsi_domaingv *gv;
   struct dds_security_fsm *first_fsm;