Bug 830290: Send blocking DBus messages from within DBus thread r=bent,qdot
authorThomas Zimmermann <tdz@users.sourceforge.net>
Fri, 01 Mar 2013 15:05:50 +0100
changeset 133757 a4a9e95ef22a9e2de07a623fdcf3df22cfaff7f1
parent 133756 f3536ccef114b61f0678a604f1b9edee9d8228be
child 133758 41fbfc8a1d2995388822c6d2ff93f63f3ec8bc9c
push id1702
push userryanvm@gmail.com
push dateMon, 03 Jun 2013 19:58:42 +0000
treeherderfx-team@c8b95e4bf23c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbent, qdot
bugs830290
milestone24.0a1
Bug 830290: Send blocking DBus messages from within DBus thread r=bent,qdot The this commit moves the sending of blocking DBus messages to the DBus thread. This allows us to avoid concurrency problems within the DBus library, which is not explicitly thread-safe. As a side note, I'd like to mention that blocking in distributed systems simply doesn't work. The dbus library is especially broken in this regard as it delays all unrelated messages until the reply for the blocking request has been received. A future commit should implement this functionality with an asyncronous call and make the related thread wait for the reply.
ipc/dbus/DBusUtils.cpp
ipc/dbus/DBusUtils.h
--- a/ipc/dbus/DBusUtils.cpp
+++ b/ipc/dbus/DBusUtils.cpp
@@ -354,17 +354,17 @@ static dbus_bool_t dbus_func_args_async_
                                                void (*user_cb)(DBusMessage*,
                                                                void*),
                                                void *user,
                                                const char *path,
                                                const char *ifc,
                                                const char *func,
                                                int first_arg_type,
                                                va_list args) {
-  DBusMessage *msg = NULL;  
+  DBusMessage *msg = NULL;
   /* Compose the command */
   msg = dbus_message_new_method_call(BLUEZ_DBUS_BASE_IFC, path, ifc, func);
 
   if (msg == NULL) {
     LOG("Could not allocate D-Bus message object!");
     goto done;
   }
 
@@ -397,65 +397,140 @@ dbus_bool_t dbus_func_args_async(DBusCon
                                     timeout_ms,
                                     reply, user,
                                     path, ifc, func,
                                     first_arg_type, lst);
   va_end(lst);
   return ret;
 }
 
+//
+// Sends a message and allows the dispatching thread to wait for the
+// reply. Only run it in DBus thread.
+//
+class DBusConnectionSendAndBlockRunnable : public DBusConnectionSendSyncRunnable
+{
+public:
+  DBusConnectionSendAndBlockRunnable(DBusConnection* aConnection,
+                                     DBusMessage* aMessage,
+                                     int aTimeout,
+                                     DBusError* aError)
+  : DBusConnectionSendSyncRunnable(aConnection, aMessage),
+    mTimeout(aTimeout),
+    mError(aError),
+    mReply(nullptr)
+  { }
+
+  NS_IMETHOD Run()
+  {
+    MOZ_ASSERT(!NS_IsMainThread());
+
+    DBusError error;
+
+    if (!mError) {
+      mError = &error;
+    }
+
+    dbus_error_init(mError);
+
+    mReply = dbus_connection_send_with_reply_and_block(mConnection, mMessage, mTimeout, mError);
+    bool success = mReply || dbus_error_has_name(mError, DBUS_ERROR_NO_REPLY);
+
+    Completed(success);
+
+    NS_ENSURE_TRUE(success, NS_ERROR_FAILURE);
+
+    return NS_OK;
+  }
+
+  DBusMessage* GetReply()
+  {
+    return mReply;
+  }
+
+protected:
+  ~DBusConnectionSendAndBlockRunnable()
+  { }
+
+private:
+  int          mTimeout;
+  DBusError*   mError;
+  DBusMessage* mReply;
+};
+
+dbus_bool_t
+dbus_func_send_and_block(DBusConnection* aConnection,
+                         int aTimeout,
+                         DBusMessage** aReply,
+                         DBusError* aError,
+                         DBusMessage* aMessage)
+{
+  nsRefPtr<DBusConnectionSendAndBlockRunnable> t(
+    new DBusConnectionSendAndBlockRunnable(aConnection, aMessage,
+                                           aTimeout, aError));
+  MOZ_ASSERT(t);
+
+  nsresult rv = DispatchToDBusThread(t);
+
+  if (NS_FAILED(rv)) {
+    if (aMessage) {
+      dbus_message_unref(aMessage);
+    }
+    return FALSE;
+  }
+
+  if (!t->WaitForCompletion()) {
+    return FALSE;
+  }
+
+  if (aReply) {
+    *aReply = t->GetReply();
+  }
+
+  return TRUE;
+}
+
 // If err is NULL, then any errors will be LOG'd, and free'd and the reply
 // will be NULL.
 // If err is not NULL, then it is assumed that dbus_error_init was already
 // called, and error's will be returned to the caller without logging. The
 // return value is NULL iff an error was set. The client must free the error if
 // set.
