2006-10-26 Havoc Pennington <hp@redhat.com>

	* doc/dbus-specification.xml: clarify the UUID text slightly

	* dbus/dbus-sysdeps-pthread.c: check for and mostly abort on
	pthread errors. Add DBusMutexPThread and DBusCondVarPThread
	in preparation for being able to extend them for e.g. recursive
	mutexes.

4 files changed, 137 insertions, 45 deletions

diff --git a/ChangeLog b/ChangeLog
index e2cc9698..88708ff9 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,14 @@
 2006-10-26  Havoc Pennington  <hp@redhat.com>
 
+	* doc/dbus-specification.xml: clarify the UUID text slightly
+
+	* dbus/dbus-sysdeps-pthread.c: check for and mostly abort on
+	pthread errors. Add DBusMutexPThread and DBusCondVarPThread 
+	in preparation for being able to extend them for e.g. recursive
+	mutexes.
+
+2006-10-26  Havoc Pennington  <hp@redhat.com>
+
         * dbus/dbus-threads.[hc]: Documentation improvements. Clarify how 
 	condition variables relate to recursive mutexes.
 	
diff --git a/dbus/dbus-sysdeps-pthread.c b/dbus/dbus-sysdeps-pthread.c
index 3b7bb6d1..0b9c5913 100644
--- a/dbus/dbus-sysdeps-pthread.c
+++ b/dbus/dbus-sysdeps-pthread.c
@@ -27,81 +27,138 @@
 
 #include <sys/time.h>
 #include <pthread.h>
+#include <string.h>
 
+typedef struct {
+  pthread_mutex_t lock;
+} DBusMutexPThread;
+
+typedef struct {
+  pthread_cond_t cond;
+} DBusCondVarPThread;
+
+#define DBUS_MUTEX(m)         ((DBusMutex*) m)
+#define DBUS_MUTEX_PTHREAD(m) ((DBusMutexPThread*) m)
+
+#define DBUS_COND_VAR(c)         ((DBusCondVar*) c)
+#define DBUS_COND_VAR_PTHREAD(c) ((DBusCondVarPThread*) c)
+
+
+#define PTHREAD_CHECK(func_name, result_or_call) do {                                  \
+    int tmp = (result_or_call);                                                        \
+    if (tmp != 0) {                                                                    \
+      _dbus_warn_check_failed ("pthread function %s failed with %d %s in %s\n",        \
+                               func_name, tmp, strerror(tmp), _DBUS_FUNCTION_NAME);    \
+    }                                                                                  \
+} while (0)
+            
 static DBusMutex*
 _dbus_pthread_mutex_new (void)
 {
-  pthread_mutex_t *retval;
+  DBusMutexPThread *pmutex;
+  int result;
   
-  retval = dbus_new (pthread_mutex_t, 1);
-  if (retval == NULL)
+  pmutex = dbus_new (DBusMutexPThread, 1);
+  if (pmutex == NULL)
     return NULL;
-  
-  if (pthread_mutex_init (retval, NULL))
+
+  result = pthread_mutex_init (&pmutex->lock, NULL);
+
+  if (result == ENOMEM || result == EAGAIN)
     {
-      dbus_free (retval);
+      dbus_free (pmutex);
       return NULL;
     }
+  else
+    {
+      PTHREAD_CHECK ("pthread_mutex_init", result);
+    }
 
-  return (DBusMutex *) retval;
+  return DBUS_MUTEX (pmutex);
 }
 
 static void
 _dbus_pthread_mutex_free (DBusMutex *mutex)
 {
-  pthread_mutex_destroy ((pthread_mutex_t *) mutex);
-  dbus_free (mutex);
+  DBusMutexPThread *pmutex = DBUS_MUTEX_PTHREAD (mutex);
+  
+  PTHREAD_CHECK ("pthread_mutex_destroy", pthread_mutex_destroy (&pmutex->lock));
+
+  dbus_free (pmutex);
 }
 
 static dbus_bool_t
 _dbus_pthread_mutex_lock (DBusMutex *mutex)
 {
-  return pthread_mutex_lock ((pthread_mutex_t *) mutex) == 0;
+  DBusMutexPThread *pmutex = DBUS_MUTEX_PTHREAD (mutex);
+
+  PTHREAD_CHECK ("pthread_mutex_lock", pthread_mutex_lock (&pmutex->lock));
+
+  return TRUE;
 }
 
 static dbus_bool_t
 _dbus_pthread_mutex_unlock (DBusMutex *mutex)
 {
-  return pthread_mutex_unlock ((pthread_mutex_t *) mutex) == 0;
+  DBusMutexPThread *pmutex = DBUS_MUTEX_PTHREAD (mutex);
+
+  PTHREAD_CHECK ("pthread_mutex_unlock", pthread_mutex_unlock (&pmutex->lock));
+
+  return TRUE;
 }
 
 static DBusCondVar *
 _dbus_pthread_condvar_new (void)
 {
-  pthread_cond_t *retval;
+  DBusCondVarPThread *pcond;
+  int result;
   
-  retval = dbus_new (pthread_cond_t, 1);
-  if (retval == NULL)
+  pcond = dbus_new (DBusCondVarPThread, 1);
+  if (pcond == NULL)
     return NULL;
-  
-  if (pthread_cond_init (retval, NULL))
+
+  result = pthread_cond_init (&pcond->cond, NULL);
+
+  if (result == EAGAIN || result == ENOMEM)
     {
-      dbus_free (retval);
+      dbus_free (pcond);
       return NULL;
     }
-  return (DBusCondVar *) retval;
+  else
+    {
+      PTHREAD_CHECK ("pthread_cond_init", result);
+    }
+  
+  return DBUS_COND_VAR (pcond);
 }
 
 static void
 _dbus_pthread_condvar_free (DBusCondVar *cond)
