Modified function sendDBusMessageWithReplyAsync with patched libdbus api call dbus_connection_send_with_reply_set_notify in order to avoid threading problems.2.1.6-p6maintain/2.1.6

3 files changed, 216 insertions, 27 deletions

diff --git a/NEWS b/NEWS
index 3e45246..e6d4b67 100644
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,11 @@
+This is CommonAPI-D-Bus 2.1.6-p6
+
+This version contains a patch for a race condition in CommonAPI-D-Bus. The cause for this bug seems to be the order of the calls dbus_connection_send_with_reply and dbus_pending_call_set_notify. It should be possible to call the notification for the reply first and then the function. The standard libdbus API however does not provide this functionality. Therefore a patch for libdbus is provided (capi-dbus-add-send-with-reply-set-notify.patch). The patch can be applied by calling:
+
+git am < capi-dbus-add-send-with-reply-set-notify.patch
+
+in the directory with the DBus source code. The patch adds the new function dbus_connection_send_with_reply_set_notify to the libdbus API.
+
 
 This is CommonAPI-D-Bus 2.1.6
 
diff --git a/capi-dbus-add-send-with-reply-set-notify.patch b/capi-dbus-add-send-with-reply-set-notify.patch
new file mode 100644
index 0000000..3f92169
--- /dev/null
+++ b/capi-dbus-add-send-with-reply-set-notify.patch
@@ -0,0 +1,201 @@
+From 1dcacc908826f59b580bd1d1a7541fb919d4bd97 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?J=C3=BCrgen=20Gehring?= <juergen.gehring@bmw.de>
+Date: Thu, 23 Apr 2015 02:15:06 -0700
+Subject: [PATCH] Add dbus_connection_send_with_reply_set_notify function for
+ proper thread handling
+
+---
+ dbus/dbus-connection.c | 155 +++++++++++++++++++++++++++++++++++++++++++++++++
+ dbus/dbus-connection.h |  10 ++++
+ 2 files changed, 165 insertions(+)
+
+diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
+index e1068e3..233f179 100644
+--- a/dbus/dbus-connection.c
++++ b/dbus/dbus-connection.c
+@@ -3482,6 +3482,161 @@ dbus_connection_send_with_reply (DBusConnection     *connection,
+ }
+ 
+ /**
++ * Queues a message to send, as with dbus_connection_send(),
++ * but also returns a #DBusPendingCall used to receive a reply to the
++ * message. If no reply is received in the given timeout_milliseconds,
++ * this function expires the pending reply and generates a synthetic
++ * error reply (generated in-process, not by the remote application)
++ * indicating that a timeout occurred.
++ *
++ * A #DBusPendingCall will see a reply message before any filters or
++ * registered object path handlers. See dbus_connection_dispatch() for
++ * details on when handlers are run.
++ *
++ * A #DBusPendingCall will always see exactly one reply message,
++ * unless it's cancelled with dbus_pending_call_cancel().
++ *
++ * If #NULL is passed for the pending_return, the #DBusPendingCall
++ * will still be generated internally, and used to track
++ * the message reply timeout. This means a timeout error will
++ * occur if no reply arrives, unlike with dbus_connection_send().
++ *
++ * If -1 is passed for the timeout, a sane default timeout is used. -1
++ * is typically the best value for the timeout for this reason, unless
++ * you want a very short or very long timeout.  If #DBUS_TIMEOUT_INFINITE is
++ * passed for the timeout, no timeout will be set and the call will block
++ * forever.
++ *
++ * @warning if the connection is disconnected or you try to send Unix
++ * file descriptors on a connection that does not support them, the
++ * #DBusPendingCall will be set to #NULL, so be careful with this.
++ *
++ * @param connection the connection
++ * @param message the message to send
++ * @param pending_return return location for a #DBusPendingCall
++ * object, or #NULL if connection is disconnected or when you try to
++ * send Unix file descriptors on a connection that does not support
++ * them.
++ * @param timeout_milliseconds timeout in milliseconds, -1 (or
++ *  #DBUS_TIMEOUT_USE_DEFAULT) for default or #DBUS_TIMEOUT_INFINITE for no
++ *  timeout
++ * @returns #FALSE if no memory, #TRUE otherwise.
++ *
++ */
++dbus_bool_t
++dbus_connection_send_with_reply_set_notify (DBusConnection     *connection,
++                                 DBusMessage        *message,
++                                 DBusPendingCall   **pending_return,
++                                 DBusPendingCallNotifyFunction function0,
++                                 void               *user_data0,
++                                 DBusFreeFunction    free_user_data0,
++                                 int                 timeout_milliseconds)
++{
++  DBusPendingCall *pending;
++  dbus_int32_t serial = -1;
++  DBusDispatchStatus status;
++
++  _dbus_return_val_if_fail (connection != NULL, FALSE);
++  _dbus_return_val_if_fail (message != NULL, FALSE);
++  _dbus_return_val_if_fail (timeout_milliseconds >= 0 || timeout_milliseconds == -1, FALSE);
++
++  if (pending_return)
++    *pending_return = NULL;
++
++  CONNECTION_LOCK (connection);
++
++#ifdef HAVE_UNIX_FD_PASSING
++
++  if (!_dbus_transport_can_pass_unix_fd(connection->transport) &&
++      message->n_unix_fds > 0)
++    {
++      /* Refuse to send fds on a connection that cannot handle
++         them. Unfortunately we cannot return a proper error here, so
++         the best we can do is return TRUE but leave *pending_return
++         as NULL. */
++      CONNECTION_UNLOCK (connection);
++      return TRUE;
++    }
++
++#endif
++
++   if (!_dbus_connection_get_is_connected_unlocked (connection))
++    {
++      CONNECTION_UNLOCK (connection);
++
++      return TRUE;
++    }
++
++  pending = _dbus_pending_call_new_unlocked (connection,
++                                             timeout_milliseconds,
++                                             reply_handler_timeout);
++
++  if (pending == NULL)
++    {
++      CONNECTION_UNLOCK (connection);
++      return FALSE;
++    }
++
++  if (!dbus_pending_call_set_notify(pending, function0, user_data0, free_user_data0))
++    {
++      CONNECTION_UNLOCK (connection);
++      return FALSE;
++    }
++
++  /* Assign a serial to the message */
++  serial = dbus_message_get_serial (message);
++  if (serial == 0)
++    {
++      serial = _dbus_connection_get_next_client_serial (connection);
++      dbus_message_set_serial (message, serial);
++    }
++
++  if (!_dbus_pending_call_set_timeout_error_unlocked (pending, message, serial))
++    goto error;
++
++  /* Insert the serial in the pending replies hash;
++   * hash takes a refcount on DBusPendingCall.
++   * Also, add the timeout.
++   */
++  if (!_dbus_connection_attach_pending_call_unlocked (connection,
++						      pending))
++    goto error;
++
++  if (!_dbus_connection_send_unlocked_no_update (connection, message, NULL))
++    {
++      _dbus_connection_detach_pending_call_and_unlock (connection,
++						       pending);
++      goto error_unlocked;
++    }
++
++  if (pending_return)
++    *pending_return = pending; /* hand off refcount */
++  else
++    {
++      _dbus_connection_detach_pending_call_unlocked (connection, pending);
++      /* we still have a ref to the pending call in this case, we unref
++       * after unlocking, below
++       */
++    }
++
++  status = _dbus_connection_get_dispatch_status_unlocked (connection);
++
++  /* this calls out to user code */
++  _dbus_connection_update_dispatch_status_and_unlock (connection, status);
++
++  if (pending_return == NULL)
++    dbus_pending_call_unref (pending);
++
++  return TRUE;
++
++ error:
++  CONNECTION_UNLOCK (connection);
++ error_unlocked:
++  dbus_pending_call_unref (pending);
++  return FALSE;
++}
++
++/**
+  * Sends a message and blocks a certain time period while waiting for
+  * a reply.  This function does not reenter the main loop,
+  * i.e. messages other than the reply are queued up but not
+diff --git a/dbus/dbus-connection.h b/dbus/dbus-connection.h
+index fe4d04e..e8cedf1 100644
+--- a/dbus/dbus-connection.h
++++ b/dbus/dbus-connection.h
+@@ -229,6 +229,16 @@ dbus_bool_t        dbus_connection_send_with_reply              (DBusConnection
+                                                                  DBusMessage                *message,
+                                                                  DBusPendingCall           **pending_return,
+                                                                  int                         timeout_milliseconds);
++
++DBUS_EXPORT
++dbus_bool_t        dbus_connection_send_with_reply_set_notify   (DBusConnection             *connection,
++                                                                 DBusMessage                *message,
++                                                                 DBusPendingCall           **pending_return,
++                                                                 DBusPendingCallNotifyFunction function0,
++                                                                 void                      * user_data0,
++                                                                 DBusFreeFunction            free_user_data0,
++                                                                 int                         timeout_milliseconds);
++
+ DBUS_EXPORT
+ DBusMessage *      dbus_connection_send_with_reply_and_block    (DBusConnection             *connection,
+                                                                  DBusMessage                *message,
+-- 
+1.9.1
+
diff --git a/src/CommonAPI/DBus/DBusConnection.cpp b/src/CommonAPI/DBus/DBusConnection.cpp
index a2fba69..b5d7b06 100644
--- a/src/CommonAPI/DBus/DBusConnection.cpp
+++ b/src/CommonAPI/DBus/DBusConnection.cpp
@@ -556,40 +556,20 @@ std::future<CallStatus> DBusConnection::sendDBusMessageWithReplyAsync(
     dbus_bool_t libdbusSuccess;
 
     suspendDispatching();
-    libdbusSuccess = dbus_connection_send_with_reply(libdbusConnection_,
+    libdbusSuccess = dbus_connection_send_with_reply_set_notify(libdbusConnection_,
                                                          dbusMessage.libdbusMessage_,
                                                          &libdbusPendingCall,
+                                                         onLibdbusPendingCallNotifyThunk,
+                                                         dbusMessageReplyAsyncHandler.get(),
+                                                         onLibdbusDataCleanup,
                                                          timeoutMilliseconds);
 
     if (!libdbusSuccess || !libdbusPendingCall) {
         dbusMessageReplyAsyncHandler->onDBusMessageReply(CallStatus::CONNECTION_FAILED, dbusMessage.createMethodError(DBUS_ERROR_DISCONNECTED));
-        resumeDispatching();
-        return dbusMessageReplyAsyncHandler->getFuture();
-    }
-
-    sendLock_.lock();
-	if (dbus_pending_call_get_completed (libdbusPendingCall)) {
-
-		onLibdbusPendingCallNotifyThunk(libdbusPendingCall, dbusMessageReplyAsyncHandler.get());
-		onLibdbusDataCleanup(dbusMessageReplyAsyncHandler.get());
-
-	} else {
-
-		libdbusSuccess = dbus_pending_call_set_notify(
-						libdbusPendingCall,
-						onLibdbusPendingCallNotifyThunk,
-						dbusMessageReplyAsyncHandler.get(),
-						onLibdbusDataCleanup);
-
-	    if (!libdbusSuccess) {
-	        dbusMessageReplyAsyncHandler->onDBusMessageReply(CallStatus::OUT_OF_MEMORY, dbusMessage);
-	        dbus_pending_call_unref(libdbusPendingCall);
-	        resumeDispatching();
-			sendLock_.unlock();
-	        return dbusMessageReplyAsyncHandler->getFuture();
-	    }
+        dbus_pending_call_unref(libdbusPendingCall);
+	    resumeDispatching();
+	    return dbusMessageReplyAsyncHandler->getFuture();
 	}
-    sendLock_.unlock();
 
     DBusMessageReplyAsyncHandler* replyAsyncHandler = dbusMessageReplyAsyncHandler.release();