-DBusMessage * dbus_func_args_timeout_valist(DBusConnection *conn,
-                                            int timeout_ms,
-                                            DBusError *err,
-                                            const char *path,
-                                            const char *ifc,
-                                            const char *func,
-                                            int first_arg_type,
-                                            va_list args) {
-  
-  DBusMessage *msg = NULL, *reply = NULL;
-  bool return_error = (err != NULL);
+DBusMessage* dbus_func_args_timeout_valist(DBusConnection* conn,
+                                           int timeout_ms,
+                                           DBusError* err,
+                                           const char* path,
+                                           const char* ifc,
+                                           const char* func,
+                                           int first_arg_type,
+                                           va_list args)
+{
+  /* Compose the command */
+  DBusMessage* msg = dbus_message_new_method_call(BLUEZ_DBUS_BASE_IFC, path, ifc, func);
 
-  if (!return_error) {
-    err = (DBusError*)malloc(sizeof(DBusError));
-    dbus_error_init(err);
-  }
-
-  /* Compose the command */
-  msg = dbus_message_new_method_call(BLUEZ_DBUS_BASE_IFC, path, ifc, func);
-
-  if (msg == NULL) {
+  if (!msg) {
     LOG("Could not allocate D-Bus message object!");
     goto done;
   }
 
   /* append arguments */
   if (!dbus_message_append_args_valist(msg, first_arg_type, args)) {
     LOG("Could not append argument to method call!");
     goto done;
   }
 
-  /* Make the call. */
-  reply = dbus_connection_send_with_reply_and_block(conn, msg, timeout_ms, err);
-  if (!return_error && dbus_error_is_set(err)) {
-    LOG_AND_FREE_DBUS_ERROR_WITH_MSG(err, msg);
-  }
+  DBusMessage* reply;
+
+  return dbus_func_send_and_block(conn, timeout_ms, &reply, err, msg) ? reply : nullptr;
 
 done:
-  if (!return_error) {
-    free(err);
+  if (msg) {
+    dbus_message_unref(msg);
   }
-  if (msg) dbus_message_unref(msg);
-  return reply;
+  return nullptr;
 }
 
 DBusMessage * dbus_func_args_timeout(DBusConnection *conn,
                                      int timeout_ms,
                                      DBusError* err,
                                      const char *path,
                                      const char *ifc,
                                      const char *func,
@@ -499,17 +574,17 @@ DBusMessage * dbus_func_args_error(DBusC
   va_start(lst, first_arg_type);
   ret = dbus_func_args_timeout_valist(conn, -1, err,
                                       path, ifc, func,
                                       first_arg_type, lst);
   va_end(lst);
   return ret;
 }
 
-int dbus_returns_int32(DBusMessage *reply) 
+int dbus_returns_int32(DBusMessage *reply)
 {
   DBusError err;
   int32_t ret = -1;
 
   dbus_error_init(&err);
   if (!dbus_message_get_args(reply, &err,
                              DBUS_TYPE_INT32, &ret,
                              DBUS_TYPE_INVALID)) {
--- a/ipc/dbus/DBusUtils.h
+++ b/ipc/dbus/DBusUtils.h
@@ -79,16 +79,22 @@ dbus_bool_t dbus_func_args_async(DBusCon
                                  DBusCallback reply,
                                  void* user,
                                  const char* path,
                                  const char* ifc,
                                  const char* func,
                                  int first_arg_type,
                                  ...);
 
+dbus_bool_t dbus_func_send_and_block(DBusConnection* aConnection,
+                                     int aTimeout,
+                                     DBusMessage** aReply,
+                                     DBusError* aError,
+                                     DBusMessage* aMessage);
+
 DBusMessage*  dbus_func_args(DBusConnection* conn,
                              const char* path,
                              const char* ifc,
                              const char* func,
                              int first_arg_type,
                              ...);
 
 DBusMessage*  dbus_func_args_error(DBusConnection* conn,