2006-11-25 Dan Williams <dcbw@redhat.com>

Rework DBus manager signal handling to be more flexible.  Previously,
	only one signal handler could be registered for a particular interface.
	The DBus manager now reference counts DBus bus matches and allows multiple
	clients to register signal handlers for the same interface and sender.

	* src/NetworkManager.c
		- (main): track NMI signal handler ID and remove it when we quit

	* src/NetworkManagerMain.h
		- Keep track of NMI signal handler ID

	* src/nm-dbus-manager.c
	  src/nm-dbus-manager.h
		- rework signal handling; each signal handler references one signal
			match, but a signal match may be referenced by one or more
			signal handlers.  Matches are refcounted and are destroyed when the
			last signal handler that references the match is removed.  This is
			necessary because two signal handlers may end up requiring the same
			dbus bus match, so the match must live until the last signal handler
			is destroyed (for example, with the wpa_supplicant network interface
			dbus interface).

	* src/dhcp-manager/nm-dhcp-manager.c
		- (nm_dhcp_manager_new): track DHCP signal handler id
		- (nm_dhcp_manager_dispose): remove DHCP signal handler

	* src/vpn-manager/nm-vpn-service.c
		- (nm_vpn_service_add_watch): track VPN service signal handler id
		- (nm_vpn_service_remove_watch): remove VPN service signal handler


git-svn-id: http://svn-archive.gnome.org/svn/NetworkManager/trunk@2124 4912f4e0-d625-0410-9fb7-b9a5a253dbdc

ChangeLog

@@ -1,3 +1,35 @@
+2006-11-25  Dan Williams  <dcbw@redhat.com>
+
+	Rework DBus manager signal handling to be more flexible.  Previously,
+	only one signal handler could be registered for a particular interface.
+	The DBus manager now reference counts DBus bus matches and allows multiple
+	clients to register signal handlers for the same interface and sender.
+
+	* src/NetworkManager.c
+		- (main): track NMI signal handler ID and remove it when we quit
+
+	* src/NetworkManagerMain.h
+		- Keep track of NMI signal handler ID
+
+	* src/nm-dbus-manager.c
+	  src/nm-dbus-manager.h
+		- rework signal handling; each signal handler references one signal
+			match, but a signal match may be referenced by one or more
+			signal handlers.  Matches are refcounted and are destroyed when the
+			last signal handler that references the match is removed.  This is
+			necessary because two signal handlers may end up requiring the same
+			dbus bus match, so the match must live until the last signal handler
+			is destroyed (for example, with the wpa_supplicant network interface
+			dbus interface).
+
+	* src/dhcp-manager/nm-dhcp-manager.c
+		- (nm_dhcp_manager_new): track DHCP signal handler id
+		- (nm_dhcp_manager_dispose): remove DHCP signal handler
+
+	* src/vpn-manager/nm-vpn-service.c
+		- (nm_vpn_service_add_watch): track VPN service signal handler id
+		- (nm_vpn_service_remove_watch): remove VPN service signal handler
+
 2006-11-25  Dan Williams  <dcbw@redhat.com>
 
 	Suggested by Helmut Schaa <hschaa@suse.de>

src/NetworkManager.c

@@ -744,6 +744,7 @@ main (int argc, char *argv[])
 	NMDBusManager *	dbus_mgr;
 	DBusConnection *dbus_connection;
 	int			exit_status = EXIT_FAILURE;
+	guint32     id;
 
 	GOptionEntry options[] = {
 		{"no-daemon", 0, 0, G_OPTION_ARG_NONE, &become_daemon, "Don't become a daemon", NULL},
@@ -831,11 +832,12 @@ main (int argc, char *argv[])
 	                  G_CALLBACK (nm_name_owner_changed_handler), nm_data);
 	g_signal_connect (G_OBJECT (dbus_mgr), "dbus-connection-changed",
 	                  G_CALLBACK (nm_dbus_connection_changed_handler), nm_data);
