diff options
| author | Havoc Pennington <hp@redhat.com> | 2006-10-01 15:36:19 +0000 | 
|---|---|---|
| committer | Havoc Pennington <hp@redhat.com> | 2006-10-01 15:36:19 +0000 | 
| commit | a2129f7cccaf0265fffe0da79ca0510b6e01131b (patch) | |
| tree | a1d889dfa99ffb86f62d87bca77d1c5b345ebfdb | |
| parent | eef10bc3c4b0a084477e713963d0a3121d652e3c (diff) | |
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
| -rw-r--r-- | ChangeLog | 65 | ||||
| -rw-r--r-- | bus/Makefile.am | 13 | ||||
| -rw-r--r-- | bus/dispatch.c | 10 | ||||
| -rw-r--r-- | configure.in | 14 | ||||
| -rw-r--r-- | dbus/Makefile.am | 5 | ||||
| -rw-r--r-- | dbus/dbus-bus.c | 81 | ||||
| -rw-r--r-- | dbus/dbus-bus.h | 2 | ||||
| -rw-r--r-- | dbus/dbus-connection-internal.h | 3 | ||||
| -rw-r--r-- | dbus/dbus-connection.c | 443 | ||||
| -rw-r--r-- | dbus/dbus-internals.c | 20 | ||||
| -rw-r--r-- | dbus/dbus-server-debug-pipe.c | 1 | ||||
| -rw-r--r-- | dbus/dbus-server-socket.c | 11 | ||||
| -rw-r--r-- | dbus/dbus-server.c | 10 | ||||
| -rw-r--r-- | dbus/dbus-sysdeps-unix.c | 18 | ||||
| -rw-r--r-- | dbus/dbus-sysdeps.c | 16 | ||||
| -rw-r--r-- | doc/TODO | 9 | ||||
| -rw-r--r-- | test/Makefile.am | 7 | ||||
| -rw-r--r-- | test/name-test/Makefile.am | 4 | ||||
| -rw-r--r-- | test/name-test/test-threads-init.c | 8 | ||||
| -rw-r--r-- | test/test-utils.c | 2 | ||||
| -rw-r--r-- | tools/Makefile.am | 7 | 
21 files changed, 562 insertions, 187 deletions
@@ -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)  diff --git a/bus/Makefile.am b/bus/Makefile.am index d3f650eb..391ea509 100644 --- a/bus/Makefile.am +++ b/bus/Makefile.am @@ -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 diff --git a/bus/dispatch.c b/bus/dispatch.c index d8b6ffb4..d374f75a 100644 --- a/bus/dispatch.c +++ b/bus/dispatch.c @@ -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"); diff --git a/configure.in b/configure.in index bfe50d6b..1be4a96c 100644 --- a/configure.in +++ b/configure.in @@ -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]) diff --git a/dbus/Makefile.am b/dbus/Makefile.am index ba326c14..d9d588aa 100644 --- a/dbus/Makefile.am +++ b/dbus/Makefile.am @@ -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: diff --git a/dbus/dbus-bus.c b/dbus/dbus-bus.c index a2336786..bc8c107f 100644 --- a/dbus/dbus-bus.c +++ b/dbus/dbus-bus.c @@ -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 diff --git a/dbus/dbus-bus.h b/dbus/dbus-bus.h index 6cefe353..b4933af3 100644 --- a/dbus/dbus-bus.h +++ b/dbus/dbus-bus.h @@ -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 diff --git a/dbus/dbus-connection-internal.h b/dbus/dbus-connection-internal.h index 6f42e168..55775d2e 100644 --- a/dbus/dbus-connection-internal.h +++ b/dbus/dbus-connection-internal.h @@ -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, diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c index 254eb008..252b5e5e 100644 --- a/dbus/dbus-connection.c +++ b/dbus/dbus-connection.c @@ -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 diff --git a/dbus/dbus-internals.c b/dbus/dbus-internals.c index 707b4005..ecddfb6f 100644 --- a/dbus/dbus-internals.c +++ b/dbus/dbus-internals.c @@ -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 diff --git a/dbus/dbus-server-debug-pipe.c b/dbus/dbus-server-debug-pipe.c index 925f8971..0fb45c63 100644 --- a/dbus/dbus-server-debug-pipe.c +++ b/dbus/dbus-server-debug-pipe.c @@ -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; diff --git a/dbus/dbus-server-socket.c b/dbus/dbus-server-socket.c index f4b5b0fd..5c11e145 100644 --- a/dbus/dbus-server-socket.c +++ b/dbus/dbus-server-socket.c @@ -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; diff --git a/dbus/dbus-server.c b/dbus/dbus-server.c index 3b9ee341..e8bb8d11 100644 --- a/dbus/dbus-server.c +++ b/dbus/dbus-server.c @@ -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. diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c index d0538010..55b61a39 100644 --- a/dbus/dbus-sysdeps-unix.c +++ b/dbus/dbus-sysdeps-unix.c @@ -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 */ diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c index 2db45900..e8c03ef3 100644 --- a/dbus/dbus-sysdeps.c +++ b/dbus/dbus-sysdeps.c @@ -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 @@ -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 + diff --git a/test/Makefile.am b/test/Makefile.am index 5c897797..1416f82a 100644 --- a/test/Makefile.am +++ b/test/Makefile.am @@ -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= diff --git a/test/name-test/Makefile.am b/test/name-test/Makefile.am index f4dc5343..75b20377 100644 --- a/test/name-test/Makefile.am +++ b/test/name-test/Makefile.am @@ -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 diff --git a/test/name-test/test-threads-init.c b/test/name-test/test-threads-init.c index e6743cce..f059993b 100644 --- a/test/name-test/test-threads-init.c +++ b/test/name-test/test-threads-init.c @@ -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; diff --git a/test/test-utils.c b/test/test-utils.c index 3577bf9c..9665eda3 100644 --- a/test/test-utils.c +++ b/test/test-utils.c @@ -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, diff --git a/tools/Makefile.am b/tools/Makefile.am index a3d786c4..fa93f621 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -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  | 
