GCond: use monotonic time for timed waits

Switch GCond to using monotonic time for timed waits by introducing a
new API based on monotonic time in a gint64: g_cond_wait_until().

Deprecate the old API based on wallclock time in a GTimeVal.

Fix up the gtk-doc for GCond while we're at it: update the examples to
use static-allocated GCond and GMutex and clarify some things a bit.
Also explain the rationale behind using an absolute time instead of a
relative time.

docs/reference/glib/glib-sections.txt

@@ -638,7 +638,7 @@ g_cond_init
 g_cond_clear
 g_cond_wait
 g_cond_timed_wait
-g_cond_timedwait
+g_cond_wait_until
 g_cond_signal
 g_cond_broadcast
 

glib/deprecated/gthread-deprecated.c

@@ -1523,5 +1523,51 @@ g_cond_free (GCond *cond)
   g_slice_free (GCond, cond);
 }
 
+/**
+ * g_cond_timed_wait:
+ * @cond: a #GCond
+ * @mutex: a #GMutex that is currently locked
+ * @abs_time: a #GTimeVal, determining the final time
+ *
+ * Waits until this thread is woken up on @cond, but not longer than
+ * until the time specified by @abs_time. The @mutex is unlocked before
+ * falling asleep and locked again before resuming.
+ *
+ * If @abs_time is %NULL, g_cond_timed_wait() acts like g_cond_wait().
+ *
+ * This function can be used even if g_thread_init() has not yet been
+ * called, and, in that case, will immediately return %TRUE.
+ *
+ * To easily calculate @abs_time a combination of g_get_current_time()
+ * and g_time_val_add() can be used.
+ *
+ * Returns: %TRUE if @cond was signalled, or %FALSE on timeout
+ * Deprecated:2.32: Use g_cond_wait_until() instead.
+ */
+gboolean
+g_cond_timed_wait (GCond    *cond,
+                   GMutex   *mutex,
+                   GTimeVal *abs_time)
+{
+  gint64 end_time;
+
+  end_time = abs_time->tv_sec;
+  end_time *= 1000000;
+  end_time += abs_time->tv_usec;
+
+#ifdef CLOCK_MONOTONIC
+  /* would be nice if we had clock_rtoffset, but that didn't seem to
+   * make it into the kernel yet...
+   */
+  end_time += g_get_monotonic_time () - g_get_real_time ();
+#else
+  /* if CLOCK_MONOTONIC is not defined then g_get_montonic_time() and
+   * g_get_real_time() are returning the same clock, so don't bother...
+   */
+#endif
+
+  return g_cond_wait_until (cond, mutex, end_time);
+}
+
 /* {{{1 Epilogue */
 /* vim: set foldmethod=marker: */

glib/deprecated/gthread.h

@@ -272,11 +272,15 @@ GLIB_VAR gboolean g_threads_got_initialized;
 GLIB_DEPRECATED
 GMutex *        g_mutex_new             (void);
 GLIB_DEPRECATED
-void                    g_mutex_free (GMutex *mutex) ;
+void            g_mutex_free            (GMutex *mutex);
 GLIB_DEPRECATED
 GCond *         g_cond_new              (void);
 GLIB_DEPRECATED
 void            g_cond_free             (GCond  *cond);
+GLIB_DEPRECATED
+gboolean        g_cond_timed_wait       (GCond          *cond,
+                                         GMutex         *mutex,
+                                         GTimeVal       *timeval);
 
 G_END_DECLS
 

glib/glib.symbols

@@ -1609,9 +1609,9 @@ g_cond_free
 g_cond_init
 g_cond_new
 g_cond_signal
-g_cond_timedwait
 g_cond_timed_wait
 g_cond_wait
+g_cond_wait_until
 g_mutex_clear
 g_mutex_free
 g_mutex_init

glib/gthread-posix.c

@@ -640,16 +640,24 @@ g_rw_lock_reader_unlock (GRWLock *rw_lock)
 static pthread_cond_t *
 g_cond_impl_new (void)
 {
+  pthread_condattr_t attr;
   pthread_cond_t *cond;
   gint status;
 
+  pthread_condattr_init (&attr);
+#ifdef CLOCK_MONOTONIC
+  pthread_condattr_setclock (&attr, CLOCK_MONOTONIC);
+#endif
+
   cond = malloc (sizeof (pthread_cond_t));
   if G_UNLIKELY (cond == NULL)
     g_thread_abort (errno, "malloc");
 
-  if G_UNLIKELY ((status = pthread_cond_init (cond, NULL)) != 0)
+  if G_UNLIKELY ((status = pthread_cond_init (cond, &attr)) != 0)
     g_thread_abort (status, "pthread_cond_init");
 
+  pthread_condattr_destroy (&attr);
+
   return cond;
 }
 