-	nm_dbus_manager_register_signal_handler (dbus_mgr,
+	id = nm_dbus_manager_register_signal_handler (dbus_mgr,
-	                                         NMI_DBUS_INTERFACE,
+	                                              NMI_DBUS_INTERFACE,
-	                                         NULL,
+	                                              NULL,
-	                                         nm_dbus_nmi_signal_handler,
+	                                              nm_dbus_nmi_signal_handler,
-	                                         nm_data);
+	                                              nm_data);
+	nm_data->nmi_sig_handler_id = id;
 
 	/* Register DBus method handlers for the main NM objects */
 	nm_data->nm_methods = nm_dbus_nm_methods_setup (nm_data);
@@ -901,6 +903,9 @@ main (int argc, char *argv[])
 
 done:
 	nm_print_open_socks ();
+
+	nm_dbus_manager_remove_signal_handler (dbus_mgr, nm_data->nmi_sig_handler_id);
+
 	nm_data_free (nm_data);
 	/* nm_data_free needs the dbus connection, so must kill the
 	 * dbus manager after that.

src/NetworkManagerMain.h

@@ -66,6 +66,7 @@ typedef struct NMData
 	NMNamedManager *		named_manager;
 	NMVPNManager *			vpn_manager;
 	NMDHCPManager *		dhcp_manager;
+	guint32                 nmi_sig_handler_id;
 
 	NMDbusMethodList *		nm_methods;
 	NMDbusMethodList *		device_methods;

src/dhcp-manager/nm-dhcp-manager.c

@@ -58,6 +58,7 @@ struct NMDHCPManager {
 	gboolean	running;
 	size_t		dhcp_sn_len;
 	NMDBusManager *	dbus_mgr;
+	guint32     sig_handler_id;
 };
 
 
@@ -90,6 +91,7 @@ nm_dhcp_manager_new (NMData *data,
                      GMainContext *main_ctx)
 {
 	NMDHCPManager *	manager;
+	guint32         id;
 
 	g_return_val_if_fail (data != NULL, NULL);
 	g_return_val_if_fail (main_ctx != NULL, NULL);
@@ -102,11 +104,12 @@ nm_dhcp_manager_new (NMData *data,
 	                                                   DHCP_SERVICE_NAME);
 	manager->dhcp_sn_len = strlen (DHCP_SERVICE_NAME);
 
-	nm_dbus_manager_register_signal_handler (manager->dbus_mgr,
+	id = nm_dbus_manager_register_signal_handler (manager->dbus_mgr,
-	                                         DHCP_SERVICE_NAME ".state",
+	                                              DHCP_SERVICE_NAME ".state",
-	                                         DHCP_SERVICE_NAME,
+	                                              DHCP_SERVICE_NAME,
-	                                         nm_dhcp_manager_process_signal,
+	                                              nm_dhcp_manager_process_signal,
-	                                         manager);
+	                                              manager);
+	manager->sig_handler_id = id;
 	g_signal_connect (G_OBJECT (manager->dbus_mgr),
 	                  "name-owner-changed",
 	                  G_CALLBACK (nm_dhcp_manager_name_owner_changed),
@@ -124,6 +127,9 @@ void nm_dhcp_manager_dispose (NMDHCPManager *manager)
 {
 	g_return_if_fail (manager != NULL);
 
+	nm_dbus_manager_remove_signal_handler (manager->dbus_mgr,
+	                                       manager->sig_handler_id);
+
 	g_object_unref (manager->dbus_mgr);
 	memset (manager, 0, sizeof (NMDHCPManager));
 	g_slice_free (NMDHCPManager, manager);

src/nm-dbus-manager.c

@@ -30,11 +30,6 @@
 #include <string.h>
 #include "nm-utils.h"
 
-static gboolean nm_dbus_manager_init_bus (NMDBusManager *self);
-static void nm_dbus_manager_cleanup (NMDBusManager *self);
-static void free_signal_handler_data (gpointer data);
-static void start_reconnection_timeout (NMDBusManager *self);
-
 enum {
 	PROP_0,
 	PROP_MAIN_CONTEXT,
@@ -55,12 +50,21 @@ G_DEFINE_TYPE(NMDBusManager, nm_dbus_manager, G_TYPE_OBJECT)
                                         NM_TYPE_DBUS_MANAGER, \
                                         NMDBusManagerPrivate))
 
+typedef struct SignalMatch {
+	guint32  refcount;
+	char *   interface;
+	char *   sender;
+	char *   owner;
+	char *   match;
+	gboolean enabled;
+} SignalMatch;
 
 typedef struct SignalHandlerData {
-	NMDBusSignalHandlerFunc	func;
+	guint32                 id;
-	char *					sender;
+
-	gpointer				user_data;
+	NMDBusSignalHandlerFunc func;
-	gboolean				enabled;
+	gpointer                user_data;
+	SignalMatch *           match;
 } SignalHandlerData;
 
 typedef struct MethodHandlerData {
@@ -69,14 +73,28 @@ typedef struct MethodHandlerData {
 } MethodHandlerData;
 
 struct _NMDBusManagerPrivate {
-	DBusConnection *connection;
+	DBusConnection * connection;
-	GMainContext *  main_ctx;
+	GMainContext *   main_ctx;
-	GSList *		msg_handlers;
+	gboolean         started;
-	GHashTable *	signal_handlers;
+
-	gboolean		started;
+	GSList *         msg_handlers;
-	gboolean		disposed;
+
+	GSList *         matches;
+	GSList *         signal_handlers;
+	guint32          sig_handler_id_counter;
+
+	gboolean         disposed;
 };
 
+
+static gboolean nm_dbus_manager_init_bus (NMDBusManager *self);
+static void nm_dbus_manager_cleanup (NMDBusManager *self);
+static void free_signal_handler_data (SignalHandlerData * data, NMDBusManager * mgr);
+static void start_reconnection_timeout (NMDBusManager *self);
+static void signal_match_unref (SignalMatch * match, NMDBusManager * mgr);
+static void signal_match_disable (SignalMatch * match);
+
+
 NMDBusManager *
 nm_dbus_manager_get (GMainContext *ctx)
 {
@@ -106,11 +124,6 @@ static void
 nm_dbus_manager_init (NMDBusManager *self)
 {
 	self->priv = NM_DBUS_MANAGER_GET_PRIVATE (self);
-
-	self->priv->signal_handlers = g_hash_table_new_full (g_str_hash,
-	                                                     g_str_equal,
-	                                                     g_free,
-	                                                     free_signal_handler_data);
 }
 
 static void
@@ -170,7 +183,28 @@ cleanup_handler_data (gpointer item, gpointer user_data)
 	MethodHandlerData * data = (MethodHandlerData *) item;
 
 	nm_dbus_method_list_unref (data->list);
-	g_slice_free (MethodHandlerData, item);
+	memset (data, 0, sizeof (MethodHandlerData));
+	g_slice_free (MethodHandlerData, data);
+}
+
+static void
+free_signal_handler_helper (gpointer item,
+                            gpointer user_data)
+{
+	NMDBusManager * mgr = (NMDBusManager *) user_data;
+	SignalHandlerData * data = (SignalHandlerData *) item;
+
+	free_signal_handler_data (data, mgr);
+}
+
+static void
+signal_match_dispose_helper (gpointer item,
+                             gpointer user_data)
+{
+	NMDBusManager * mgr = (NMDBusManager *) user_data;
+	SignalMatch * match = (SignalMatch *) item;
+
+	signal_match_unref (match, mgr);
 }
 
 static void
@@ -180,11 +214,14 @@ nm_dbus_manager_finalize (GObject *object)
 
 	g_return_if_fail (self->priv != NULL);
 
+	/* Must be done before the dbus connection is disposed */
+	g_slist_foreach (self->priv->signal_handlers, free_signal_handler_helper, self);
+	g_slist_foreach (self->priv->matches, signal_match_dispose_helper, self);
+
 	nm_dbus_manager_cleanup (self);
 	g_main_context_unref (self->priv->main_ctx);
 	g_slist_foreach (self->priv->msg_handlers, cleanup_handler_data, NULL);
 	g_slist_free (self->priv->msg_handlers);
