Fix g_cond_wait_until() vs. monotonic time
authorRyan Lortie <desrt@desrt.ca>
Tue, 18 Feb 2014 23:50:18 +0000 (18:50 -0500)
committerRyan Lortie <desrt@desrt.ca>
Thu, 20 Feb 2014 22:55:09 +0000 (17:55 -0500)
We've had a relatively rocky path with g_cond_wait_until() on systems
that either don't support pthread_condattr_setclock() or where
g_get_monotonic_time() is not based on CLOCK_MONOTONIC (ie: Android and
Mac OS).

Fortunately, both of these platforms seem to share
pthread_cond_timedwait_relative_np() which allows us to implement
g_cond_wait_until() without races.

With this patch, we now require that one of pthread_condattr_setclock()
or pthread_cond_timedwait_relative_np() exists.  A quick look around
suggests that this is true for all platforms that we care about.

This patch removes our use of pthread_cond_timedwait_monotonic() and
pthread_cond_timedwait_monotonic_np() which were Android-only APIs.

https://bugzilla.gnome.org/show_bug.cgi?id=673607

configure.ac
glib/gthread-posix.c

index 29c9f20..4fdd30a 100644 (file)
@@ -2235,23 +2235,14 @@ AS_IF([ test x"$have_threads" = xposix], [
              AC_DEFINE(HAVE_PTHREAD_CONDATTR_SETCLOCK,1,
                 [Have function pthread_condattr_setclock])],
             [AC_MSG_RESULT(no)])
-        AC_MSG_CHECKING(for pthread_cond_timedwait_monotonic)
+        AC_MSG_CHECKING(for pthread_cond_timedwait_relative_np)
         AC_LINK_IFELSE(
             [AC_LANG_PROGRAM(
                 [#include <pthread.h>],
-                [pthread_cond_timedwait_monotonic(NULL, NULL, NULL)])],
+                [pthread_cond_timedwait_relative_np(NULL, NULL, NULL)])],
             [AC_MSG_RESULT(yes)
-             AC_DEFINE(HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC,1,
-                [Have function pthread_cond_timedwait_monotonic])],
-            [AC_MSG_RESULT(no)])
-        AC_MSG_CHECKING(for pthread_cond_timedwait_monotonic_np)
-        AC_LINK_IFELSE(
-            [AC_LANG_PROGRAM(
-                [#include <pthread.h>],
-                [pthread_cond_timedwait_monotonic_np(NULL, NULL, NULL)])],
-            [AC_MSG_RESULT(yes)
-             AC_DEFINE(HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC_NP,1,
-                [Have function pthread_cond_timedwait_monotonic_np])],
+             AC_DEFINE(HAVE_PTHREAD_COND_TIMEDWAIT_RELATIVE_NP,1,
+                [Have function pthread_cond_timedwait_relative_np])],
             [AC_MSG_RESULT(no)])
         CPPFLAGS="$glib_save_CPPFLAGS"
 ])
index 374cded..4a01f4d 100644 (file)
@@ -45,6 +45,7 @@
 #include "gslice.h"
 #include "gmessages.h"
 #include "gstrfuncs.h"
+#include "gmain.h"
 
 #include <stdlib.h>
 #include <stdio.h>
@@ -853,19 +854,41 @@ g_cond_wait_until (GCond  *cond,
   struct timespec ts;
   gint status;
 
-  ts.tv_sec = end_time / 1000000;
-  ts.tv_nsec = (end_time % 1000000) * 1000;
+#ifdef HAVE_PTHREAD_COND_TIMEDWAIT_RELATIVE_NP
+  /* end_time is given relative to the monotonic clock as returned by
+   * g_get_monotonic_time().
+   *
+   * Since this pthreads wants the relative time, convert it back again.
+   */
+  {
+    gint64 now = g_get_monotonic_time ();
+    gint64 relative;
 
-#if defined(HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC)
-  if ((status = pthread_cond_timedwait_monotonic (g_cond_get_impl (cond), g_mutex_get_impl (mutex), &ts)) == 0)
-    return TRUE;
-#elif defined(HAVE_PTHREAD_COND_TIMEDWAIT_MONOTONIC_NP)
-  if ((status = pthread_cond_timedwait_monotonic_np (g_cond_get_impl (cond), g_mutex_get_impl (mutex), &ts)) == 0)
-    return TRUE;
+    if (end_time <= now)
+      return FALSE;
+
+    relative = end_time - now;
+
+    ts.tv_sec = relative / 1000000;
+    ts.tv_nsec = (relative % 1000000) * 1000;
+
+    if ((status = pthread_cond_timedwait_relative_np (g_cond_get_impl (cond), g_mutex_get_impl (mutex), &ts)) == 0)
+      return TRUE;
+  }
+#elif defined (HAVE_PTHREAD_CONDATTR_SETCLOCK) && defined (CLOCK_MONOTONIC)
+  /* This is the exact check we used during init to set the clock to
+   * monotonic, so if we're in this branch, timedwait() will already be
+   * expecting a monotonic clock.
+   */
+  {
+    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), &ts)) == 0)
+      return TRUE;
+  }
 #else
-  /* Pray that the cond is actually using the monotonic clock */
-  if ((status = pthread_cond_timedwait (g_cond_get_impl (cond), g_mutex_get_impl (mutex), &ts)) == 0)
-    return TRUE;
+#error Cannot support GCond on your platform.
 #endif
 
   if G_UNLIKELY (status != ETIMEDOUT)