shared connections fixes

Havoc Pennington hp at redhat.com
Sun Oct 1 02:07:09 PDT 2006


Hi,

I went in to clean up some docs and stuff on the shared connections 
code, and ended up finding some major issues that took hours to debug. 
Anyway, this patch fixes those. Probably a substantial risk of breaking 
something though. Review would be a good idea...

Havoc
-------------- next part --------------
Index: ChangeLog
===================================================================
RCS file: /cvs/dbus/dbus/ChangeLog,v
retrieving revision 1.1132
diff -u -p -r1.1132 ChangeLog
--- ChangeLog	1 Oct 2006 03:34:21 -0000	1.1132
+++ ChangeLog	1 Oct 2006 09:03:45 -0000
@@ -1,3 +1,68 @@
+2006-10-01  Havoc Pennington  <hp at redhat.com>
+
+	* dbus/dbus-connection.c (_dbus_connection_close_if_only_one_ref): 
+	Add a hack to make DBusNewConnectionFunction work right.
+
+	* dbus/dbus-server-socket.c (handle_new_client_fd_and_unlock): use
+	the hack here. Also, fix the todo about refcount leak.
+	
+	* dbus/dbus-server-debug-pipe.c (_dbus_transport_debug_pipe_new):
+	and use the hack here
+	
+        * dbus/dbus-connection.c: Kill the "shared" flag vs. the
+	"shareable" flag; this was completely broken, since it meant 
+	dbus_connection_open() returned a connection of unknown
+	shared-ness. Now, we always hold a ref on anything opened 
+	as shareable.
+
+	Move the call to notify dbus-bus.c into
+	connection_forget_shared_unlocked, so libdbus consistently forgets
+	all its knowledge of a connection at once. This exposed numerous
+	places where things were totally broken if we dropped a ref inside
+	get_dispatch_status_unlocked where
+	connection_forget_shared_unlocked was previously, so move
+	connection_forget_shared_unlocked into
+	_dbus_connection_update_dispatch_status_and_unlock. Also move the
+	exit_on_disconnect here.
+
+	(shared_connections_shutdown): this assumed weak refs to the
+	shared connections; since we have strong refs now, the assertion 
+	was failing and stuff was left in the hash. Fix it to close
+	still-open shared connections.
+	
+	* bus/dispatch.c: fixup to use dbus_connection_open_private on the 
+	debug pipe connections
+	
+	* dbus/dbus-connection.c (dbus_connection_dispatch): only notify
+	dbus-bus.c if the closed connection is in fact shared
+	(_dbus_connection_close_possibly_shared): rename from 
+	_dbus_connection_close_internal
+	(dbus_connection_close, dbus_connection_open,
+	dbus_connection_open_private): Improve docs to explain the deal
+	with when you should close or unref or both
+
+	* dbus/dbus-bus.c
+	(_dbus_bus_notify_shared_connection_disconnected_unlocked): rename
+	from _dbus_bus_check_connection_and_unref_unlocked and modify to
+	loop over all connections
+
+	* test/test-utils.c (test_connection_shutdown): don't try to close
+	shared connections.
+
+	* test/name-test/test-threads-init.c (main): fix warnings in here
+
+	* dbus/dbus-sysdeps.c (_dbus_abort): support DBUS_BLOCK_ON_ABORT
+	env variable to cause blocking waiting for gdb; drop
+	DBUS_PRINT_BACKTRACE and just call _dbus_print_backtrace() 
+	unconditionally.
+
+	* configure.in: add -export-dynamic to libtool flags if assertions enabled
+	so _dbus_print_backtrace works.
+
+	* dbus/dbus-sysdeps-unix.c (_dbus_print_backtrace): use fprintf
+	instead of _dbus_verbose to print the backtrace, and diagnose lack 
+	of -rdynamic/-export-dynamic
+	
 2006-09-30  Havoc Pennington  <hp at redhat.com>
 
 	* dbus/dbus-bus.c (dbus_bus_get_private, dbus_bus_get) 
Index: configure.in
===================================================================
RCS file: /cvs/dbus/dbus/configure.in,v
retrieving revision 1.181
diff -u -p -r1.181 configure.in
--- configure.in	1 Oct 2006 03:18:47 -0000	1.181
+++ configure.in	1 Oct 2006 09:03:45 -0000
@@ -82,10 +82,24 @@ fi
 if test x$enable_verbose_mode = xyes; then
     AC_DEFINE(DBUS_ENABLE_VERBOSE_MODE,1,[Support a verbose mode])
 fi
+
 if test x$enable_asserts = xno; then
     AC_DEFINE(DBUS_DISABLE_ASSERT,1,[Disable assertion checking])
     AC_DEFINE(G_DISABLE_ASSERT,1,[Disable GLib assertion macros])
+    R_DYNAMIC_LDFLAG=""
+else
+    # -rdynamic is needed for glibc's backtrace_symbols to work.
+    # No clue how much overhead this adds, but it's useful 
+    # to do this on any assertion failure,
+    # so for now it's enabled anytime asserts are (currently not
+    # in production builds).
+
+    # To get -rdynamic you pass -export-dynamic to libtool.
+    AC_DEFINE(DBUS_BUILT_R_DYNAMIC,1,[whether -export-dynamic was passed to libtool])
+    R_DYNAMIC_LDFLAG=-export-dynamic
 fi
+AC_SUBST(R_DYNAMIC_LDFLAG)
+
 if test x$enable_checks = xno; then
     AC_DEFINE(DBUS_DISABLE_CHECKS,1,[Disable public API sanity checking])
     AC_DEFINE(G_DISABLE_CHECKS,1,[Disable GLib public API sanity checking])
Index: bus/Makefile.am
===================================================================
RCS file: /cvs/dbus/dbus/bus/Makefile.am,v
retrieving revision 1.41
diff -u -p -r1.41 Makefile.am
--- bus/Makefile.am	25 Aug 2006 19:18:52 -0000	1.41
+++ bus/Makefile.am	1 Oct 2006 09:03:46 -0000
@@ -75,12 +75,14 @@ dbus_daemon_LDADD=					\
 	$(DBUS_BUS_LIBS)				\
 	$(top_builddir)/dbus/libdbus-convenience.la
 
+dbus_daemon_LDFLAGS=@R_DYNAMIC_LDFLAG@
+
 ## note that TESTS has special meaning (stuff to use in make check)
 ## so if adding tests not to be run in make check, don't add them to 
 ## TESTS
 if DBUS_BUILD_TESTS
-TESTS_ENVIRONMENT=DBUS_TEST_DATA=$(top_builddir)/test/data DBUS_TEST_HOMEDIR=$(top_builddir)/dbus
-TESTS=bus-test 
+TESTS_ENVIRONMENT=DBUS_TEST_DATA=$(top_builddir)/test/data DBUS_TEST_HOMEDIR=$(top_builddir)/dbus DBUS_FATAL_WARNINGS=1 DBUS_BLOCK_ON_ABORT=1
+TESTS=bus-test
 else
 TESTS=
 endif
@@ -94,6 +96,7 @@ bus_test_SOURCES=				\
 	test-main.c
 
 bus_test_LDADD=$(top_builddir)/dbus/libdbus-convenience.la $(DBUS_BUS_LIBS)
+bus_test_LDFLAGS=@R_DYNAMIC_LDFLAG@
 
 ## mop up the gcov files
 clean-local:
@@ -104,9 +107,9 @@ uninstall-hook:
 
 install-data-hook:
 	if test '!' -d $(DESTDIR)$(DBUS_DAEMONDIR); then \
