2003-04-06 Havoc Pennington <hp@pobox.com>

	* dbus/dbus-threads.c: Redo how the fake debug mutexes are done
	so it detects deadlocks and also we actually init threads when
	debugging.

6 files changed, 170 insertions, 24 deletions

diff --git a/ChangeLog b/ChangeLog
index 874b5bd6..9bb927ff 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,11 @@
 2003-04-06  Havoc Pennington  <hp@pobox.com>
 
+	* dbus/dbus-threads.c: Redo how the fake debug mutexes are done 
+	so it detects deadlocks and also we actually init threads when 
+	debugging.
+
+2003-04-06  Havoc Pennington  <hp@pobox.com>
+
 	* dbus/dbus-server-unix.c (_dbus_server_new_for_domain_socket):
 	save the domain socket name, and unlink it when we disconnect the
 	server. Means that at least when we exit normally, we won't leave
diff --git a/bus/test-main.c b/bus/test-main.c
index 8ef6bfc4..73491487 100644
--- a/bus/test-main.c
+++ b/bus/test-main.c
@@ -68,6 +68,12 @@ main (int argc, char **argv)
     }
 
   _dbus_string_init_const (&test_data_dir, dir);
+
+#if 0
+  /* FIXME this is disabled because of thread bugs that need fixing... */
+  if (!_dbus_threads_init_debug ())
+    die ("initializing debug threads");
+#endif
   
   printf ("%s: Running config file parser test\n", argv[0]);
   if (!bus_config_parser_test (&test_data_dir))
diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index 1a00b702..e28e45b1 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -2053,6 +2053,10 @@ dbus_connection_dispatch (DBusConnection *connection)
  * successful adds. i.e. if #FALSE is returned the net result
  * should be that dbus_connection_set_watch_functions() has no effect,
  * but the add_function and remove_function may have been called.
+ *
+ * @todo We need to drop the lock when we call the
+ * add/remove/toggled functions which can be a side effect
+ * of setting the watch functions.
  * 
  * @param connection the connection.
  * @param add_function function to begin monitoring a new descriptor.
