2006-03-02 John (J5) Palmieri <johnp@redhat.com>

* dbus/dbus-connection.c:
	(_dbus_connection_block_pending_call):
	Check to see if our data has already been read off the connection
	by another blocking pending call before we block in poll.
	(check_for_reply_and_update_dispatch_unlocked):
	Code taken from _dbus_connection_block_pending_call - checks for
	an already read reply and updates the dispatch if there is one.

	* test/name-test/test-pending-call-dispatch.c:
	New test for making sure we don't get stuck polling a
	dbus connection which has no data on the socket when
	blocking out of order on two or more pending calls.

ChangeLog

@@ -1,3 +1,18 @@
+2006-03-02  John (J5) Palmieri  <johnp@redhat.com>
+
+	* dbus/dbus-connection.c: 
+	(_dbus_connection_block_pending_call):
+	Check to see if our data has already been read off the connection
+	by another blocking pending call before we block in poll.
+	(check_for_reply_and_update_dispatch_unlocked):
+	Code taken from _dbus_connection_block_pending_call - checks for
+	an already read reply and updates the dispatch if there is one.
+
+	* test/name-test/test-pending-call-dispatch.c:
+	New test for making sure we don't get stuck polling a 
+	dbus connection which has no data on the socket when
+	blocking out of order on two or more pending calls.
+
 2006-02-28  Thiago Macieira <thiago.macieira@trolltech.com>
 
 	* qt/Makefile.am: Patch by Sjoerd Simons. More .moc issues:

dbus/dbus-connection.c

@@ -2533,6 +2533,36 @@ check_for_reply_unlocked (DBusConnection *connection,
   return NULL;
 }
 
+static dbus_bool_t
+check_for_reply_and_update_dispatch_unlocked (DBusPendingCall *pending)
+{
+  DBusMessage *reply;
+  DBusDispatchStatus status;
+  DBusConnection *connection;
+
+  connection = pending->connection;
+
+  reply = check_for_reply_unlocked (connection, pending->reply_serial);
+  if (reply != NULL)
+    {
+      _dbus_verbose ("%s checked for reply\n", _DBUS_FUNCTION_NAME);
+
+      _dbus_verbose ("dbus_connection_send_with_reply_and_block(): got reply\n");
+
+      _dbus_pending_call_complete_and_unlock (pending, reply);
+      dbus_message_unref (reply);
+
+      CONNECTION_LOCK (connection);
+      status = _dbus_connection_get_dispatch_status_unlocked (connection);
+      _dbus_connection_update_dispatch_status_and_unlock (connection, status);
+      dbus_pending_call_unref (pending);
+
+      return TRUE;
+    }
+
+  return FALSE;
+}
+
 /**
  * When a function that blocks has been called with a timeout, and we
  * run out of memory, the time to wait for memory is based on the
@@ -2616,6 +2646,11 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
                  start_tv_sec, start_tv_usec,
                  end_tv_sec, end_tv_usec);
 
+  /* check to see if we already got the data off the socket */
+  /* from another blocked pending call */
+  if (check_for_reply_and_update_dispatch_unlocked (pending))
+    return;
+
   /* Now we wait... */
   /* always block at least once as we know we don't have the reply yet */
   _dbus_connection_do_iteration_unlocked (connection,
@@ -2645,27 +2680,8 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
     }
   
   if (status == DBUS_DISPATCH_DATA_REMAINS)
-    {
-      DBusMessage *reply;
-      
-      reply = check_for_reply_unlocked (connection, client_serial);
-      if (reply != NULL)
-        {
-          _dbus_verbose ("%s checked for reply\n", _DBUS_FUNCTION_NAME);
-
-          _dbus_verbose ("dbus_connection_send_with_reply_and_block(): got reply\n");
-          
-          _dbus_pending_call_complete_and_unlock (pending, reply);
-          dbus_message_unref (reply);
-
-          CONNECTION_LOCK (connection);
-          status = _dbus_connection_get_dispatch_status_unlocked (connection);
-          _dbus_connection_update_dispatch_status_and_unlock (connection, status);
-          dbus_pending_call_unref (pending);
-          
-          return;
-        }
-    }
+    if (check_for_reply_and_update_dispatch_unlocked (pending))
+      return;  
   
   _dbus_get_current_time (&tv_sec, &tv_usec);
   

test/name-test/Makefile.am

@@ -16,11 +16,18 @@ if DBUS_BUILD_TESTS
 
 ## we use noinst_PROGRAMS not check_PROGRAMS for TESTS so that we
 ## build even when not doing "make check"
-noinst_PROGRAMS=test-names 
+noinst_PROGRAMS=test-names test-pending-call-dispatch 
 
 test_names_SOURCES=				\
 	test-names.c                             
 
 test_names_LDADD=$(top_builddir)/dbus/libdbus-1.la $(top_builddir)/dbus/libdbus-convenience.la