@@ -680,17 +688,16 @@ g_cond_get_impl (GCond *cond)
  * g_cond_init:
  * @cond: an uninitialized #GCond
  *
- * Initialized a #GCond so that it can be used.
+ * Initialises a #GCond so that it can be used.
  *
- * This function is useful to initialize a #GCond that has been
+ * This function is useful to initialise a #GCond that has been
- * allocated on the stack, or as part of a larger structure.
+ * allocated as part of a larger structure.  It is not necessary to
- * It is not necessary to initialize a #GCond that has been
+ * initialise a #GCond that has been statically allocated.
- * statically allocated.
  *
  * To undo the effect of g_cond_init() when a #GCond is no longer
  * needed, use g_cond_clear().
  *
- * Calling g_cond_init() on an already initialized #GCond leads
+ * Calling g_cond_init() on an already-initialised #GCond leads
  * to undefined behaviour.
  *
  * Since: 2.32
@@ -703,7 +710,7 @@ g_cond_init (GCond *cond)
 
 /**
  * g_cond_clear:
- * @cond: an initialized #GCond
+ * @cond: an initialised #GCond
  *
  * Frees the resources allocated to a #GCond with g_cond_init().
  *
@@ -726,12 +733,19 @@ g_cond_clear (GCond *cond)
  * @cond: a #GCond
  * @mutex: a #GMutex that is currently locked
  *
- * Waits until this thread is woken up on @cond. The @mutex is unlocked
+ * Atomically releases @mutex and waits until @cond is signalled.
- * before falling asleep and locked again before resuming.
  *
- * This function can be used even if g_thread_init() has not yet been
+ * When using condition variables, it is possible that a spurious wakeup
- * called, and, in that case, will immediately return.
+ * may occur (ie: g_cond_wait() returns even though g_cond_signal() was
- */
+ * not called).  It's also possible that a stolen wakeup may occur.
+ * This is when g_cond_signal() is called, but another thread acquires
+ * @mutex before this thread and modifies the state of the program in
+ * such a way that when g_cond_wait() is able to return, the expected
+ * condition is no longer met.
+ *
+ * For this reason, g_cond_wait() must always be used in a loop.  See
+ * the documentation for #GCond for a complete example.
+ **/
 void
 g_cond_wait (GCond  *cond,
              GMutex *mutex)