-{
-  pthread_cond_destroy ((pthread_cond_t *) cond);
-  dbus_free (cond);
+{  
+  DBusCondVarPThread *pcond = DBUS_COND_VAR_PTHREAD (cond);
+  
+  PTHREAD_CHECK ("pthread_cond_destroy", pthread_cond_destroy (&pcond->cond));
+
+  dbus_free (pcond);
 }
 
 static void
 _dbus_pthread_condvar_wait (DBusCondVar *cond,
-                    DBusMutex   *mutex)
+                            DBusMutex   *mutex)
 {
-  pthread_cond_wait ((pthread_cond_t *)cond,
-		     (pthread_mutex_t *) mutex);
+  DBusMutexPThread *pmutex = DBUS_MUTEX_PTHREAD (mutex);
+  DBusCondVarPThread *pcond = DBUS_COND_VAR_PTHREAD (cond);
+  
+  PTHREAD_CHECK ("pthread_cond_wait", pthread_cond_wait (&pcond->cond, &pmutex->lock));
 }
 
 static dbus_bool_t
 _dbus_pthread_condvar_wait_timeout (DBusCondVar               *cond,
-				     DBusMutex                 *mutex,
-				     int                        timeout_milliseconds)
+                                    DBusMutex                 *mutex,
+                                    int                        timeout_milliseconds)
 {
+  DBusMutexPThread *pmutex = DBUS_MUTEX_PTHREAD (mutex);
+  DBusCondVarPThread *pcond = DBUS_COND_VAR_PTHREAD (cond);
   struct timeval time_now;
   struct timespec end_time;
   int result;
@@ -115,10 +172,13 @@ _dbus_pthread_condvar_wait_timeout (DBusCondVar               *cond,
       end_time.tv_sec += 1;
       end_time.tv_nsec -= 1000*1000*1000;
     }
-  
-  result = pthread_cond_timedwait ((pthread_cond_t *) cond,
-				   (pthread_mutex_t *) mutex,
-				   &end_time);
+
+  result = pthread_cond_timedwait (&pcond->cond, &pmutex->lock, &end_time);
+
+  if (result != ETIMEDOUT)
+    {
+      PTHREAD_CHECK ("pthread_cond_timedwait", result);
+    }
   
   /* return true if we did not time out */
   return result != ETIMEDOUT;
@@ -127,13 +187,17 @@ _dbus_pthread_condvar_wait_timeout (DBusCondVar               *cond,
 static void
 _dbus_pthread_condvar_wake_one (DBusCondVar *cond)
 {
-  pthread_cond_signal ((pthread_cond_t *)cond);
+  DBusCondVarPThread *pcond = DBUS_COND_VAR_PTHREAD (cond);
+
+  PTHREAD_CHECK ("pthread_cond_signal", pthread_cond_signal (&pcond->cond));
 }
 
 static void
 _dbus_pthread_condvar_wake_all (DBusCondVar *cond)
 {
-  pthread_cond_broadcast ((pthread_cond_t *)cond);
+  DBusCondVarPThread *pcond = DBUS_COND_VAR_PTHREAD (cond);
+  
+  PTHREAD_CHECK ("pthread_cond_broadcast", pthread_cond_broadcast (&pcond->cond));
 }
 
 static const DBusThreadFunctions pthread_functions =
diff --git a/dbus/dbus-threads.h b/dbus/dbus-threads.h
index 5cb13b1a..1cf533d6 100644
--- a/dbus/dbus-threads.h
+++ b/dbus/dbus-threads.h
@@ -51,24 +51,28 @@ typedef dbus_bool_t (* DBusMutexLockFunction)   (DBusMutex *mutex);
 /** Deprecated, provide DBusRecursiveMutexUnlockFunction instead. Return value is unlock success, but gets ignored in practice. */
 typedef dbus_bool_t (* DBusMutexUnlockFunction) (DBusMutex *mutex);
 
-/** Creates a new recursively-lockable mutex, or returns #NULL if not enough memory.
- * Found in #DBusThreadFunctions. Do not just use PTHREAD_MUTEX_RECURSIVE for this, because
- * it does not save/restore the recursion count when waiting on a condition. libdbus
- * requires the Java-style behavior where the mutex is fully unlocked to wait on
- * a condition.
+/** Creates a new recursively-lockable mutex, or returns #NULL if not
+ * enough memory.  Can only fail due to lack of memory.  Found in
+ * #DBusThreadFunctions. Do not just use PTHREAD_MUTEX_RECURSIVE for
+ * this, because it does not save/restore the recursion count when
+ * waiting on a condition. libdbus requires the Java-style behavior
+ * where the mutex is fully unlocked to wait on a condition.
  */
 typedef DBusMutex*  (* DBusRecursiveMutexNewFunction)    (void);
 /** Frees a recursively-lockable mutex.  Found in #DBusThreadFunctions.
  */
 typedef void        (* DBusRecursiveMutexFreeFunction)   (DBusMutex *mutex);
 /** Locks a recursively-lockable mutex.  Found in #DBusThreadFunctions.
+ * Can only fail due to lack of memory.
  */
 typedef void        (* DBusRecursiveMutexLockFunction)   (DBusMutex *mutex);
 /** Unlocks a recursively-lockable mutex.  Found in #DBusThreadFunctions.
+ * Can only fail due to lack of memory.
  */
 typedef void        (* DBusRecursiveMutexUnlockFunction) (DBusMutex *mutex);
 
 /** Creates a new condition variable.  Found in #DBusThreadFunctions.
+ * Can only fail (returning #NULL) due to lack of memory.
  */
 typedef DBusCondVar*  (* DBusCondVarNewFunction)         (void);
 /** Frees a condition variable.  Found in #DBusThreadFunctions.
@@ -82,6 +86,8 @@ typedef void          (* DBusCondVarFreeFunction)        (DBusCondVar *cond);
  * condition variables (does not save/restore the recursion count) so
  * don't try using simply pthread_cond_wait() and a
  * PTHREAD_MUTEX_RECURSIVE to implement this, it won't work right.
+ *
+ * Has no error conditions. Must succeed if it returns.
  */
 typedef void          (* DBusCondVarWaitFunction)        (DBusCondVar *cond,
 							  DBusMutex   *mutex);
@@ -89,14 +95,21 @@ typedef void          (* DBusCondVarWaitFunction)        (DBusCondVar *cond,
 /** Waits on a condition variable with a timeout.  Found in
  *  #DBusThreadFunctions. Returns #TRUE if the wait did not
  *  time out, and #FALSE if it did.
+ *
+ * Has no error conditions. Must succeed if it returns. 
  */
 typedef dbus_bool_t   (* DBusCondVarWaitTimeoutFunction) (DBusCondVar *cond,
 							  DBusMutex   *mutex,
 							  int          timeout_milliseconds);
 /** Wakes one waiting thread on a condition variable.  Found in #DBusThreadFunctions.
+ *
+ * Has no error conditions. Must succeed if it returns.
  */
 typedef void          (* DBusCondVarWakeOneFunction) (DBusCondVar *cond);
+
 /** Wakes all waiting threads on a condition variable.  Found in #DBusThreadFunctions.
+ *
+ * Has no error conditions. Must succeed if it returns.
  */
 typedef void          (* DBusCondVarWakeAllFunction) (DBusCondVar *cond);
 
diff --git a/doc/dbus-specification.xml b/doc/dbus-specification.xml
index 977734d7..1e4ac4f5 100644
--- a/doc/dbus-specification.xml
+++ b/doc/dbus-specification.xml
@@ -2311,11 +2311,24 @@
         UUID representing the identity of the machine the process is running on.
         This UUID must be the same for all processes on a single system at least
         until that system next reboots. It should be the same across reboots 
-        if possible, but may change due to reconfiguration or hardware changes.
+        if possible, but this is not always possible to implement and is not 
+        guaranteed.
         It does not matter which object path a GetMachineId is sent to.  The
         reference implementation handles this method automatically.
       </para>
       <para>
+        The UUID is intended to be per-instance-of-the-operating-system, so may represent
+        a virtual machine running on a hypervisor, rather than a physical machine.
+        Basically if two processes see the same UUID, they should also see the same
+        shared memory, UNIX domain sockets, process IDs, and other features that require 
+        a running OS kernel in common between the processes.
+      </para>
+      <para>
+        The UUID is often used where other programs might use a hostname. Hostnames 
+        can change without rebooting, however, or just be "localhost" - so the UUID
+        is more robust.
+      </para>
+      <para>
         The UUID must contain 128 bits of data and be hex-encoded (meaning, the hex 
         string contains 32 ASCII characters). The hex-encoded string may not contain 
         hyphens or other non-hex-digit characters, and it must be exactly 32 characters long.
@@ -2323,13 +2336,6 @@
         since the UNIX epoch in the last 32 bits of the UUID, and to put randomly-generated bits
         in the first 96 bits of the UUID.
       </para>
-      <para>
-        The UUID is intended to be per-instance-of-the-operating-system, so may represent
-        a virtual machine running on a hypervisor, rather than a physical machine.
-        Basically if two processes see the same UUID, they should also see the same
-        shared memory, UNIX domain sockets, process IDs, and other features that require 
-        a running OS kernel in common between the processes.
-      </para>
     </sect2>
 
     <sect2 id="standard-interfaces-introspectable">