From 4033c616ff23eb1e647a0b0cd13ac54f28e1242b Mon Sep 17 00:00:00 2001 From: Ryan Lortie Date: Thu, 13 Oct 2011 23:24:23 -0400 Subject: [PATCH] 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 | 2 +- glib/deprecated/gthread-deprecated.c | 46 ++++++++ glib/deprecated/gthread.h | 12 ++- glib/glib.symbols | 2 +- glib/gthread-posix.c | 148 ++++++++++++++------------ glib/gthread-win32.c | 30 +----- glib/gthread.c | 42 +++++--- glib/gthread.h | 5 +- 8 files changed, 167 insertions(+), 120 deletions(-) diff --git a/docs/reference/glib/glib-sections.txt b/docs/reference/glib/glib-sections.txt index 0e34cb081..bc3ec1ca6 100644 --- a/docs/reference/glib/glib-sections.txt +++ b/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 diff --git a/glib/deprecated/gthread-deprecated.c b/glib/deprecated/gthread-deprecated.c index a669e50e7..8e3ec1539 100644 --- a/glib/deprecated/gthread-deprecated.c +++ b/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: */ diff --git a/glib/deprecated/gthread.h b/glib/deprecated/gthread.h index fcecd1ec3..7a5c32718 100644 --- a/glib/deprecated/gthread.h +++ b/glib/deprecated/gthread.h @@ -270,13 +270,17 @@ GLIB_VAR gboolean g_threads_got_initialized; #endif GLIB_DEPRECATED -GMutex * g_mutex_new (void); +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); +GCond * g_cond_new (void); GLIB_DEPRECATED -void g_cond_free (GCond *cond); +void g_cond_free (GCond *cond); +GLIB_DEPRECATED +gboolean g_cond_timed_wait (GCond *cond, + GMutex *mutex, + GTimeVal *timeval); G_END_DECLS diff --git a/glib/glib.symbols b/glib/glib.symbols index 48738fe34..62a46552e 100644 --- a/glib/glib.symbols +++ b/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 diff --git a/glib/gthread-posix.c b/glib/gthread-posix.c index 1f9a8133a..8b7372f61 100644 --- a/glib/gthread-posix.c +++ b/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 - * allocated on the stack, or as part of a larger structure. - * It is not necessary to initialize a #GCond that has been - * statically allocated. + * This function is useful to initialise a #GCond that has been + * allocated as part of a larger structure. It is not necessary to + * initialise a #GCond that has been 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 - * before falling asleep and locked again before resuming. + * Atomically releases @mutex and waits until @cond is signalled. * - * This function can be used even if g_thread_init() has not yet been - * called, and, in that case, will immediately return. - */ + * When using condition variables, it is possible that a spurious wakeup + * 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 - * until the time specified by @abs_time. The @mutex is unlocked before - * falling asleep and locked again before resuming. + * Waits until either @cond is signalled or @end_time has passed. * - * 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 - * called, and, in that case, will immediately return %TRUE. + * %TRUE is returned if the condition variable was signalled (or in the + * 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() - * and g_time_val_add() can be used. + * The following code shows how to correctly perform a timed wait on a + * condition variable (extended the example presented in the + * documentation for #GCond): * - * Returns: %TRUE if @cond was signalled, or %FALSE on timeout - */ -gboolean -g_cond_timed_wait (GCond *cond, - GMutex *mutex, - GTimeVal *abs_time) -{ - 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 + * |[ + * gpointer + * pop_data_timed (void) + * { + * gint64 end_time; + * gpointer data; * - * A variant of g_cond_timed_wait() that takes @abs_time - * as a #gint64 instead of a #GTimeVal. - * See g_cond_timed_wait() for details. + * g_mutex_lock (&data_mutex); * - * 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, - GMutex *mutex, - gint64 abs_time) +g_cond_wait_until (GCond *cond, + GMutex *mutex, + gint64 end_time) { - struct timespec end_time; + struct timespec ts; gint status; - end_time.tv_sec = abs_time / 1000000; - end_time.tv_nsec = (abs_time % 1000000) * 1000; + ts.tv_sec = end_time / 1000000; + 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) diff --git a/glib/gthread-win32.c b/glib/gthread-win32.c index 5199f9b72..38c600e2c 100644 --- a/glib/gthread-win32.c +++ b/glib/gthread-win32.c @@ -301,9 +301,9 @@ g_cond_wait (GCond *cond, } gboolean -g_cond_timedwait (GCond *cond, - GMutex *entered_mutex, - gint64 abs_time) +g_cond_wait_until (GCond *cond, + GMutex *entered_mutex, + 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; diff --git a/glib/gthread.c b/glib/gthread.c index 331b471ff..af27110b6 100644 --- a/glib/gthread.c +++ b/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. + * * * * Using GCond to block a thread until a condition is satisfied * * - * 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_mutex_unlock (data_mutex); + * g_cond_signal (&data_cond); + * 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(). * - * It is important to use the g_cond_wait() and - * g_cond_timed_wait() functions only inside a loop which checks for the - * condition to be true. It is not guaranteed that the waiting thread - * will find the condition fulfilled after it wakes up, even if the - * signaling thread left the condition in that state: another thread may - * have altered the condition before the waiting thread got the chance - * to be woken up, even if the condition itself is protected by a - * #GMutex, like above. + * The example shows that use of a condition variable must always be + * paired with a mutex. Without the use of a mutex, there would be a + * race between the check of current_data by the + * while loop in pop_data and waiting. + * Specifically, another thread could set pop_data + * after the check, and signal the cond (with nobody waiting on it) + * before the first thread goes to sleep. #GCond is specifically useful + * 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 diff --git a/glib/gthread.h b/glib/gthread.h index 6afffa1cc..c3b0f0c95 100644 --- a/glib/gthread.h +++ b/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, - GMutex *mutex, - GTimeVal *timeval); -gboolean g_cond_timedwait (GCond *cond, +gboolean g_cond_wait_until (GCond *cond, GMutex *mutex, gint64 abs_time);