- 		$(mkinstalldirs) $(DESTDIR)$(DBUS_DAEMONDIR); \
- 		chmod 755 $(DESTDIR)$(DBUS_DAEMONDIR); \
- 	fi
+		$(mkinstalldirs) $(DESTDIR)$(DBUS_DAEMONDIR); \
+		chmod 755 $(DESTDIR)$(DBUS_DAEMONDIR); \
+	fi
 	$(INSTALL_PROGRAM) dbus-daemon $(DESTDIR)$(DBUS_DAEMONDIR)
 	$(mkinstalldirs) $(DESTDIR)$(localstatedir)/run/dbus
 	$(mkinstalldirs) $(DESTDIR)$(configdir)/system.d
Index: bus/dispatch.c
===================================================================
RCS file: /cvs/dbus/dbus/bus/dispatch.c,v
retrieving revision 1.76
diff -u -p -r1.76 dispatch.c
--- bus/dispatch.c	1 Oct 2006 03:18:47 -0000	1.76
+++ bus/dispatch.c	1 Oct 2006 09:03:48 -0000
@@ -1479,7 +1479,7 @@ check_hello_connection (BusContext *cont
 
   dbus_error_init (&error);
 
-  connection = dbus_connection_open ("debug-pipe:name=test-server", &error);
+  connection = dbus_connection_open_private ("debug-pipe:name=test-server", &error);
   if (connection == NULL)
     {
       _DBUS_ASSERT_ERROR_IS_SET (&error);
@@ -3998,7 +3998,7 @@ bus_dispatch_test (const DBusString *tes
   if (context == NULL)
     return FALSE;
   
-  foo = dbus_connection_open ("debug-pipe:name=test-server", &error);
+  foo = dbus_connection_open_private ("debug-pipe:name=test-server", &error);
   if (foo == NULL)
     _dbus_assert_not_reached ("could not alloc connection");
 
@@ -4016,7 +4016,7 @@ bus_dispatch_test (const DBusString *tes
   if (!check_add_match_all (context, foo))
     _dbus_assert_not_reached ("AddMatch message failed");
   
-  bar = dbus_connection_open ("debug-pipe:name=test-server", &error);
+  bar = dbus_connection_open_private ("debug-pipe:name=test-server", &error);
   if (bar == NULL)
     _dbus_assert_not_reached ("could not alloc connection");
 
@@ -4031,7 +4031,7 @@ bus_dispatch_test (const DBusString *tes
   if (!check_add_match_all (context, bar))
     _dbus_assert_not_reached ("AddMatch message failed");
   
-  baz = dbus_connection_open ("debug-pipe:name=test-server", &error);
+  baz = dbus_connection_open_private ("debug-pipe:name=test-server", &error);
   if (baz == NULL)
     _dbus_assert_not_reached ("could not alloc connection");
 
@@ -4125,7 +4125,7 @@ bus_dispatch_sha1_test (const DBusString
   if (context == NULL)
     return FALSE;
 
-  foo = dbus_connection_open ("debug-pipe:name=test-server", &error);
+  foo = dbus_connection_open_private ("debug-pipe:name=test-server", &error);
   if (foo == NULL)
     _dbus_assert_not_reached ("could not alloc connection");
 
Index: dbus/Makefile.am
===================================================================
RCS file: /cvs/dbus/dbus/dbus/Makefile.am,v
retrieving revision 1.84
diff -u -p -r1.84 Makefile.am
--- dbus/Makefile.am	1 Oct 2006 03:18:47 -0000	1.84
+++ dbus/Makefile.am	1 Oct 2006 09:03:48 -0000
@@ -164,7 +164,9 @@ noinst_LTLIBRARIES=libdbus-convenience.l
 libdbus_1_la_LIBADD= $(DBUS_CLIENT_LIBS)
 ## don't export symbols that start with "_" (we use this 
 ## convention for internal symbols)
-libdbus_1_la_LDFLAGS= -export-symbols-regex "^[^_].*" -version-info $(LT_CURRENT):$(LT_REVISION):$(LT_AGE) -no-undefined
+libdbus_1_la_LDFLAGS= -export-symbols-regex "^[^_].*" -version-info $(LT_CURRENT):$(LT_REVISION):$(LT_AGE) -no-undefined @R_DYNAMIC_LDFLAG@
+
+libdbus_convenience_la_LDFLAGS=@R_DYNAMIC_LDFLAG@
 
 ## note that TESTS has special meaning (stuff to use in make check)
 ## so if adding tests not to be run in make check, don't add them to 
@@ -184,6 +186,7 @@ dbus_test_SOURCES=				\
 	dbus-test-main.c
 
 dbus_test_LDADD=libdbus-convenience.la
+dbus_test_LDFLAGS=@R_DYNAMIC_LDFLAG@
 
 ## mop up the gcov files
 clean-local:
Index: dbus/dbus-bus.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-bus.c,v
retrieving revision 1.58
diff -u -p -r1.58 dbus-bus.c
--- dbus/dbus-bus.c	1 Oct 2006 03:34:21 -0000	1.58
+++ dbus/dbus-bus.c	1 Oct 2006 09:03:49 -0000
@@ -158,6 +158,7 @@ init_connections_unlocked (void)
            /* Use default system bus address if none set in environment */
            bus_connection_addresses[DBUS_BUS_SYSTEM] =
              _dbus_strdup (DBUS_SYSTEM_BUS_DEFAULT_ADDRESS);
+
            if (bus_connection_addresses[DBUS_BUS_SYSTEM] == NULL)
              return FALSE;
            
@@ -179,7 +180,8 @@ init_connections_unlocked (void)
 	  if (bus_connection_addresses[DBUS_BUS_SESSION] == NULL)
 	    bus_connection_addresses[DBUS_BUS_SESSION] =
 	      _dbus_strdup (DBUS_SESSION_BUS_DEFAULT_ADDRESS);
-           if (bus_connection_addresses[DBUS_BUS_SESSION] == NULL)
+          
+          if (bus_connection_addresses[DBUS_BUS_SESSION] == NULL)
              return FALSE;
 
           _dbus_verbose ("  \"%s\"\n", bus_connection_addresses[DBUS_BUS_SESSION] ?
@@ -310,27 +312,32 @@ ensure_bus_data (DBusConnection *connect
   return bd;
 }
 
-/* internal function that checks to see if this
-   is a shared connection owned by the bus and if it is unref it */
+/**
+ * Internal function that checks to see if this
+ * is a shared connection owned by the bus and if it is unref it.
+ *
+ * @param connection a connection that has been disconnected.
+ */
 void
-_dbus_bus_check_connection_and_unref_unlocked (DBusConnection *connection)
+_dbus_bus_notify_shared_connection_disconnected_unlocked (DBusConnection *connection)
 {
+  int i;
+  
   _DBUS_LOCK (bus);
 
-  if (bus_connections[DBUS_BUS_SYSTEM] == connection)
-    {
-      bus_connections[DBUS_BUS_SYSTEM] = NULL;
-      _dbus_connection_unref_unlocked (connection);
-    }
-  else if (bus_connections[DBUS_BUS_SESSION] == connection)
-    {
-      bus_connections[DBUS_BUS_SESSION] = NULL;
-      _dbus_connection_unref_unlocked (connection);
-    }
-  else if (bus_connections[DBUS_BUS_STARTER] == connection)
+  /* We are expecting to have the connection saved in only one of these
+   * slots, but someone could in a pathological case set system and session
+   * bus to the same bus or something. Or set one of them to the starter
+   * bus without setting the starter bus type in the env variable.
+   * So we don't break the loop as soon as we find a match.
+   */
+  for (i = 0; i < N_BUS_TYPES; ++i)
     {
-      bus_connections[DBUS_BUS_STARTER] = NULL;
-      _dbus_connection_unref_unlocked (connection);
+      if (bus_connections[i] == connection)
+        {
+          bus_connections[i] = NULL;
+          _dbus_connection_unref_unlocked (connection);
+        }
     }
 
   _DBUS_UNLOCK (bus);
@@ -392,7 +399,7 @@ internal_bus_get (DBusBusType  type,
     }
 
   if (private)
-    connection = dbus_connection_open_private(address, error);
+    connection = dbus_connection_open_private (address, error);
   else
     connection = dbus_connection_open (address, error);
   
@@ -412,7 +419,7 @@ internal_bus_get (DBusBusType  type,
   if (!dbus_bus_register (connection, error))
     {
       _DBUS_ASSERT_ERROR_IS_SET (error);
-      _dbus_connection_close_internal (connection);
+      _dbus_connection_close_possibly_shared (connection);
       dbus_connection_unref (connection);
 
       _DBUS_UNLOCK (bus);
@@ -432,6 +439,8 @@ internal_bus_get (DBusBusType  type,
   bd->is_well_known = TRUE;
 
   _DBUS_UNLOCK (bus);
+
+  /* Return a reference to the caller */
   return connection;
 }
 
@@ -446,11 +455,22 @@ internal_bus_get (DBusBusType  type,
 /**
  * Connects to a bus daemon and registers the client with it.  If a
  * connection to the bus already exists, then that connection is
- * returned.  Caller owns a reference to the bus.
+ * returned.  The caller of this function owns a reference to the bus.
  *
+ * The caller may NOT call dbus_connection_close() on this connection;
+ * see dbus_connection_open() and dbus_connection_close() for details
+ * on that.
+ *
+ * If this function obtains a new connection object never before
+ * returned from dbus_bus_get(), it will call
+ * dbus_connection_set_exit_on_disconnect(), so the application
+ * will exit if the connection closes. You can undo this
+ * by calling dbus_connection_set_exit_on_disconnect() yourself
+ * after you get the connection.
+ * 
  * @param type bus type
  * @param error address where an error can be returned.
- * @returns a DBusConnection with new ref
+ * @returns a #DBusConnection with new ref
  */
 DBusConnection *
 dbus_bus_get (DBusBusType  type,
@@ -460,10 +480,20 @@ dbus_bus_get (DBusBusType  type,
 }
 
 /**
- * Connects to a bus daemon and registers the client with it.  Unlike
- * dbus_bus_get(), always creates a new connection. This connection
+ * Connects to a bus daemon and registers the client with it as with dbus_bus_register().
+ * Unlike dbus_bus_get(), always creates a new connection. This connection
  * will not be saved or recycled by libdbus. Caller owns a reference
- * to the bus.
+ * to the bus and must either close it or know it to be closed
+ * prior to releasing this reference.
+ *
+ * See dbus_connection_open_private() for more details on when to
+ * close and unref this connection.
+ *
+ * This function calls
+ * dbus_connection_set_exit_on_disconnect() on the new connection, so the application
+ * will exit if the connection closes. You can undo this
+ * by calling dbus_connection_set_exit_on_disconnect() yourself
+ * after you get the connection.
  *
  * @param type bus type
  * @param error address where an error can be returned.
@@ -481,6 +511,9 @@ dbus_bus_get_private (DBusBusType  type,
  * thing an application does when connecting to the message bus.
  * If registration succeeds, the unique name will be set,
  * and can be obtained using dbus_bus_get_unique_name().
+ *
+ * If you use dbus_bus_get() or dbus_bus_get_private() this
+ * function will be called for you.
  * 
  * @param connection the connection
  * @param error place to store errors
Index: dbus/dbus-bus.h
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-bus.h,v
retrieving revision 1.18
diff -u -p -r1.18 dbus-bus.h
--- dbus/dbus-bus.h	6 Sep 2006 22:00:07 -0000	1.18
+++ dbus/dbus-bus.h	1 Oct 2006 09:03:49 -0000
@@ -68,7 +68,7 @@ void            dbus_bus_remove_match   
                                            const char     *rule,
                                            DBusError      *error);
 
-void           _dbus_bus_check_connection_and_unref_unlocked (DBusConnection *connection);
+void           _dbus_bus_notify_shared_connection_disconnected_unlocked (DBusConnection *connection);
 
 DBUS_END_DECLS
 
Index: dbus/dbus-connection-internal.h
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-connection-internal.h,v
retrieving revision 1.28
diff -u -p -r1.28 dbus-connection-internal.h
--- dbus/dbus-connection-internal.h	7 Sep 2006 19:02:22 -0000	1.28
+++ dbus/dbus-connection-internal.h	1 Oct 2006 09:03:49 -0000
@@ -77,7 +77,8 @@ DBusConnection*   _dbus_connection_new_f
 void              _dbus_connection_do_iteration_unlocked       (DBusConnection     *connection,
                                                                 unsigned int        flags,
                                                                 int                 timeout_milliseconds);
-void              _dbus_connection_close_internal              (DBusConnection *connection);
+void              _dbus_connection_close_possibly_shared       (DBusConnection     *connection);
+void              _dbus_connection_close_if_only_one_ref       (DBusConnection     *connection);
 
 DBusPendingCall*  _dbus_pending_call_new                       (DBusConnection     *connection,
                                                                 int                 timeout_milliseconds,
Index: dbus/dbus-connection.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-connection.c,v
retrieving revision 1.135
diff -u -p -r1.135 dbus-connection.c
--- dbus/dbus-connection.c	1 Oct 2006 03:18:47 -0000	1.135
+++ dbus/dbus-connection.c	1 Oct 2006 09:03:54 -0000
@@ -238,9 +238,7 @@ struct DBusConnection
 
   char *server_guid; /**< GUID of server if we are in shared_connections, #NULL if server GUID is unknown or connection is private */
 
-  unsigned int shareable : 1; /**< #TRUE if connection can go in shared_connections once we know the GUID */
- 
-  unsigned int shared : 1; /** < #TRUE if connection is shared and we hold a ref to it */
+  unsigned int shareable : 1; /**< #TRUE if libdbus owns a reference to the connection and can return it from dbus_connection_open() more than once */
 
   unsigned int dispatch_acquired : 1; /**< Someone has dispatch path (can drain incoming queue) */
   unsigned int io_path_acquired : 1;  /**< Someone has transport io path (can use the transport to read/write messages) */
@@ -248,6 +246,14 @@ struct DBusConnection
   unsigned int exit_on_disconnect : 1; /**< If #TRUE, exit after handling disconnect signal */
 
   unsigned int route_peer_messages : 1; /**< If #TRUE, if org.freedesktop.DBus.Peer messages have a bus name, don't handle them automatically */
+
+  unsigned int disconnected_message_arrived : 1;   /**< We popped or are dispatching the disconnected message.
+                                                    * if the disconnect_message_link is NULL then we queued it, but
+                                                    * this flag is whether it got to the head of the queue.
+                                                    */
+  unsigned int disconnected_message_processed : 1; /**< We did our default handling of the disconnected message,
+                                                    * such as closing the connection.
+                                                    */
   
 #ifndef DBUS_DISABLE_CHECKS
   unsigned int have_connection_lock : 1; /**< Used to check locking */
@@ -265,6 +271,8 @@ static void               _dbus_connecti
 static void               _dbus_connection_acquire_dispatch                  (DBusConnection     *connection);
 static void               _dbus_connection_release_dispatch                  (DBusConnection     *connection);
 static DBusDispatchStatus _dbus_connection_flush_unlocked                    (DBusConnection     *connection);
+static void               _dbus_connection_close_possibly_shared_and_unlock  (DBusConnection     *connection);
+static dbus_bool_t        _dbus_connection_get_is_connected_unlocked         (DBusConnection     *connection);
 
 static DBusMessageFilter *
 _dbus_message_filter_ref (DBusMessageFilter *filter)
@@ -366,11 +374,11 @@ _dbus_connection_queue_received_message 
  */ 
 void 
 _dbus_connection_test_get_locks (DBusConnection *conn,
-                                 DBusMutex **mutex_loc,
-                                 DBusMutex **dispatch_mutex_loc,
-                                 DBusMutex **io_path_mutex_loc,
-                                 DBusCondVar **dispatch_cond_loc,
-                                 DBusCondVar **io_path_cond_loc)
+                                 DBusMutex     **mutex_loc,
+                                 DBusMutex     **dispatch_mutex_loc,
+                                 DBusMutex     **io_path_mutex_loc,
+                                 DBusCondVar   **dispatch_cond_loc,
+                                 DBusCondVar   **io_path_cond_loc)
 {
   *mutex_loc = conn->mutex;
   *dispatch_mutex_loc = conn->dispatch_mutex;
@@ -1178,6 +1186,8 @@ _dbus_connection_new_for_transport (DBus
   connection->exit_on_disconnect = FALSE;
   connection->shareable = FALSE;
   connection->route_peer_messages = FALSE;
+  connection->disconnected_message_arrived = FALSE;
+  connection->disconnected_message_processed = FALSE;
   
 #ifndef DBUS_DISABLE_CHECKS
   connection->generation = _dbus_current_generation;
@@ -1365,10 +1375,42 @@ static DBusHashTable *shared_connections
 static void
 shared_connections_shutdown (void *data)
 {
+  int n_entries;
+  
   _DBUS_LOCK (shared_connections);
+  
+  /* This is a little bit unpleasant... better ideas? */
+  while ((n_entries = _dbus_hash_table_get_n_entries (shared_connections)) > 0)
+    {
+      DBusConnection *connection;
+      DBusMessage *message;
+      DBusHashIter iter;
+      
+      _dbus_hash_iter_init (shared_connections, &iter);
+      _dbus_hash_iter_next (&iter);
+       
+      connection = _dbus_hash_iter_get_value (&iter);
 
-  _dbus_assert (_dbus_hash_table_get_n_entries (shared_connections) == 0);
+      _DBUS_UNLOCK (shared_connections);
+
+      dbus_connection_ref (connection);
+      _dbus_connection_close_possibly_shared (connection);
+
+      /* Churn through to the Disconnected message */
+      while ((message = dbus_connection_pop_message (connection)))
+        {
+          dbus_message_unref (message);
+        }
+      dbus_connection_unref (connection);
+      
+      _DBUS_LOCK (shared_connections);
 
+      /* The connection should now be dead and not in our hash ... */
+      _dbus_assert (_dbus_hash_table_get_n_entries (shared_connections) < n_entries);
+    }
+
+  _dbus_assert (_dbus_hash_table_get_n_entries (shared_connections) == 0);
+  
   _dbus_hash_table_unref (shared_connections);
   shared_connections = NULL;
   
@@ -1419,23 +1461,35 @@ connection_lookup_shared (DBusAddressEnt
       
       if (guid != NULL)
         {
-          *result = _dbus_hash_table_lookup_string (shared_connections,
-                                                    guid);
+          DBusConnection *connection;
+          
+          connection = _dbus_hash_table_lookup_string (shared_connections,
+                                                       guid);
 
-          if (*result)
+          if (connection)
             {
-              /* The DBusConnection can't have been disconnected
-               * between the lookup and this code, because the
-               * disconnection will take the shared_connections lock to
-               * remove the connection. It can't have been finalized
-               * since you have to disconnect prior to finalize.
-               *
-               * Thus it's safe to ref the connection.
+              /* The DBusConnection can't be finalized without taking
+               * the shared_connections lock to remove it from the
+               * hash.  So it's safe to ref the connection here.
+               * However, it may be disconnected if the Disconnected
+               * message hasn't been processed yet, in which case we
+               * want to pretend it isn't in the hash and avoid
+               * returning it.
                */
-              dbus_connection_ref (*result);
-
-              _dbus_verbose ("looked up existing connection to server guid %s\n",
-                             guid);
+              CONNECTION_LOCK (connection);
+              if (_dbus_connection_get_is_connected_unlocked (connection))
+                {
+                  _dbus_connection_ref_unlocked (connection);
+                  *result = connection;
+                  _dbus_verbose ("looked up existing connection to server guid %s\n",
+                                 guid);
+                }
+              else
+                {
+                  _dbus_verbose ("looked up existing connection to server guid %s but it was disconnected so ignoring it\n",
+                                 guid);
+                }
+              CONNECTION_UNLOCK (connection);
             }
         }
       
@@ -1451,14 +1505,24 @@ connection_record_shared_unlocked (DBusC
   char *guid_key;
   char *guid_in_connection;
 
+  HAVE_LOCK_CHECK (connection);
+  _dbus_assert (connection->server_guid == NULL);
+  _dbus_assert (connection->shareable);
+
+  /* get a hard ref on this connection, even if
+   * we won't in fact store it in the hash, we still
+   * need to hold a ref on it until it's disconnected.
+   */
+  _dbus_connection_ref_unlocked (connection);
+
+  if (guid == NULL)
+    return TRUE; /* don't store in the hash */
+  
   /* A separate copy of the key is required in the hash table, because
    * we don't have a lock on the connection when we are doing a hash
    * lookup.
    */
   
-  _dbus_assert (connection->server_guid == NULL);
-  _dbus_assert (connection->shareable);
-  
   guid_key = _dbus_strdup (guid);
   if (guid_key == NULL)
     return FALSE;
@@ -1483,10 +1547,6 @@ connection_record_shared_unlocked (DBusC
     }
 
   connection->server_guid = guid_in_connection;
-  connection->shared = TRUE;
-
-  /* get a hard ref on this connection */
-  dbus_connection_ref (connection);
 
   _dbus_verbose ("stored connection to %s to be shared\n",
                  connection->server_guid);
@@ -1502,25 +1562,30 @@ static void
 connection_forget_shared_unlocked (DBusConnection *connection)
 {
   HAVE_LOCK_CHECK (connection);
- 
-  if (connection->server_guid == NULL)
-    return;
 
-  _dbus_verbose ("dropping connection to %s out of the shared table\n",
-                 connection->server_guid);
+  if (!connection->shareable)
+    return;
   
-  _DBUS_LOCK (shared_connections);
+  if (connection->server_guid != NULL)
+    {
+      _dbus_verbose ("dropping connection to %s out of the shared table\n",
+                     connection->server_guid);
+      
+      _DBUS_LOCK (shared_connections);
+      
+      if (!_dbus_hash_table_remove_string (shared_connections,
+                                           connection->server_guid))
+        _dbus_assert_not_reached ("connection was not in the shared table");
+      
+      dbus_free (connection->server_guid);
+      connection->server_guid = NULL;
+      _DBUS_UNLOCK (shared_connections);
+    }
 
-  if (!_dbus_hash_table_remove_string (shared_connections,
-                                       connection->server_guid))
-    _dbus_assert_not_reached ("connection was not in the shared table");
+  _dbus_bus_notify_shared_connection_disconnected_unlocked (connection);
   
-  dbus_free (connection->server_guid);
-  connection->server_guid = NULL;
-
-  /* remove the hash ref */
+  /* remove our reference held on all shareable connections */
   _dbus_connection_unref_unlocked (connection);
-  _DBUS_UNLOCK (shared_connections);
 }
 
 static DBusConnection*
@@ -1603,36 +1668,42 @@ _dbus_connection_open_internal (const ch
         {
           connection = connection_try_from_address_entry (entries[i],
                                                           &tmp_error);
-          
-          if (connection != NULL && shared)
-            {
-              const char *guid;
 
-              connection->shareable = TRUE;
-              
-              guid = dbus_address_entry_get_value (entries[i], "guid");
-
-              /* we don't have a connection lock but we know nobody
-               * else has a handle to the connection
-               */
-              
-              if (guid &&
-                  !connection_record_shared_unlocked (connection, guid))
+          if (connection != NULL)
+            {
+              CONNECTION_LOCK (connection);
+          
+              if (shared)
                 {
-                  _DBUS_SET_OOM (&tmp_error);
-                  _dbus_connection_close_internal (connection);
-                  dbus_connection_unref (connection);
-                  connection = NULL;
+                  const char *guid;
+                  
+                  connection->shareable = TRUE;
+                  
+                  /* guid may be NULL */
+                  guid = dbus_address_entry_get_value (entries[i], "guid");
+                  
+                  if (!connection_record_shared_unlocked (connection, guid))
+                    {
+                      _DBUS_SET_OOM (&tmp_error);
+                      _dbus_connection_close_possibly_shared_and_unlock (connection);
+                      dbus_connection_unref (connection);
+                      connection = NULL;
+                    }
+                  
+                  /* Note: as of now the connection is possibly shared
+                   * since another thread could have pulled it from the table.
+                   * However, we still have it locked so that thread isn't
+                   * doing anything more than blocking on the lock.
+                   */
                 }
-
-              /* but as of now the connection is possibly shared
-               * since another thread could have pulled it from the table
-               */
             }
         }
       
       if (connection)
-	break;
+        {
+          HAVE_LOCK_CHECK (connection);
+          break;
+        }
 
       _DBUS_ASSERT_ERROR_IS_SET (&tmp_error);
       
@@ -1641,8 +1712,6 @@ _dbus_connection_open_internal (const ch
       else
         dbus_error_free (&tmp_error);
     }
-
-  /* NOTE we don't have a lock on a possibly-shared connection object */
   
   _DBUS_ASSERT_ERROR_IS_CLEAR (error);
   _DBUS_ASSERT_ERROR_IS_CLEAR (&tmp_error);
@@ -1655,6 +1724,8 @@ _dbus_connection_open_internal (const ch
   else
     {
       dbus_error_free (&first_error);
+
+      CONNECTION_UNLOCK (connection);
     }
   
   dbus_address_entries_free (entries);
@@ -1683,6 +1754,10 @@ _dbus_connection_open_internal (const ch
  * reason for the failure in the error parameter. Pass #NULL for the
  * error parameter if you aren't interested in the reason for
  * failure.
+ *
+ * Because this connection is shared, no user of the connection
+ * may call dbus_connection_close(). However, when you are done with the
+ * connection you should call dbus_connection_unref().
  * 
  * @param address the address.
  * @param error address where an error can be returned.
@@ -1713,6 +1788,14 @@ dbus_connection_open (const char     *ad
  * reason for the failure in the error parameter. Pass #NULL for the
  * error parameter if you aren't interested in the reason for
  * failure.
+ *
+ * When you are done with this connection, you must
+ * dbus_connection_close() to disconnect it,
+ * and dbus_connection_unref() to free the connection object.
+ * 
+ * (The dbus_connection_close() can be skipped if the
+ * connection is already known to be disconnected, for example
+ * if you are inside a handler for the Disconnected signal.)
  * 
  * @param address the address.
  * @param error address where an error can be returned.
@@ -1869,12 +1952,19 @@ _dbus_connection_last_unref (DBusConnect
 
 /**
  * Decrements the reference count of a DBusConnection, and finalizes
- * it if the count reaches zero.  It is a bug to drop the last reference
- * to a connection that has not been disconnected.
+ * it if the count reaches zero.
  *
- * @todo in practice it can be quite tricky to never unref a connection
- * that's still connected; maybe there's some way we could avoid
- * the requirement.
+ * Note: it is a bug to drop the last reference to a connection that
+ * is still connected.
+ *
+ * For shared connections, libdbus will own a reference
+ * as long as the connection is connected, so you can know that either
+ * you don't have the last reference, or it's OK to drop the last reference.
+ * Most connections are shared.
+ *
+ * For private connections, the creator of the connection must arrange for
+ * dbus_connection_close() to be called prior to dropping the last reference.
+ * Private connections come from dbus_connection_open_private() or dbus_bus_get_private().
  *
  * @param connection the connection.
  */
@@ -1912,11 +2002,19 @@ dbus_connection_unref (DBusConnection *c
 }
 
 static void
-_dbus_connection_close_internal_and_unlock (DBusConnection *connection)
+_dbus_connection_close_possibly_shared_and_unlock (DBusConnection *connection)
 {
   DBusDispatchStatus status;
+
+  HAVE_LOCK_CHECK (connection);
   
   _dbus_verbose ("Disconnecting %p\n", connection);
+
+  /* We need to ref because update_dispatch_status_and_unlock will unref
+   * the connection if it was shared and libdbus was the only remaining
+   * refcount holder.
+   */
+  _dbus_connection_ref_unlocked (connection);
   
   _dbus_transport_disconnect (connection->transport);
 
@@ -1925,34 +2023,62 @@ _dbus_connection_close_internal_and_unlo
 
   /* this calls out to user code */
   _dbus_connection_update_dispatch_status_and_unlock (connection, status);
+
+  /* could also call out to user code */
+  dbus_connection_unref (connection);
 }
 
 void
-_dbus_connection_close_internal (DBusConnection *connection)
+_dbus_connection_close_possibly_shared (DBusConnection *connection)
 {
   _dbus_assert (connection != NULL);
   _dbus_assert (connection->generation == _dbus_current_generation);
 
   CONNECTION_LOCK (connection);
-  _dbus_connection_close_internal_and_unlock (connection);
+  _dbus_connection_close_possibly_shared_and_unlock (connection);
 }
 
 /**
- * Closes the connection, so no further data can be sent or received.
- * Any further attempts to send data will result in errors.  This
- * function does not affect the connection's reference count.  It's
- * safe to disconnect a connection more than once; all calls after the
+ * Closes a private connection, so no further data can be sent or received.
+ * This disconnects the transport (such as a socket) underlying the
+ * connection.
+ *
+ * Attempts to send messages after closing a connection are safe, but will result in
+ * error replies generated locally in libdbus.
+ * 
+ * This function does not affect the connection's reference count.  It's
+ * safe to close a connection more than once; all calls after the
  * first do nothing. It's impossible to "reopen" a connection, a
  * new connection must be created. This function may result in a call
  * to the DBusDispatchStatusFunction set with
  * dbus_connection_set_dispatch_status_function(), as the disconnect
  * message it generates needs to be dispatched.
  *
- * If the connection is a shared connection we print out a warning that
- * you can not close shared connection and we return.  Internal calls
- * should use _dbus_connection_close_internal() to close shared connections.
+ * If a connection is dropped by the remote application, it will
+ * close itself. 
+ * 
+ * You must close a connection prior to releasing the last reference to
+ * the connection. If you dbus_connection_unref() for the last time
+ * without closing the connection, the results are undefined; it
+ * is a bug in your program and libdbus will try to print a warning.
+ *
+ * You may not close a shared connection. Connections created with
+ * dbus_connection_open() or dbus_bus_get() are shared.
+ * These connections are owned by libdbus, and applications should
+ * only unref them, never close them. Applications can know it is
+ * safe to unref these connections because libdbus will be holding a
+ * reference as long as the connection is open. Thus, either the
+ * connection is closed and it is OK to drop the last reference,
+ * or the connection is open and the app knows it does not have the
+ * last reference.
+ *
+ * Connections created with dbus_connection_open_private() or
+ * dbus_bus_get_private() are not kept track of or referenced by
+ * libdbus. The creator of these connections is responsible for
+ * calling dbus_connection_close() prior to releasing the last
+ * reference, if the connection is not already disconnected.
  *
- * @param connection the connection.
+ * @param connection the private (unshared) connection to close
  */
 void
 dbus_connection_close (DBusConnection *connection)
@@ -1962,15 +2088,52 @@ dbus_connection_close (DBusConnection *c
 
   CONNECTION_LOCK (connection);
 
-  if (connection->shared)
+  if (connection->shareable)
     {
       CONNECTION_UNLOCK (connection);
 
-      _dbus_warn ("Applications can not close shared connections.  Please fix this in your app.  Ignoring close request and continuing.");
+      _dbus_warn ("Applications must not close shared connections - see dbus_connection_close() docs. This is a bug in the application.\n");
       return;
     }
 
-  _dbus_connection_close_internal_and_unlock (connection);
+  _dbus_connection_close_possibly_shared_and_unlock (connection);
+}
+
+/**
+ * Used internally to handle the semantics of dbus_server_set_new_connection_function().
+ * If the new connection function does not ref the connection, we want to close it.
+ *
+ * A bit of a hack, probably the new connection function should have returned a value
+ * for whether to close, or should have had to close the connection itself if it
+ * didn't want it.
+ *
+ * But, this works OK as long as the new connection function doesn't do anything
+ * crazy like keep the connection around without ref'ing it.
+ *
+ * We have lock the connection across refcount check and close in case
+ * the new connection function spawns a thread that closes and unrefs.
+ * In that case, if the app thread
+ * closes and unrefs first, we'll harmlessly close again; if the app thread
+ * still has the ref, we'll close and then the app will close harmlessly.
+ * If the app unrefs without closing, the app is broken since if the
+ * app refs from the new connection function it is supposed to also close.
+ *
+ * If we didn't atomically check the refcount and close with the lock held
+ * though, we could screw this up.
+ * 
+ * @param connection the connection
+ */
+void
+_dbus_connection_close_if_only_one_ref (DBusConnection *connection)
+{
+  CONNECTION_LOCK (connection);
+  
+  _dbus_assert (connection->refcount.value > 0);
+
+  if (connection->refcount.value == 1)
+    _dbus_connection_close_possibly_shared_and_unlock (connection);
+  else
+    CONNECTION_UNLOCK (connection);
 }
 
 static dbus_bool_t
@@ -1981,11 +2144,14 @@ _dbus_connection_get_is_connected_unlock
 }
 
 /**
- * Gets whether the connection is currently connected.  All
- * connections are connected when they are opened.  A connection may
+ * Gets whether the connection is currently open.  A connection may
  * become disconnected when the remote application closes its end, or
  * exits; a connection may also be disconnected with
  * dbus_connection_close().
+ * 
+ * There are not separate states for "closed" and "disconnected," the two
+ * terms are synonymous. This function should really be called
+ * get_is_open() but for historical reasons is not.
  *
  * @param connection the connection.
  * @returns #TRUE if the connection is still alive.
@@ -2557,7 +2723,7 @@ connection_timeout_and_complete_all_pend
       _dbus_hash_iter_init (connection->pending_replies, &iter);
       _dbus_hash_iter_next (&iter);
        
-      pending = (DBusPendingCall *) _dbus_hash_iter_get_value (&iter);
+      pending = _dbus_hash_iter_get_value (&iter);
       _dbus_pending_call_ref_unlocked (pending);
        
       _dbus_pending_call_queue_timeout_error_unlocked (pending, 
@@ -3116,6 +3282,27 @@ dbus_connection_read_write (DBusConnecti
    return _dbus_connection_read_write_dispatch(connection, timeout_milliseconds, FALSE);
 }
 
+/* We need to call this anytime we pop the head of the queue, and then
+ * update_dispatch_status_and_unlock needs to be called afterward
+ * which will "process" the disconnected message and set
+ * disconnected_message_processed.
+ */
+static void
+check_disconnected_message_arrived_unlocked (DBusConnection *connection,
+                                             DBusMessage    *head_of_queue)
+{
+  HAVE_LOCK_CHECK (connection);
+
+  /* checking that the link is NULL is an optimization to avoid the is_signal call */
+  if (connection->disconnect_message_link == NULL &&
+      dbus_message_is_signal (head_of_queue,
+                              DBUS_INTERFACE_LOCAL,
+                              "Disconnected"))
+    {
+      connection->disconnected_message_arrived = TRUE;
+    }
+}
+
 /**
  * Returns the first-received message from the incoming message queue,
  * leaving it in the queue. If the queue is empty, returns #NULL.
@@ -3163,11 +3350,15 @@ dbus_connection_borrow_message (DBusConn
   
   message = connection->message_borrowed;
 
+  check_disconnected_message_arrived_unlocked (connection, message);
+  
   /* Note that we KEEP the dispatch lock until the message is returned */
   if (message == NULL)
     _dbus_connection_release_dispatch (connection);
 
   CONNECTION_UNLOCK (connection);
+
+  /* We don't update dispatch status until it's returned or stolen */
   
   return message;
 }
@@ -3184,6 +3375,8 @@ void
 dbus_connection_return_message (DBusConnection *connection,
 				DBusMessage    *message)
 {
+  DBusDispatchStatus status;
+  
   _dbus_return_if_fail (connection != NULL);
   _dbus_return_if_fail (message != NULL);
   _dbus_return_if_fail (message == connection->message_borrowed);
@@ -3195,9 +3388,10 @@ dbus_connection_return_message (DBusConn
   
   connection->message_borrowed = NULL;
 
-  _dbus_connection_release_dispatch (connection);
-  
-  CONNECTION_UNLOCK (connection);
+  _dbus_connection_release_dispatch (connection); 
+
+  status = _dbus_connection_get_dispatch_status_unlocked (connection);
+  _dbus_connection_update_dispatch_status_and_unlock (connection, status);
 }
 
 /**
@@ -3214,6 +3408,7 @@ dbus_connection_steal_borrowed_message (
 					DBusMessage    *message)
 {
   DBusMessage *pop_message;
+  DBusDispatchStatus status;
 
   _dbus_return_if_fail (connection != NULL);
   _dbus_return_if_fail (message != NULL);
@@ -3235,8 +3430,9 @@ dbus_connection_steal_borrowed_message (
   connection->message_borrowed = NULL;
 
   _dbus_connection_release_dispatch (connection);
-  
-  CONNECTION_UNLOCK (connection);
+
+  status = _dbus_connection_get_dispatch_status_unlocked (connection);
+  _dbus_connection_update_dispatch_status_and_unlock (connection, status);
 }
 
 /* See dbus_connection_pop_message, but requires the caller to own
@@ -3271,6 +3467,8 @@ _dbus_connection_pop_message_link_unlock
                      dbus_message_get_signature (link->data),
                      connection, connection->n_incoming);
 
+      check_disconnected_message_arrived_unlocked (connection, link->data);
+      
       return link;
     }
   else
@@ -3375,7 +3573,9 @@ dbus_connection_pop_message (DBusConnect
   _dbus_verbose ("Returning popped message %p\n", message);    
 
   _dbus_connection_release_dispatch (connection);
-  CONNECTION_UNLOCK (connection);
+
+  status = _dbus_connection_get_dispatch_status_unlocked (connection);
+  _dbus_connection_update_dispatch_status_and_unlock (connection, status);
   
   return message;
 }
@@ -3476,12 +3676,12 @@ _dbus_connection_get_dispatch_status_unl
             {
               _dbus_verbose ("Sending disconnect message from %s\n",
                              _DBUS_FUNCTION_NAME);
-
-              connection_forget_shared_unlocked (connection);
-             
-              /* If we have pending calls queued timeouts on disconnect */
+          
+              /* If we have pending calls, queue their timeouts - we want the Disconnected
+               * to be the last message, after these timeouts.
+               */
               connection_timeout_and_complete_all_pending_calls_unlocked (connection);
- 
+              
               /* We haven't sent the disconnect message already,
                * and all real messages have been queued up.
                */
@@ -3538,6 +3738,27 @@ _dbus_connection_update_dispatch_status_
   function = connection->dispatch_status_function;
   data = connection->dispatch_status_data;
 
+  if (connection->disconnected_message_arrived &&
+      !connection->disconnected_message_processed)
+    {
+      connection->disconnected_message_processed = TRUE;
+      
+      /* this does an unref, but we have a ref
+       * so we should not run the finalizer here
+       * inside the lock.
+       */
+      connection_forget_shared_unlocked (connection);
+
+      if (connection->exit_on_disconnect)
+        {
+          CONNECTION_UNLOCK (connection);            
+          
+          _dbus_verbose ("Exiting on Disconnected signal\n");
+          _dbus_exit (1);
+          _dbus_assert_not_reached ("Call to exit() returned");
+        }
+    }
+  
   /* We drop the lock */
   CONNECTION_UNLOCK (connection);
   
@@ -3981,22 +4202,6 @@ dbus_connection_dispatch (DBusConnection
     {
       _dbus_verbose (" ... done dispatching in %s\n", _DBUS_FUNCTION_NAME);
       
-      if (dbus_message_is_signal (message,
-                                  DBUS_INTERFACE_LOCAL,
-                                  "Disconnected"))
-        {
-          _dbus_bus_check_connection_and_unref_unlocked (connection);
-
-          if (connection->exit_on_disconnect)
-            {
-              CONNECTION_UNLOCK (connection);            
- 
-              _dbus_verbose ("Exiting on Disconnected signal\n");
-              _dbus_exit (1);
-              _dbus_assert_not_reached ("Call to exit() returned");
-            }
-        }
-      
       _dbus_list_free_link (message_link);
       dbus_message_unref (message); /* don't want the message to count in max message limits
                                      * in computing dispatch status below
Index: dbus/dbus-internals.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-internals.c,v
retrieving revision 1.49
diff -u -p -r1.49 dbus-internals.c
--- dbus/dbus-internals.c	1 Oct 2006 03:18:47 -0000	1.49
+++ dbus/dbus-internals.c	1 Oct 2006 09:03:54 -0000
@@ -190,6 +190,9 @@
  */
 const char _dbus_no_memory_message[] = "Not enough memory";
 
+static dbus_bool_t warn_initted = FALSE;
+static dbus_bool_t fatal_warnings = FALSE;
+
 /**
  * Prints a warning message to stderr.
  *
@@ -199,12 +202,27 @@ void
 _dbus_warn (const char *format,
             ...)
 {
-  /* FIXME not portable enough? */
   va_list args;
 
+  if (!warn_initted)
+    {
+      const char *s;
+      s = _dbus_getenv ("DBUS_FATAL_WARNINGS");
+      if (s && *s)
+        fatal_warnings = TRUE;
+
+      warn_initted = TRUE;
+    }
+  
   va_start (args, format);
   vfprintf (stderr, format, args);
   va_end (args);
+
+  if (fatal_warnings)
+    {
+      fflush (stderr);
+      _dbus_abort ();
+    }
 }
 
 #ifdef DBUS_ENABLE_VERBOSE_MODE
Index: dbus/dbus-server-debug-pipe.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-server-debug-pipe.c,v
retrieving revision 1.23
diff -u -p -r1.23 dbus-server-debug-pipe.c
--- dbus/dbus-server-debug-pipe.c	16 Sep 2006 19:24:08 -0000	1.23
+++ dbus/dbus-server-debug-pipe.c	1 Oct 2006 09:03:55 -0000
@@ -317,6 +317,7 @@ _dbus_transport_debug_pipe_new (const ch
   /* If no one grabbed a reference, the connection will die,
    * and the client transport will get an immediate disconnect
    */
+  _dbus_connection_close_if_only_one_ref (connection);
   dbus_connection_unref (connection);
 
   return client_transport;
Index: dbus/dbus-server-socket.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-server-socket.c,v
retrieving revision 1.2
diff -u -p -r1.2 dbus-server-socket.c
--- dbus/dbus-server-socket.c	16 Sep 2006 19:24:08 -0000	1.2
+++ dbus/dbus-server-socket.c	1 Oct 2006 09:03:55 -0000
@@ -69,14 +69,6 @@ socket_finalize (DBusServer *server)
   dbus_free (server);
 }
 
-/**
- * @todo unreffing the connection at the end may cause
- * us to drop the last ref to the connection before
- * disconnecting it. That is invalid.
- *
- * @todo doesn't this leak a server refcount if
- * new_connection_function is NULL?
- */
 /* Return value is just for memory, not other failures. */
 static dbus_bool_t
 handle_new_client_fd_and_unlock (DBusServer *server,
@@ -140,10 +132,11 @@ handle_new_client_fd_and_unlock (DBusSer
     {
       (* new_connection_function) (server, connection,
                                    new_connection_data);
-      dbus_server_unref (server);
     }
+  dbus_server_unref (server);
   
   /* If no one grabbed a reference, the connection will die. */
+  _dbus_connection_close_if_only_one_ref (connection);
   dbus_connection_unref (connection);
 
   return TRUE;
Index: dbus/dbus-server.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-server.c,v
retrieving revision 1.49
diff -u -p -r1.49 dbus-server.c
--- dbus/dbus-server.c	1 Oct 2006 03:18:47 -0000	1.49
+++ dbus/dbus-server.c	1 Oct 2006 09:03:56 -0000
@@ -791,7 +791,15 @@ dbus_server_get_address (DBusServer *ser
  * function is passed each new connection as the connection is
  * created. If the new connection function increments the connection's
  * reference count, the connection will stay alive. Otherwise, the
- * connection will be unreferenced and closed.
+ * connection will be unreferenced and closed. The new connection
+ * function may also close the connection itself, which is considered
+ * good form if the connection is not wanted.
+ *
+ * The connection here is private in the sense of
+ * dbus_connection_open_private(), so if the new connection function
+ * keeps a reference it must arrange for the connection to be closed.
+ * i.e. libdbus does not own this connection once the new connection
+ * function takes a reference.
  *
  * @param server the server.
  * @param function a function to handle new connections.
Index: dbus/dbus-sysdeps-unix.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-sysdeps-unix.c,v
retrieving revision 1.7
diff -u -p -r1.7 dbus-sysdeps-unix.c
--- dbus/dbus-sysdeps-unix.c	1 Oct 2006 03:18:47 -0000	1.7
+++ dbus/dbus-sysdeps-unix.c	1 Oct 2006 09:03:57 -0000
@@ -2129,14 +2129,14 @@ _dbus_set_fd_nonblocking (int           
 
 #if !defined (DBUS_DISABLE_ASSERT) || defined(DBUS_BUILD_TESTS)
 /**
- * On GNU libc systems, print a crude backtrace to the verbose log.
- * On other systems, print "no backtrace support"
- *
+ * On GNU libc systems, print a crude backtrace to stderr.  On other
+ * systems, print "no backtrace support" and block for possible gdb
+ * attachment if an appropriate environment variable is set.
  */
 void
 _dbus_print_backtrace (void)
-{
-#if defined (HAVE_BACKTRACE) && defined (DBUS_ENABLE_VERBOSE_MODE)
+{  
+#if defined (HAVE_BACKTRACE) && defined (DBUS_BUILT_R_DYNAMIC)
   void *bt[500];
   int bt_size;
   int i;
@@ -2149,13 +2149,17 @@ _dbus_print_backtrace (void)
   i = 0;
   while (i < bt_size)
     {
-      _dbus_verbose ("  %s\n", syms[i]);
+      /* don't use dbus_warn since it can _dbus_abort() */
+      fprintf (stderr, "  %s\n", syms[i]);
       ++i;
     }
+  fflush (stderr);
 
   free (syms);
+#elif defined (HAVE_BACKTRACE) && ! defined (DBUS_BUILT_R_DYNAMIC)
+  _dbus_warn ("  D-Bus not built with -rdynamic so unable to print a backtrace\n");
 #else
-  _dbus_verbose ("  D-Bus not compiled with backtrace support\n");
+  _dbus_warn ("  D-Bus not compiled with backtrace support so unable to print a backtrace\n");
 #endif
 }
 #endif /* asserts or tests enabled */
Index: dbus/dbus-sysdeps.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-sysdeps.c,v
retrieving revision 1.114
diff -u -p -r1.114 dbus-sysdeps.c
--- dbus/dbus-sysdeps.c	1 Oct 2006 03:18:47 -0000	1.114
+++ dbus/dbus-sysdeps.c	1 Oct 2006 09:03:58 -0000
@@ -60,14 +60,20 @@ _DBUS_DEFINE_GLOBAL_LOCK (sid_atom_cache
 void
 _dbus_abort (void)
 {
-#ifdef DBUS_ENABLE_VERBOSE_MODE
   const char *s;
-  s = _dbus_getenv ("DBUS_PRINT_BACKTRACE");
+  
+  _dbus_print_backtrace ();
+  
+  s = _dbus_getenv ("DBUS_BLOCK_ON_ABORT");
   if (s && *s)
-    _dbus_print_backtrace ();
-#endif
+    {
+      /* don't use _dbus_warn here since it can _dbus_abort() */
+      fprintf (stderr, "  Process %lu sleeping for gdb attach\n", (unsigned long) _dbus_getpid());
+      _dbus_sleep_milliseconds (1000 * 60);
+    }
+  
   abort ();
-  _dbus_exit (1); /* in case someone manages to ignore SIGABRT */
+  _dbus_exit (1); /* in case someone manages to ignore SIGABRT ? */
 }
 #endif
 
Index: doc/TODO
===================================================================
RCS file: /cvs/dbus/dbus/doc/TODO,v
retrieving revision 1.108
diff -u -p -r1.108 TODO
--- doc/TODO	14 Sep 2006 04:26:00 -0000	1.108
+++ doc/TODO	1 Oct 2006 09:03:58 -0000
@@ -47,6 +47,11 @@ Might as Well for 1.0
 Can Be Post 1.0
 ===
 
+ - _dbus_connection_unref_unlocked() is essentially always broken because
+   the connection finalizer calls non-unlocked functions. One fix is to make 
+   the finalizer run with the lock held, but since it calls out to the app that may 
+   be pretty broken. More likely all the uses of unref_unlocked are just wrong.
+
  - if the GUID is obtained only during authentication, not in the address, 
    we could still share the connection
 
@@ -132,4 +137,8 @@ Should Be Post 1.0
 ===
 
  - look into supporting the concept of a "connection" generically
+   (what does this TODO item mean?)
+
+ - test/name-test should be named test/with-bus or something like that
+
 
Index: test/Makefile.am
===================================================================
RCS file: /cvs/dbus/dbus/test/Makefile.am,v
retrieving revision 1.42
diff -u -p -r1.42 Makefile.am
--- test/Makefile.am	25 Aug 2006 19:59:49 -0000	1.42
+++ test/Makefile.am	1 Oct 2006 09:03:58 -0000
@@ -62,12 +62,19 @@ decode_gcov_SOURCES=				\
 TEST_LIBS=$(DBUS_TEST_LIBS) $(top_builddir)/dbus/libdbus-convenience.la
 
 test_service_LDADD=$(TEST_LIBS)
+test_service_LDFLAGS=@R_DYNAMIC_LDFLAG@
 test_names_LDADD=$(TEST_LIBS)
+test_names_LDFLAGS=@R_DYNAMIC_LDFLAG@
 ## break_loader_LDADD= $(TEST_LIBS)
+## break_loader_LDFLAGS=@R_DYNAMIC_LDFLAG@
 test_shell_service_LDADD=$(TEST_LIBS)
+test_shell_service_LDFLAGS=@R_DYNAMIC_LDFLAG@
 shell_test_LDADD=$(TEST_LIBS)
+shell_test_LDFLAGS=@R_DYNAMIC_LDFLAG@
 spawn_test_LDADD=$(TEST_LIBS)
+spawn_test_LDFLAGS=@R_DYNAMIC_LDFLAG@
 decode_gcov_LDADD=$(TEST_LIBS)
+decode_gcov_LDFLAGS=@R_DYNAMIC_LDFLAG@
 
 EXTRA_DIST=
 
Index: test/test-utils.c
===================================================================
RCS file: /cvs/dbus/dbus/test/test-utils.c,v
retrieving revision 1.7
diff -u -p -r1.7 test-utils.c
--- test/test-utils.c	6 Sep 2006 22:00:07 -0000	1.7
+++ test/test-utils.c	1 Oct 2006 09:03:58 -0000
@@ -171,8 +171,6 @@ void
 test_connection_shutdown (DBusLoop       *loop,
                           DBusConnection *connection)
 {
-  _dbus_connection_close_internal (connection);
-
   if (!dbus_connection_set_watch_functions (connection,
                                             NULL,
                                             NULL,
Index: test/name-test/Makefile.am
===================================================================
RCS file: /cvs/dbus/dbus/test/name-test/Makefile.am,v
retrieving revision 1.4
diff -u -p -r1.4 Makefile.am
--- test/name-test/Makefile.am	25 Aug 2006 19:50:16 -0000	1.4
+++ test/name-test/Makefile.am	1 Oct 2006 09:03:58 -0000
@@ -22,17 +22,19 @@ test_names_SOURCES=				\
 	test-names.c
 
 test_names_LDADD=$(top_builddir)/dbus/libdbus-1.la $(top_builddir)/dbus/libdbus-convenience.la
+test_names_LDFLAGS=@R_DYNAMIC_LDFLAG@
 
 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
-
+test_pending_call_dispatch_LDFLAGS=@R_DYNAMIC_LDFLAG@
 
 test_threads_init_SOURCES =            \
 	test-threads-init.c
 
 test_threads_init_LDADD=$(top_builddir)/dbus/libdbus-1.la $(top_builddir)/dbus/libdbus-convenience.la
+test_threads_init_LDFLAGS=@R_DYNAMIC_LDFLAG@
 
 endif
 
Index: test/name-test/test-threads-init.c
===================================================================
RCS file: /cvs/dbus/dbus/test/name-test/test-threads-init.c,v
retrieving revision 1.1
diff -u -p -r1.1 test-threads-init.c
--- test/name-test/test-threads-init.c	16 Aug 2006 22:30:15 -0000	1.1
+++ test/name-test/test-threads-init.c	1 Oct 2006 09:03:58 -0000
@@ -1,6 +1,6 @@
 /**
-* Test to make sure late thread initialization works
-**/
+ * Test to make sure late thread initialization works
+ */
 
 #include <dbus/dbus.h>
 #include <dbus/dbus-sysdeps.h>
@@ -8,6 +8,7 @@
 #include <stdlib.h>
 
 #include <dbus/dbus-internals.h>
+#include <dbus/dbus-connection-internal.h>
 
 static void
 _run_iteration (DBusConnection *conn)
@@ -107,9 +108,6 @@ check_condvar_lock (DBusCondVar *condvar
 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;
Index: tools/Makefile.am
===================================================================
RCS file: /cvs/dbus/dbus/tools/Makefile.am,v
retrieving revision 1.23
diff -u -p -r1.23 Makefile.am
--- tools/Makefile.am	1 Oct 2006 03:18:47 -0000	1.23
+++ tools/Makefile.am	1 Oct 2006 09:03:58 -0000
@@ -23,9 +23,16 @@ dbus_uuidgen_SOURCES=				\
 	dbus-uuidgen.c
 
 dbus_send_LDADD= $(top_builddir)/dbus/libdbus-1.la
+dbus_send_LDFLAGS=@R_DYNAMIC_LDFLAG@
+
 dbus_monitor_LDADD= $(top_builddir)/dbus/libdbus-1.la
+dbus_monitor_LDFLAGS=@R_DYNAMIC_LDFLAG@
+
 dbus_uuidgen_LDADD= $(top_builddir)/dbus/libdbus-1.la
+dbus_uuidgen_LDFLAGS=@R_DYNAMIC_LDFLAG@
+
 dbus_launch_LDADD= $(DBUS_X_LIBS)
+dbus_launch_LDFLAGS=@R_DYNAMIC_LDFLAG@
 
 man_MANS = dbus-send.1 dbus-monitor.1 dbus-launch.1 dbus-cleanup-sockets.1 dbus-uuidgen.1
 EXTRA_DIST = $(man_MANS) run-with-tmp-session-bus.sh


More information about the dbus mailing list