-	g_hash_table_destroy (self->priv->signal_handlers);
 
 	G_OBJECT_CLASS (nm_dbus_manager_parent_class)->finalize (object);
 }
@@ -242,7 +279,7 @@ static void
 nm_dbus_manager_cleanup (NMDBusManager *self)
 {
 	if (self->priv->connection) {
-		dbus_connection_close (self->priv->connection);
+		dbus_connection_unref (self->priv->connection);
 		self->priv->connection = NULL;
 	}
 	self->priv->started = FALSE;
@@ -274,37 +311,182 @@ nm_dbus_manager_reconnect (gpointer user_data)
 	return success ? FALSE : TRUE;
 }
 
-static char *
+static SignalMatch *
-get_match_for (const char *interface, const char *sender)
+signal_match_new (const char *interface,
-{
-	return g_strdup_printf ("type='signal',interface='%s',sender='%s'",
-	                        interface, sender);
-}
-
-static gboolean
-add_match_helper (NMDBusManager *self,
-                  const char *interface,
                   const char *sender)
 {
-	gboolean success = FALSE;
+	SignalMatch * match;
-	DBusError error;
-	char * match;
 
-	g_return_val_if_fail (self != NULL, FALSE);
+	g_return_val_if_fail (interface || sender, NULL);
-	g_return_val_if_fail (interface != NULL, FALSE);
-	g_return_val_if_fail (sender != NULL, FALSE);
 
-	match = get_match_for (interface, sender);
+	match = g_slice_new0 (SignalMatch);
-	dbus_error_init (&error);
+	g_return_val_if_fail (match != NULL, NULL);
-	dbus_bus_add_match (self->priv->connection, match, &error);
+	match->refcount = 1;
-	if (dbus_error_is_set (&error)) {
+
-		nm_warning ("failed to add signal match for '%s'.", interface);
+	if (interface) {
-		dbus_error_free (&error);
+		match->interface = g_strdup (interface);
-	} else {
+		if (!match->interface)
-		success = TRUE;
+			goto error;
 	}
-	g_free (match);
+
-	return success;
+	if (sender) {
+		match->sender = g_strdup (sender);
+		if (!match->sender)
+			goto error;
+	}
+
+	if (interface && sender) {
+		match->match = g_strdup_printf ("type='signal',interface='%s',sender='%s'",
+		                                interface, sender);
+	} else if (interface && !sender) {
+		match->match = g_strdup_printf ("type='signal',interface='%s'", interface);
+	} else if (sender && !interface) {
+		match->match = g_strdup_printf ("type='signal',sender='%s'", sender);
+	}
+
+	if (!match->match)
+		goto error;
+
+	return match;
+
+error:
+	signal_match_unref (match, NULL);
+	return NULL;
+}
+
+static void
+signal_match_ref (SignalMatch * match)
+{
+	g_return_if_fail (match != NULL);
+	g_return_if_fail (match->refcount > 0);
+
+	match->refcount++;
+}
+
+static void
+signal_match_unref (SignalMatch * match,
+                    NMDBusManager * mgr)
+{
+	DBusError error;
+
+	g_return_if_fail (match != NULL);
+	g_return_if_fail (match->refcount > 0);
+	
+	match->refcount--;
+	if (match->refcount > 0)
+		return;
+
+	/* Remove the DBus bus match on dispose */
+	if (mgr) {
+	 	dbus_error_init (&error);
+		dbus_bus_remove_match (mgr->priv->connection, match->match, &error);
+		if (dbus_error_is_set (&error)) {
+			nm_warning ("failed to remove signal match for sender '%s', "
+			            "interface '%s'.",
+			            match->sender ? match->sender : "(none)",
+			            match->interface ? match->interface : "(none)");
+			dbus_error_free (&error);
+		}
+	}
+	match->enabled = FALSE;
+
+	g_free (match->interface);
+	g_free (match->sender);
+	g_free (match->owner);
+	g_free (match->match);
+	memset (match, 0, sizeof (SignalMatch));
+	g_slice_free (SignalMatch, match);
+}
+
+static SignalMatch *
+find_signal_match (NMDBusManager *self,
+                   const char *interface,
+                   const char *sender)
+{
+	SignalMatch * found = NULL;
+	GSList *      elt;
+
+	g_return_val_if_fail (self != NULL, NULL);
+	g_return_val_if_fail (interface || sender, NULL);
+
+	for (elt = self->priv->matches; elt; elt = g_slist_next (elt)) {
+		SignalMatch * match = (SignalMatch *) elt->data;
+
+		if (!match)
+			continue;
+
+		if (interface && sender) {
+			if (!match->interface || !match->sender)
+				continue;
+			if (!strcmp (match->interface, interface) && !strcmp (match->sender, sender)) {
+				found = match;
+				break;
+			}
+		} else if (interface && !sender) {
+			if (!match->interface || match->sender)
+				continue;
+			if (!strcmp (match->interface, interface)) {
+				found = match;
+				break;
+			}
+		} else if (sender && !interface) {
+			if (!match->sender || match->interface)
+				continue;
+			if (!strcmp (match->sender, sender)) {
+				found = match;
+				break;
+			}
+		}
+	}
+
+	return found;
+}
+
+static void
+signal_match_enable (NMDBusManager * mgr,
+                     SignalMatch * match,
+                     const char * owner)
+{
+	DBusError error;
+
+	g_return_if_fail (match != NULL);
+
+	if (match->enabled == TRUE)
+		return;
+
+	if (!mgr->priv->connection)
+		return;
+
+	dbus_error_init (&error);
+	dbus_bus_add_match (mgr->priv->connection, match->match, &error);
+	if (dbus_error_is_set (&error)) {
+		nm_warning ("failed to add signal match for sender '%s', "
+		            "interface '%s'.",
+		            match->sender ? match->sender : "(none)",
+		            match->interface ? match->interface : "(none)");
+		dbus_error_free (&error);
+		signal_match_disable (match);
+	} else {
+		g_free (match->owner);
+		if (owner) {
+			match->owner = g_strdup (owner);
+		} else if (match->sender) {
+			match->owner = nm_dbus_manager_get_name_owner (mgr, match->sender);
+			if (match->owner == NULL)
+				nm_warning ("Couldn't get name owner for '%s'.", match->sender);
+		}
+		match->enabled = TRUE;
+	}
+}
+
+static void
+signal_match_disable (SignalMatch * match)
+{
+	g_return_if_fail (match != NULL);
+
+	match->enabled = FALSE;
+	g_free (match->owner);
+	match->owner = NULL;
 }
 
 static void
@@ -322,6 +504,56 @@ start_reconnection_timeout (NMDBusManager *self)
 	g_source_unref (source);
 }
 
+static gboolean
+dispatch_signal (NMDBusManager * self,
+                 DBusMessage *   message)
+{
+	gboolean            handled = FALSE;
+	GSList *            elt;
+	const char *        interface;
+	const char *        sender;
+
+	g_return_val_if_fail (self != NULL, FALSE);
+	g_return_val_if_fail (message != NULL, FALSE);
+
+	interface = dbus_message_get_interface (message);
+	sender = dbus_message_get_sender (message);
+
+	g_return_val_if_fail (interface != NULL, FALSE);
+	g_return_val_if_fail (sender != NULL, FALSE);
+
+	for (elt = self->priv->signal_handlers; elt; elt = g_slist_next (elt)) {
+		gboolean            dispatch = FALSE;
+		SignalHandlerData *	handler = (SignalHandlerData *) elt->data;
+		SignalMatch *       match = handler->match;
+
+		if (match->sender && !match->interface) {
+			if (!strcmp (match->sender, sender)
+			    || (match->owner && !strcmp (match->owner, sender)))
+				dispatch = TRUE;
+		} else if (match->interface && !match->sender) {
+			if (!strcmp (match->interface, interface))
+				dispatch = TRUE;
+		} else if (match->interface && match->sender) {
+			if (!strcmp (match->interface, interface)
+			    && (!strcmp (match->sender, sender)
+			        || !strcmp (match->owner, sender)))
+				dispatch = TRUE;
+		}
+		if (!dispatch)
+			continue;
+
+		handled = (*handler->func) (self->priv->connection,
+		                            message,
+		                            handler->user_data);
+		if (handled)
+			break;
+	}
+
+	return handled;
+}
+
+
 static DBusHandlerResult
 nm_dbus_manager_signal_handler (DBusConnection *connection,
                                 DBusMessage *message,
@@ -358,24 +590,22 @@ nm_dbus_manager_signal_handler (DBusConnection *connection,
 		                                 DBUS_TYPE_STRING, &new_owner,
 		                                 DBUS_TYPE_INVALID);
 		if (success) {
-			SignalHandlerData * sig_data;
+			SignalMatch *       match;
 			gboolean			old_owner_good = (old_owner && strlen (old_owner));
 			gboolean			new_owner_good = (new_owner && strlen (new_owner));
 
-			sig_data = g_hash_table_lookup (self->priv->signal_handlers,
+			match = find_signal_match (self, NULL, name);
-			                                name);
 
 			if (!old_owner_good && new_owner_good) {
-				/* Add any matches registered with us */
+				/* Add any matches for this owner */
-				if (sig_data) {
+				if (match) {
-					sig_data->enabled = add_match_helper (self,
+					signal_match_enable (self, match, new_owner);
-					                                      name,
-					                                      sig_data->sender);
 				}
 			} else if (old_owner_good && !new_owner_good) {
 				/* Mark any matches for services that have gone away as disabled. */
-				if (sig_data)
+				if (match) {
-					sig_data->enabled = FALSE;
+					signal_match_disable (match);
+				}
 			}
 
 			g_signal_emit (G_OBJECT (self), 
@@ -395,18 +625,7 @@ nm_dbus_manager_signal_handler (DBusConnection *connection,
 
 		handled = TRUE;
 	} else {
-		SignalHandlerData *	cb_data;		
+		handled = dispatch_signal (self, message);
-		const char *		interface;
-
-		interface = dbus_message_get_interface (message);
-		if (interface) {
-			if ((cb_data = g_hash_table_lookup (self->priv->signal_handlers,
-			                                    interface))) {
-				handled = (*cb_data->func) (connection,
-				                            message,
-				                            cb_data->user_data);
-			}
-		}
 	}
 
 	return (handled ? DBUS_HANDLER_RESULT_HANDLED : DBUS_HANDLER_RESULT_NOT_YET_HANDLED);
@@ -624,29 +843,6 @@ out:
 	return success;
 }
 
-static void
-register_signal_handler (gpointer key,
-                         gpointer value,
-                         gpointer user_data)
-{
-	NMDBusManager *self = NM_DBUS_MANAGER (user_data);
-	SignalHandlerData *	cb_data = (SignalHandlerData *) value;
-
-	if (nm_dbus_manager_name_has_owner (self, key))
-		add_match_helper (self, key, cb_data->sender);
-}
-
-static gboolean
-nm_dbus_manager_register_signal_handlers (NMDBusManager *self)
-{
-	g_return_val_if_fail (self != NULL, FALSE);
-
-	g_hash_table_foreach (self->priv->signal_handlers,
-	                      register_signal_handler,
-	                      self);
-	return TRUE;
-}
-
 /* Register our service on the bus; shouldn't be called until
  * all necessary message handlers have been registered, because
  * when we register on the bus, clients may start to call.
@@ -657,6 +853,7 @@ nm_dbus_manager_start_service (NMDBusManager *self)
 	DBusError	error;
 	gboolean	success = FALSE;
 	int			flags, ret;
+	GSList *    elt;
 
 	g_return_val_if_fail (self != NULL, FALSE);
 
@@ -670,8 +867,9 @@ nm_dbus_manager_start_service (NMDBusManager *self)
 		goto out;
 
 	/* And our signal handlers */
-	if (!nm_dbus_manager_register_signal_handlers (self))
+	for (elt = self->priv->matches; elt; elt = g_slist_next (elt)) {
-		goto out;
+		signal_match_enable (self, (SignalMatch *) elt->data, NULL);
+	}
 
 	dbus_error_init (&error);
 #if (DBUS_VERSION_MAJOR == 0) && (DBUS_VERSION_MINOR >= 60)
@@ -741,83 +939,108 @@ nm_dbus_manager_register_method_list (NMDBusManager *self,
 }
 
 static void
-free_signal_handler_data (gpointer data)
+free_signal_handler_data (SignalHandlerData * data,
+                          NMDBusManager * mgr)
 {
-	SignalHandlerData *	cb_data = (SignalHandlerData *) data;
+	g_return_if_fail (mgr != NULL);
+	g_return_if_fail (data != NULL);
 
-	g_return_if_fail (cb_data != NULL);
+	if (data->match)
+		signal_match_unref (data->match, mgr);
 
-	g_free (cb_data->sender);
+	memset (data, 0, sizeof (SignalHandlerData));
-	g_slice_free (SignalHandlerData, cb_data);
+	g_slice_free (SignalHandlerData, data);
 }
 
-void
+guint32
 nm_dbus_manager_register_signal_handler (NMDBusManager *self,
                                          const char *interface,
                                          const char *sender,
                                          NMDBusSignalHandlerFunc callback,
                                          gpointer user_data)
 {
-	SignalHandlerData *	cb_data;
+	static GStaticMutex mutex = G_STATIC_MUTEX_INIT;
+	SignalHandlerData *	sig_handler;
+	SignalMatch * match = NULL;
 
-	g_return_if_fail (self != NULL);
+	g_return_val_if_fail (self != NULL, 0);
-	g_return_if_fail (interface != NULL);
+	g_return_val_if_fail (callback != NULL, 0);
-	g_return_if_fail (callback != NULL);
 
-	if (!(cb_data = g_slice_new0 (SignalHandlerData)))
+	/* One of interface or sender must be specified */
-		return;
+	g_return_val_if_fail (interface || sender, 0);
-	cb_data->sender = sender ? g_strdup (sender) : g_strdup (interface);
-	cb_data->func = callback;
-	cb_data->user_data = user_data;
-	g_hash_table_insert (self->priv->signal_handlers,
-	                     g_strdup (interface),
-	                     cb_data);
 
-	if (nm_dbus_manager_name_has_owner (self, cb_data->sender))
+	if (!(sig_handler = g_slice_new0 (SignalHandlerData))) {
-		cb_data->enabled = add_match_helper (self, interface, cb_data->sender);
+		nm_warning ("Not enough memory for new signal handler.");
+		return 0;
+	}
+	sig_handler->func = callback;
+	sig_handler->user_data = user_data;
+
+	/* Find or create the DBus bus match */
+	match = find_signal_match (self, interface, sender);
+	if (match != NULL) {
+		sig_handler->match = match;
+		signal_match_ref (match);		
+	} else {
+		sig_handler->match = signal_match_new (interface, sender);
+		if (sig_handler->match == NULL) {
+			nm_warning ("Could not create new signal match.");
+			free_signal_handler_data (sig_handler, self);
+			return 0;
+		}
+	}
+
+	signal_match_enable (self, sig_handler->match, NULL);
+
+	g_static_mutex_lock (&mutex);
+	self->priv->sig_handler_id_counter++;
+	sig_handler->id = self->priv->sig_handler_id_counter;
+	g_static_mutex_unlock (&mutex);
+
+	self->priv->matches = g_slist_append (self->priv->matches, sig_handler->match);
+	self->priv->signal_handlers = g_slist_append (self->priv->signal_handlers,
+	                                              sig_handler);
+
+	return sig_handler->id;
 }
 
 void
 nm_dbus_manager_remove_signal_handler (NMDBusManager *self,
-                                       const char *interface)
+                                       guint32 id)
 {
-	SignalHandlerData *	cb_data;
+	GSList * elt;
-	DBusError error;
+	SignalHandlerData *	sig_handler = NULL;
-	char * match;
 
 	g_return_if_fail (self != NULL);
-	g_return_if_fail (interface != NULL);
+	g_return_if_fail (id > 0);
 
-	cb_data = g_hash_table_lookup (self->priv->signal_handlers, interface);
+	for (elt = self->priv->signal_handlers; elt; elt = g_slist_next (elt)) {
-	if (!cb_data)
+		SignalHandlerData * handler = (SignalHandlerData *) elt->data;
+
+		if (handler && (handler->id == id)) {
+			sig_handler = handler;
+			break;
+		}
+	}
+
+	/* Not found */
+	if (!sig_handler)
 		return;
 
-	if (cb_data->enabled == FALSE)
+	free_signal_handler_data (sig_handler, self);
-		goto out;
-	if (nm_dbus_manager_name_has_owner (self, cb_data->sender))
-		goto out;
-
-	match = get_match_for (interface, cb_data->sender);
- 	dbus_error_init (&error);
-	dbus_bus_remove_match (self->priv->connection, match, &error);
-	if (dbus_error_is_set (&error)) {
-		nm_warning ("failed to remove signal match for '%s'.", interface);
-		dbus_error_free (&error);
-	}
-	g_free (match);
-
-out:
-	g_hash_table_remove (self->priv->signal_handlers, interface);
 }
 
 DBusConnection *
 nm_dbus_manager_get_dbus_connection (NMDBusManager *self)
 {
+	DBusConnection * connection;
 	GValue value = {0,};
 
 	g_return_val_if_fail (self != NULL, NULL);
 
 	g_value_init (&value, G_TYPE_POINTER);
 	g_object_get_property (G_OBJECT (self), "dbus-connection", &value);
-	return g_value_get_pointer (&value);
+	connection = g_value_get_pointer (&value);
+	g_value_unset (&value);
+	return connection;
 }

src/nm-dbus-manager.h

@@ -80,14 +80,14 @@ void nm_dbus_manager_register_method_list (NMDBusManager *self,
 gboolean nm_dbus_manager_name_has_owner   (NMDBusManager *self,
                                            const char *name);
 
-void nm_dbus_manager_register_signal_handler (NMDBusManager *self,
+guint32 nm_dbus_manager_register_signal_handler (NMDBusManager *self,
                                            const char *interface,
                                            const char *sender,
                                            NMDBusSignalHandlerFunc callback,
                                            gpointer user_data);
 
 void nm_dbus_manager_remove_signal_handler (NMDBusManager *self,
-                                           const char *interface);
+                                           guint32 id);
 
 DBusConnection * nm_dbus_manager_get_dbus_connection (NMDBusManager *self);
 

src/vpn-manager/nm-vpn-service.c

@@ -46,6 +46,7 @@ struct NMVPNService
 	NMVPNManager *	manager;
 	NMData *		app_data;
 	GPid			pid;
+	guint32         sig_handler_id;
 	gulong			watch_id;
 	gulong			dbus_con_watch_id;
 	NMDBusManager *	dbus_mgr;
@@ -1130,6 +1131,8 @@ void nm_vpn_service_stop_connection (NMVPNService *service, NMVPNActRequest *req
 static void
 nm_vpn_service_add_watch (NMVPNService *service)
 {
+	guint32 id;
+
 	g_return_if_fail (service != NULL);
 
 	if (service->watch_id)
@@ -1138,11 +1141,12 @@ nm_vpn_service_add_watch (NMVPNService *service)
 	/* Add a dbus filter for this connection's service name so its signals
 	 * get delivered to us.
 	 */
-	nm_dbus_manager_register_signal_handler (service->dbus_mgr,
+	id = nm_dbus_manager_register_signal_handler (service->dbus_mgr,
-	                                         service->service,
+	                                              service->service,
-	                                         NULL,
+	                                              NULL,
-	                                         nm_vpn_service_process_signal,
+	                                              nm_vpn_service_process_signal,
-	                                         service);
+	                                              service);
+	service->sig_handler_id = id;
 	service->watch_id = g_signal_connect (service->dbus_mgr,
 	                                      "name-owner-changed",
 	                                      G_CALLBACK (nm_vpn_service_name_owner_changed),
@@ -1162,7 +1166,7 @@ nm_vpn_service_remove_watch (NMVPNService *service)
 	if (!service->watch_id)
 		return;
 
-	nm_dbus_manager_remove_signal_handler (service->dbus_mgr, service->service);
+	nm_dbus_manager_remove_signal_handler (service->dbus_mgr, service->sig_handler_id);
 	g_signal_handler_disconnect (service->dbus_mgr, service->watch_id);
 	service->watch_id = 0;
 	g_signal_handler_disconnect (service->dbus_mgr, service->dbus_con_watch_id);