@@ -2075,7 +2079,10 @@ dbus_connection_set_watch_functions (DBusConnection              *connection,
   dbus_mutex_lock (connection->mutex);
   /* ref connection for slightly better reentrancy */
   _dbus_connection_ref_unlocked (connection);
-  
+
+  /* FIXME this can call back into user code, and we need to drop the
+   * connection lock when it does.
+   */
   retval = _dbus_watch_list_set_functions (connection->watches,
                                            add_function, remove_function,
                                            toggled_function,
diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h
index f37009b9..b6222fae 100644
--- a/dbus/dbus-internals.h
+++ b/dbus/dbus-internals.h
@@ -214,6 +214,8 @@ _DBUS_DECLARE_GLOBAL_LOCK (bus);
 _DBUS_DECLARE_GLOBAL_LOCK (shutdown_funcs);
 #define _DBUS_N_GLOBAL_LOCKS (8)
 
+dbus_bool_t _dbus_threads_init_debug (void);
+
 DBUS_END_DECLS;
 
 #endif /* DBUS_INTERNALS_H */
diff --git a/dbus/dbus-test.c b/dbus/dbus-test.c
index 76776a95..19d6d7cd 100644
--- a/dbus/dbus-test.c
+++ b/dbus/dbus-test.c
@@ -65,6 +65,9 @@ void
 dbus_internal_do_not_use_run_tests (const char *test_data_dir)
 {
 #ifdef DBUS_BUILD_TESTS
+  if (!_dbus_threads_init_debug ())
+    die ("debug threads init");
+  
   if (test_data_dir == NULL)
     test_data_dir = _dbus_getenv ("DBUS_TEST_DATA");
 
diff --git a/dbus/dbus-threads.c b/dbus/dbus-threads.c
index 15ce33ca..e8b9f3cd 100644
--- a/dbus/dbus-threads.c
+++ b/dbus/dbus-threads.c
@@ -1,7 +1,7 @@
 /* -*- mode: C; c-file-style: "gnu" -*- */
 /* dbus-threads.h  D-BUS threads handling
  *
- * Copyright (C) 2002  Red Hat Inc.
+ * Copyright (C) 2002, 2003 Red Hat Inc.
  *
  * Licensed under the Academic Free License version 1.2
  * 
@@ -35,18 +35,10 @@ static DBusThreadFunctions thread_functions =
 static int thread_init_generation = 0;
 
 /** This is used for the no-op default mutex pointer, just to be distinct from #NULL */
-#ifdef DBUS_BUILD_TESTS
-#define _DBUS_DUMMY_MUTEX_NEW ((DBusMutex*)_dbus_strdup ("FakeMutex"))
-#else
-#define _DBUS_DUMMY_MUTEX_NEW ((DBusMutex*)0xABCDEF)
-#endif
+#define _DBUS_DUMMY_MUTEX ((DBusMutex*)0xABCDEF)
 
 /** This is used for the no-op default mutex pointer, just to be distinct from #NULL */
-#ifdef DBUS_BUILD_TESTS
-#define _DBUS_DUMMY_CONDVAR_NEW ((DBusCondVar*)_dbus_strdup ("FakeCondvar"))
-#else
-#define _DBUS_DUMMY_CONDVAR_NEW ((DBusCondVar*)0xABCDEF2)
-#endif
+#define _DBUS_DUMMY_CONDVAR ((DBusCondVar*)0xABCDEF2)
 
 /**
  * @defgroup DBusThreads Thread functions
@@ -72,7 +64,7 @@ dbus_mutex_new (void)
   if (thread_functions.mutex_new)
     return (* thread_functions.mutex_new) ();
   else
-    return _DBUS_DUMMY_MUTEX_NEW;
+    return _DBUS_DUMMY_MUTEX;
 }
 
 /**
@@ -84,11 +76,6 @@ dbus_mutex_free (DBusMutex *mutex)
 {
   if (mutex && thread_functions.mutex_free)
     (* thread_functions.mutex_free) (mutex);
-#ifdef DBUS_BUILD_TESTS
-  /* Free the fake mutex */
-  else
-    dbus_free (mutex);
-#endif
 }
 
 /**
@@ -134,7 +121,7 @@ dbus_condvar_new (void)
   if (thread_functions.condvar_new)
     return (* thread_functions.condvar_new) ();
   else
-    return _DBUS_DUMMY_CONDVAR_NEW;
+    return _DBUS_DUMMY_CONDVAR;
 }
 
 /**
@@ -146,11 +133,6 @@ dbus_condvar_free (DBusCondVar *cond)
 {
   if (cond && thread_functions.condvar_free)
     (* thread_functions.condvar_free) (cond);
-#ifdef DBUS_BUILD_TESTS
-  else
-    /* Free the fake condvar */
-    dbus_free (cond);
-#endif
 }
 
 /**
@@ -368,4 +350,144 @@ dbus_threads_init (const DBusThreadFunctions *functions)
   return TRUE;
 }
 
+
+#ifdef DBUS_BUILD_TESTS
+/** Fake mutex used for debugging */
+typedef struct DBusFakeMutex DBusFakeMutex;
+/** Fake mutex used for debugging */
+struct DBusFakeMutex
+{
+  dbus_bool_t locked; /**< Mutex is "locked" */
+};	
+
+static DBusMutex *  dbus_fake_mutex_new            (void);
+static void         dbus_fake_mutex_free           (DBusMutex   *mutex);
+static dbus_bool_t  dbus_fake_mutex_lock           (DBusMutex   *mutex);
+static dbus_bool_t  dbus_fake_mutex_unlock         (DBusMutex   *mutex);
+static DBusCondVar* dbus_fake_condvar_new          (void);
+static void         dbus_fake_condvar_free         (DBusCondVar *cond);
+static void         dbus_fake_condvar_wait         (DBusCondVar *cond,
+                                                    DBusMutex   *mutex);
+static dbus_bool_t  dbus_fake_condvar_wait_timeout (DBusCondVar *cond,
+                                                    DBusMutex   *mutex,
+                                                    int          timeout_msec);
+static void         dbus_fake_condvar_wake_one     (DBusCondVar *cond);
+static void         dbus_fake_condvar_wake_all     (DBusCondVar *cond);
+
+
+static const DBusThreadFunctions fake_functions =
+{
+  DBUS_THREAD_FUNCTIONS_MUTEX_NEW_MASK |
+  DBUS_THREAD_FUNCTIONS_MUTEX_FREE_MASK |
+  DBUS_THREAD_FUNCTIONS_MUTEX_LOCK_MASK |
+  DBUS_THREAD_FUNCTIONS_MUTEX_UNLOCK_MASK |
+  DBUS_THREAD_FUNCTIONS_CONDVAR_NEW_MASK |
+  DBUS_THREAD_FUNCTIONS_CONDVAR_FREE_MASK |
+  DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_MASK |
+  DBUS_THREAD_FUNCTIONS_CONDVAR_WAIT_TIMEOUT_MASK |
+  DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ONE_MASK|
+  DBUS_THREAD_FUNCTIONS_CONDVAR_WAKE_ALL_MASK,
+  dbus_fake_mutex_new,
+  dbus_fake_mutex_free,
+  dbus_fake_mutex_lock,
+  dbus_fake_mutex_unlock,
+  dbus_fake_condvar_new,
+  dbus_fake_condvar_free,
+  dbus_fake_condvar_wait,
+  dbus_fake_condvar_wait_timeout,
+  dbus_fake_condvar_wake_one,
+  dbus_fake_condvar_wake_all
+};
+
+static DBusMutex *
+dbus_fake_mutex_new (void)
+{
+  DBusFakeMutex *mutex;
+
+  mutex = dbus_new0 (DBusFakeMutex, 1);
+
+  return (DBusMutex *)mutex;
+}
+
+static void
+dbus_fake_mutex_free (DBusMutex *mutex)
+{
+  DBusFakeMutex *fake = (DBusFakeMutex*) mutex;
+
+  _dbus_assert (!fake->locked);
+  
+  dbus_free (fake);
+}
+
+static dbus_bool_t
+dbus_fake_mutex_lock (DBusMutex *mutex)
+{
+  DBusFakeMutex *fake = (DBusFakeMutex*) mutex;
+
+  _dbus_assert (!fake->locked);
+
+  fake->locked = TRUE;
+  
+  return TRUE;
+}
+
+static dbus_bool_t
+dbus_fake_mutex_unlock (DBusMutex *mutex)
+{
+  DBusFakeMutex *fake = (DBusFakeMutex*) mutex;
+
+  _dbus_assert (fake->locked);
+
+  fake->locked = FALSE;
+  
+  return TRUE;
+}
+
+static DBusCondVar*
+dbus_fake_condvar_new (void)
+{
+  return (DBusCondVar*) _dbus_strdup ("FakeCondvar");
+}
+
+static void
+dbus_fake_condvar_free (DBusCondVar *cond)
+{
+  dbus_free (cond);
+}
+
+static void
+dbus_fake_condvar_wait (DBusCondVar *cond,
+		    DBusMutex   *mutex)
+{
+  
+}
+
+static dbus_bool_t
+dbus_fake_condvar_wait_timeout (DBusCondVar *cond,
+                                DBusMutex   *mutex,
+                                int         timeout_msec)
+{
+  return TRUE;
+}
+
+static void
+dbus_fake_condvar_wake_one (DBusCondVar *cond)
+{
+
+}
+
+static void
+dbus_fake_condvar_wake_all (DBusCondVar *cond)
+{
+
+}
+
+dbus_bool_t
+_dbus_threads_init_debug (void)
+{
+  return dbus_threads_init (&fake_functions);
+}
+
+#endif /* DBUS_BUILD_TESTS */
+
 /** @} */