+
+test_pending_call_dispatch_SOURCES =		\
+	test-pending-call-dispatch.c
+
+test_pending_call_dispatch_LDADD=$(top_builddir)/dbus/libdbus-1.la $(top_builddir)/dbus/libdbus-convenience.la
+
+
 endif
 

test/name-test/run-test.sh

@@ -28,3 +28,6 @@ if test -z "$DBUS_TEST_NAME_IN_RUN_TEST"; then
 fi 
 echo "running test-names"
 libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/name-test/test-names || die "test-client failed"
+
+echo "running test-pending-call-dispatch"
+libtool --mode=execute $DEBUG $DBUS_TOP_BUILDDIR/test/name-test/test-pending-call-dispatch || die "test-client failed"

test/name-test/test-pending-call-dispatch.c

@@ -0,0 +1,123 @@
+/**
+* Test to make sure we don't get stuck polling a dbus connection
+* which has no data on the socket.  This was an issue where
+* one pending call would read all the data off the bus
+* and the second pending call would not check to see
+* if its data had already been read before polling the connection
+* and blocking.
+**/
+
+#include <dbus/dbus.h>
+#include <dbus/dbus-sysdeps.h>
+#include <stdio.h>
+#include <stdlib.h>
+
+static void
+_run_iteration (DBusConnection *conn)
+{
+  DBusPendingCall *echo_pending;
+  DBusPendingCall *dbus_pending;
+  DBusMessage *method;
+  DBusMessage *reply;
+  char *echo = "echo";
+
+  /* send the first message */
+  method = dbus_message_new_method_call ("org.freedesktop.DBus.TestSuiteEchoService",
+                                         "/org/freedesktop/TestSuite",
+                                         "org.freedesktop.TestSuite",
+                                         "Echo");
+
+  dbus_message_append_args (method, DBUS_TYPE_STRING, &echo, NULL);
+  dbus_connection_send_with_reply (conn, method, &echo_pending, -1);
+  dbus_message_unref (method);
+  
+  /* send the second message */
+  method = dbus_message_new_method_call (DBUS_SERVICE_DBUS,
+                                         DBUS_PATH_DBUS,
+                                         "org.freedesktop.Introspectable",
+                                         "Introspect");
+
+  dbus_connection_send_with_reply (conn, method, &dbus_pending, -1);
+  dbus_message_unref (method);
+
+  /* block on the second message (should return immediately) */
+  dbus_pending_call_block (dbus_pending);
+
+  /* block on the first message */
+  /* if it does not return immediately chances 
+     are we hit the block in poll bug */
+  dbus_pending_call_block (echo_pending);
+
+  /* check the reply only to make sure we
+     are not getting errors unrelated
+     to the block in poll bug */
+  reply = dbus_pending_call_steal_reply (echo_pending);
+
+  if (reply == NULL)
+    {
+      printf ("Failed: Reply is NULL ***\n");
+      exit (1);
+    }
+
+  if (dbus_message_get_type (reply) == DBUS_MESSAGE_TYPE_ERROR)
+    {
+      printf ("Failed: Reply is error: %s ***\n", dbus_message_get_error_name (reply));
+      exit (1);
+    } 
+
+  dbus_message_unref (reply);
+  dbus_pending_call_unref (dbus_pending);
+  dbus_pending_call_unref (echo_pending);
+  
+}
+
+int
+main (int argc, char *argv[])
+{
+  long start_tv_sec, start_tv_usec;
+  long end_tv_sec, end_tv_usec;
+  int i;
+  DBusMessage *method;
+  DBusConnection *conn;
+  DBusError error;
+
+  /* Time each iteration and make sure it doesn't take more than 5 seconds
+     to complete.  Outside influences may cause connections to take longer
+     but if it does and we are stuck in a poll call then we know the 
+     stuck in poll bug has come back to haunt us */
+
+  printf ("*** Testing stuck in poll\n");
+
+  dbus_error_init (&error);
+
+  conn = dbus_bus_get (DBUS_BUS_SESSION, &error);
+
+  /* run 100 times to make sure */
+  for (i = 0; i < 100; i++)
+    {
+      long delta;
+      
+      _dbus_get_current_time (&start_tv_sec, &start_tv_usec);
+      _run_iteration (conn);
+      _dbus_get_current_time (&end_tv_sec, &end_tv_usec);
+
+      /* we just care about seconds */
+      delta = end_tv_sec - start_tv_sec;
+      printf ("Iter %i: %is\n", i, delta);
+      if (delta >= 5)
+        {
+	  printf ("Failed: looks like we might have been be stuck in poll ***\n");
+	  exit (1);
+	}
+    }
+ 
+  method = dbus_message_new_method_call ("org.freedesktop.TestSuiteEchoService",
+                                         "/org/freedesktop/TestSuite",
+                                         "org.freedesktop.TestSuite",
+                                         "Exit");
+  dbus_connection_send (conn, method, NULL);
+  dbus_message_unref (method);
+
+  printf ("Success ***\n");
+  exit (0);
+}