@@ -785,77 +799,75 @@ g_cond_broadcast (GCond *cond)
 }
 
 /**
- * g_cond_timed_wait:
+ * g_cond_wait_until:
  * @cond: a #GCond
  * @mutex: a #GMutex that is currently locked
- * @abs_time: a #GTimeVal, determining the final time
+ * @end_time: the monotonic time to wait until
  *
- * Waits until this thread is woken up on @cond, but not longer than
+ * Waits until either @cond is signalled or @end_time has passed.
- * until the time specified by @abs_time. The @mutex is unlocked before
- * falling asleep and locked again before resuming.
  *
- * If @abs_time is %NULL, g_cond_timed_wait() acts like g_cond_wait().
+ * As with g_cond_wait() it is possible that a spurious or stolen wakeup
+ * could occur.  For that reason, waiting on a condition variable should
+ * always be in a loop, based on an explicitly-checked predicate.
  *
- * This function can be used even if g_thread_init() has not yet been
+ * %TRUE is returned if the condition variable was signalled (or in the
- * called, and, in that case, will immediately return %TRUE.
+ * case of a spurious wakeup).  %FALSE is returned if @end_time has
+ * passed.
  *
- * To easily calculate @abs_time a combination of g_get_current_time()
+ * The following code shows how to correctly perform a timed wait on a
- * and g_time_val_add() can be used.
+ * condition variable (extended the example presented in the
+ * documentation for #GCond):
  *
- * Returns: %TRUE if @cond was signalled, or %FALSE on timeout
+ * |[
- */
+ * gpointer
-gboolean
+ * pop_data_timed (void)
-g_cond_timed_wait (GCond    *cond,
+ * {
-                   GMutex   *mutex,
+ *   gint64 end_time;
-                   GTimeVal *abs_time)
+ *   gpointer data;
-{
-  struct timespec end_time;
-  gint status;
-
-  if (abs_time == NULL)
-    {
-      g_cond_wait (cond, mutex);
-      return TRUE;
-    }
-
-  end_time.tv_sec = abs_time->tv_sec;
-  end_time.tv_nsec = abs_time->tv_usec * 1000;
-
-  if ((status = pthread_cond_timedwait (g_cond_get_impl (cond), g_mutex_get_impl (mutex), &end_time)) == 0)
-    return TRUE;
-
-  if G_UNLIKELY (status != ETIMEDOUT)
-    g_thread_abort (status, "pthread_cond_timedwait");
-
-  return FALSE;
-}
-
-/**
- * g_cond_timedwait:
- * @cond: a #GCond
- * @mutex: a #GMutex that is currently locked
- * @abs_time: the final time, in microseconds
  *
- * A variant of g_cond_timed_wait() that takes @abs_time
+ *   g_mutex_lock (&data_mutex);
- * as a #gint64 instead of a #GTimeVal.
- * See g_cond_timed_wait() for details.
  *
- * Returns: %TRUE if @cond was signalled, or %FALSE on timeout
+ *   end_time = g_get_monotonic_time () + 5 * G_TIME_SPAN_SECOND;
+ *   while (!current_data)
+ *     if (!g_cond_wait_until (&data_cond, &data_mutex, end_time))
+ *       {
+ *         // timeout has passed.
+ *         g_mutex_unlock (&data_mutex);
+ *         return NULL;
+ *       }
  *
+ *   // there is data for us
+ *   data = current_data;
+ *   current_data = NULL;
+ *
+ *   g_mutex_unlock (&data_mutex);
+ *
+ *   return data;
+ * }
+ * ]|
+ *
+ * Notice that the end time is calculated once, before entering the
+ * loop and reused.  This is the motivation behind the use of absolute
+ * time on this API -- if a relative time of 5 seconds were passed
+ * directly to the call and a spurious wakeup occured, the program would
+ * have to start over waiting again (which would lead to a total wait
+ * time of more than 5 seconds).
+ *
+ * Returns: %TRUE on a signal, %FALSE on a timeout
  * Since: 2.32
- */
+ **/
 gboolean
-g_cond_timedwait (GCond  *cond,
+g_cond_wait_until (GCond  *cond,
                    GMutex *mutex,
-                  gint64  abs_time)
+                   gint64  end_time)
 {
-  struct timespec end_time;
+  struct timespec ts;
   gint status;
 
-  end_time.tv_sec = abs_time / 1000000;
+  ts.tv_sec = end_time / 1000000;
-  end_time.tv_nsec = (abs_time % 1000000) * 1000;
+  ts.tv_nsec = (end_time % 1000000) * 1000;
 
-  if ((status = pthread_cond_timedwait (g_cond_get_impl (cond), g_mutex_get_impl (mutex), &end_time)) == 0)
+  if ((status = pthread_cond_timedwait (g_cond_get_impl (cond), g_mutex_get_impl (mutex), &ts)) == 0)
     return TRUE;
 
   if G_UNLIKELY (status != ETIMEDOUT)

glib/gthread-win32.c

@@ -301,9 +301,9 @@ g_cond_wait (GCond  *cond,
 }
 
 gboolean
-g_cond_timedwait (GCond  *cond,
+g_cond_wait_until (GCond  *cond,
                    GMutex *entered_mutex,
-                  gint64  abs_time)
+                   gint64  end_time)
 {
   gint64 span;
   FILETIME ft;
@@ -315,7 +315,7 @@ g_cond_timedwait (GCond  *cond,
   now -= G_GINT64_CONSTANT (116444736000000000);
   now /= 10;
 
-  span = abs_time - now;
+  span = end_time - now;
 
   if G_UNLIKELY (span < 0)
     span = 0;
@@ -326,28 +326,6 @@ g_cond_timedwait (GCond  *cond,
   return g_thread_impl_vtable.SleepConditionVariableSRW (cond, entered_mutex, span / 1000, 0);
 }
 
-gboolean
-g_cond_timed_wait (GCond    *cond,
-                   GMutex   *entered_mutex,
-                   GTimeVal *abs_time)
-{
-  if (abs_time)
-    {
-      gint64 micros;
-
-      micros = abs_time->tv_sec;
-      micros *= 1000000;
-      micros += abs_time->tv_usec;
-
-      return g_cond_timedwait (cond, entered_mutex, micros);
-    }
-  else
-    {
-      g_cond_wait (cond, entered_mutex);
-      return TRUE;
-    }
-}
-
 /* {{{1 GPrivate */
 
 typedef struct _GPrivateDestructor GPrivateDestructor;

glib/gthread.c

@@ -350,22 +350,27 @@
  * condition they signal the #GCond, and that causes the waiting
  * threads to be woken up.
  *
+ * Consider the following example of a shared variable.  One or more
+ * threads can wait for data to be published to the variable and when
+ * another thread publishes the data, it can signal one of the waiting
+ * threads to wake up to collect the data.
+ *
  * <example>
  *  <title>
  *   Using GCond to block a thread until a condition is satisfied
  *  </title>
  *  <programlisting>
- *   GCond* data_cond = NULL; /<!-- -->* Must be initialized somewhere *<!-- -->/
- *   GMutex* data_mutex = NULL; /<!-- -->* Must be initialized somewhere *<!-- -->/
  *   gpointer current_data = NULL;
+ *   GMutex data_mutex;
+ *   GCond data_cond;
  *
  *   void
  *   push_data (gpointer data)
  *   {
- *     g_mutex_lock (data_mutex);
+ *     g_mutex_lock (&data_mutex);
  *     current_data = data;
- *     g_cond_signal (data_cond);
+ *     g_cond_signal (&data_cond);
- *     g_mutex_unlock (data_mutex);
+ *     g_mutex_unlock (&data_mutex);
  *   }
  *
  *   gpointer
@@ -373,12 +378,12 @@
  *   {
  *     gpointer data;
  *
- *     g_mutex_lock (data_mutex);
+ *     g_mutex_lock (&data_mutex);
  *     while (!current_data)
- *       g_cond_wait (data_cond, data_mutex);
+ *       g_cond_wait (&data_cond, &data_mutex);
  *     data = current_data;
  *     current_data = NULL;
- *     g_mutex_unlock (data_mutex);
+ *     g_mutex_unlock (&data_mutex);
  *
  *     return data;
  *   }
@@ -389,14 +394,19 @@
  * current_data is non-%NULL, i.e. until some other thread
  * has called push_data().
  *
- * <note><para>It is important to use the g_cond_wait() and
+ * The example shows that use of a condition variable must always be
- * g_cond_timed_wait() functions only inside a loop which checks for the
+ * paired with a mutex.  Without the use of a mutex, there would be a
- * condition to be true.  It is not guaranteed that the waiting thread
+ * race between the check of <varname>current_data</varname> by the
- * will find the condition fulfilled after it wakes up, even if the
+ * while loop in <function>pop_data</function> and waiting.
- * signaling thread left the condition in that state: another thread may
+ * Specifically, another thread could set <varname>pop_data</varname>
- * have altered the condition before the waiting thread got the chance
+ * after the check, and signal the cond (with nobody waiting on it)
- * to be woken up, even if the condition itself is protected by a
+ * before the first thread goes to sleep.  #GCond is specifically useful
- * #GMutex, like above.</para></note>
+ * for its ability to release the mutex and go to sleep atomically.
+ *
+ * It is also important to use the g_cond_wait() and g_cond_wait_until()
+ * functions only inside a loop which checks for the condition to be
+ * true.  See g_cond_wait() for an explanation of why the condition may
+ * not be true even after it returns.
  *
  * If a #GCond is allocated in static storage then it can be used
  * without initialisation.  Otherwise, you should call g_cond_init() on

glib/gthread.h

@@ -179,10 +179,7 @@ void            g_cond_wait                     (GCond          *cond,
                                                  GMutex         *mutex);
 void            g_cond_signal                   (GCond          *cond);
 void            g_cond_broadcast                (GCond          *cond);
-gboolean        g_cond_timed_wait               (GCond          *cond,
+gboolean        g_cond_wait_until               (GCond          *cond,
-                                                 GMutex         *mutex,
-                                                 GTimeVal       *timeval);
-gboolean        g_cond_timedwait                (GCond          *cond,
                                                  GMutex         *mutex,
                                                  gint64          abs_time);