2006-10-01 Havoc Pennington <hp@redhat.com>
authorHavoc Pennington <hp@redhat.com>
Sun, 1 Oct 2006 15:36:19 +0000 (15:36 +0000)
committerHavoc Pennington <hp@redhat.com>
Sun, 1 Oct 2006 15:36:19 +0000 (15:36 +0000)
* 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

21 files changed:
ChangeLog
bus/Makefile.am
bus/dispatch.c
configure.in
dbus/Makefile.am
dbus/dbus-bus.c
dbus/dbus-bus.h
dbus/dbus-connection-internal.h
dbus/dbus-connection.c
dbus/dbus-internals.c
dbus/dbus-server-debug-pipe.c
dbus/dbus-server-socket.c
dbus/dbus-server.c
dbus/dbus-sysdeps-unix.c
dbus/dbus-sysdeps.c
doc/TODO
test/Makefile.am
test/name-test/Makefile.am
test/name-test/test-threads-init.c
test/test-utils.c
tools/Makefile.am

index fbba11a..b2bd8dd 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,68 @@
+2006-10-01  Havoc Pennington  <hp@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@redhat.com>
 
        * dbus/dbus-bus.c (dbus_bus_get_private, dbus_bus_get) 
index d3f650e..391ea50 100644 (file)
@@ -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 d8b6ffb..d374f75 100644 (file)
@@ -1479,7 +1479,7 @@ check_hello_connection (BusContext *context)
 
   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 *test_data_dir)
   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 *test_data_dir)
   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 *test_data_dir)
   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 *test_data_dir)
   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 bfe50d6..1be4a96 100644 (file)
@@ -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 ba326c1..d9d588a 100644 (file)
@@ -164,7 +164,9 @@ noinst_LTLIBRARIES=libdbus-convenience.la
 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 a233678..bc8c107 100644 (file)
@@ -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 *connection)
   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 6cefe35..b4933af 100644 (file)
@@ -68,7 +68,7 @@ void            dbus_bus_remove_match     (DBusConnection *connection,
                                            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 6f42e16..55775d2 100644 (file)
@@ -77,7 +77,8 @@ DBusConnection*   _dbus_connection_new_for_transport           (DBusTransport
 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 254eb00..252b5e5 100644 (file)
@@ -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_connection_last_unref                        (DB
 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 (DBusConnection *connection,
  */ 
 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 (DBusTransport *transport)
   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 = NULL;
 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 (DBusAddressEntry  *entry,
       
       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 (DBusConnection *connection,
   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 (DBusConnection *connection,
     }
 
   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 char     *address,
         {
           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 char     *address,
       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 char     *address,
   else
     {
       dbus_error_free (&first_error);
+
+      CONNECTION_UNLOCK (connection);
     }
   
   dbus_address_entries_free (entries);
@@ -1683,6 +1754,10 @@ _dbus_connection_open_internal (const char     *address,
  * 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     *address,
  * 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 (DBusConnection *connection)
 
 /**
  * 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.
+ *
+ * Note: it is a bug to drop the last reference to a connection that
+ * is still connected.
  *
- * @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.
+ * 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 *connection)
 }
 
 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_unlock (DBusConnection *connection)
 
   /* 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.
- *
- * @param connection the connection.
+ * 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 private (unshared) connection to close
  */
 void
 dbus_connection_close (DBusConnection *connection)
@@ -1962,15 +2088,52 @@ dbus_connection_close (DBusConnection *connection)
 
   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 to 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_unlocked (DBusConnection *connection)
 }
 
 /**
- * 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_pending_calls_unlocked (DBusConnection *conn
       _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 (DBusConnection *connection,
    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 (DBusConnection *connection)
   
   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 (DBusConnection *connection,
   
   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 (DBusConnection *connection,
                                        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 (DBusConnection *connection,
   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_unlocked (DBusConnection *connection)
                      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 (DBusConnection *connection)
   _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_unlocked (DBusConnection *connection)
             {
               _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_and_unlock (DBusConnection    *connectio
   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 *connection)
     {
       _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 707b400..ecddfb6 100644 (file)
  */
 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 925f897..0fb45c6 100644 (file)
@@ -317,6 +317,7 @@ _dbus_transport_debug_pipe_new (const char     *server_name,
   /* 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 f4b5b0f..5c11e14 100644 (file)
@@ -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 (DBusServer *server,
     {
       (* 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 3b9ee34..e8bb8d1 100644 (file)
@@ -791,7 +791,15 @@ dbus_server_get_address (DBusServer *server)
  * 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 d053801..55b61a3 100644 (file)
@@ -2129,14 +2129,14 @@ _dbus_set_fd_nonblocking (int             fd,
 
 #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)
+  fprintf (stderr, "  D-Bus not built with -rdynamic so unable to print a backtrace\n");
 #else
-  _dbus_verbose ("  D-Bus not compiled with backtrace support\n");
+  fprintf (stderr, "  D-Bus not compiled with backtrace support so unable to print a backtrace\n");
 #endif
 }
 #endif /* asserts or tests enabled */
index 2db4590..e8c03ef 100644 (file)
@@ -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 fc9ef2e..38ed940 100644 (file)
--- a/doc/TODO
+++ b/doc/TODO
@@ -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 5c89779..1416f82 100644 (file)
@@ -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 f4dc534..75b2037 100644 (file)
@@ -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 e6743cc..f059993 100644 (file)
@@ -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 *condvar1,
 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 3577bf9..9665eda 100644 (file)
@@ -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 a3d786c..fa93f62 100644 (file)
@@ -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