proxy: merge branch 'th/proxy-rh1444374'

https://bugzilla.redhat.com/show_bug.cgi?id=1444374

src/devices/nm-device.c

@@ -323,7 +323,7 @@ typedef struct _NMDevicePrivate {
 	/* Proxy Configuration */
 	NMProxyConfig *proxy_config;
 	NMPacrunnerManager *pacrunner_manager;
-	bool proxy_config_sent;
+	NMPacrunnerCallId *pacrunner_call_id;
 
 	/* IP4 configuration info */
 	NMIP4Config *   ip4_config;     /* Combined config from VPN, settings, and device */
@@ -8880,25 +8880,34 @@ nm_device_reactivate_ip6_config (NMDevice *self,
 }
 
 static void
-reactivate_proxy_config (NMDevice *self)
+_pacrunner_manager_send (NMDevice *self)
 {
 	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
 
-	if (!priv->proxy_config_sent)
+	nm_pacrunner_manager_remove_clear (priv->pacrunner_manager,
-		return;
+	                                   &priv->pacrunner_call_id);
 
-	nm_pacrunner_manager_remove (priv->pacrunner_manager,
+	if (!priv->pacrunner_manager)
-	                             nm_device_get_ip_iface (self));
+		priv->pacrunner_manager = g_object_ref (nm_pacrunner_manager_get ());
 
-	nm_device_set_proxy_config (self, priv->dhcp4.pac_url);
+	priv->pacrunner_call_id = nm_pacrunner_manager_send (priv->pacrunner_manager,
-	nm_pacrunner_manager_send (priv->pacrunner_manager,
-	                           nm_device_get_ip_iface (self),
 	                                                     nm_device_get_ip_iface (self),
 	                                                     priv->proxy_config,
 	                                                     NULL,
 	                                                     NULL);
 }
 
+static void
+reactivate_proxy_config (NMDevice *self)
+{
+	NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self);
+
+	if (!priv->pacrunner_call_id)
+		return;
+	nm_device_set_proxy_config (self, priv->dhcp4.pac_url);
+	_pacrunner_manager_send (self);
+}
+
 static gboolean
 can_reapply_change (NMDevice *self, const char *setting_name,
                     NMSetting *s_old, NMSetting *s_new,
@@ -12565,11 +12574,8 @@ _set_state_full (NMDevice *self,
 			}
 		}
 
-		if (priv->proxy_config_sent) {
+		nm_pacrunner_manager_remove_clear (priv->pacrunner_manager,
-			nm_pacrunner_manager_remove (priv->pacrunner_manager,
+		                                   &priv->pacrunner_call_id);
-			                             nm_device_get_ip_iface (self));
-			priv->proxy_config_sent = FALSE;
-		}
 		break;
 	case NM_DEVICE_STATE_DISCONNECTED:
 		if (   priv->queued_act_request
@@ -12594,15 +12600,8 @@ _set_state_full (NMDevice *self,
 		                           req,
 		                           NULL, NULL, NULL);
 
-		if (priv->proxy_config) {
+		if (priv->proxy_config)
-			nm_pacrunner_manager_send (priv->pacrunner_manager,
+			_pacrunner_manager_send (self);
-			                           nm_device_get_ip_iface (self),
-			                           nm_device_get_ip_iface (self),
-			                           priv->proxy_config,
-			                           NULL,
-			                           NULL);
-			priv->proxy_config_sent = TRUE;
-		}
 		break;
 	case NM_DEVICE_STATE_FAILED:
 		/* Usually upon failure the activation chain is interrupted in
@@ -13608,8 +13607,6 @@ nm_device_init (NMDevice *self)
 	priv->ip6_saved_properties = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_free);
 	priv->sys_iface_state = NM_DEVICE_SYS_IFACE_STATE_EXTERNAL;
 
-	priv->pacrunner_manager = g_object_ref (nm_pacrunner_manager_get ());
-
 	priv->default_route.v4_is_assumed = TRUE;
 	priv->default_route.v6_is_assumed = TRUE;
 
@@ -13734,6 +13731,8 @@ dispose (GObject *object)
 
 	dispatcher_cleanup (self);
 
+	nm_pacrunner_manager_remove_clear (priv->pacrunner_manager,
+	                                   &priv->pacrunner_call_id);
 	g_clear_object (&priv->pacrunner_manager);
 
 	_cleanup_generic_pre (self, CLEANUP_TYPE_KEEP);

src/nm-pacrunner-manager.c

@@ -36,14 +36,15 @@ static void pacrunner_remove_done (GDBusProxy *proxy, GAsyncResult *res, gpointe
 
 /*****************************************************************************/
 
-typedef struct {
+struct _NMPacrunnerCallId {
-	char *tag;
 	NMPacrunnerManager *manager;
 	GVariant *args;
 	char *path;
 	guint refcount;
 	bool removed;
-} Config;
+};
+
+typedef struct _NMPacrunnerCallId Config;
 
 typedef struct {
 	char *iface;
@@ -74,16 +75,25 @@ NM_DEFINE_SINGLETON_GETTER (NMPacrunnerManager, nm_pacrunner_manager_get, NM_TYP
 #define _NMLOG_DOMAIN      LOGD_PROXY
 #define _NMLOG(level, ...) __NMLOG_DEFAULT (level, _NMLOG_DOMAIN, "pacrunner", __VA_ARGS__)
 
+#define _NMLOG2_PREFIX_NAME "pacrunner"
+#define _NMLOG2(level, config, ...) \
+	G_STMT_START { \
+		nm_log ((level), _NMLOG_DOMAIN, NULL, NULL, \
+		        "%s%p]: " _NM_UTILS_MACRO_FIRST(__VA_ARGS__), \
+		        "pacrunner: call[", \
+		        (config) \
+		        _NM_UTILS_MACRO_REST(__VA_ARGS__)); \
+	} G_STMT_END
+
 /*****************************************************************************/
 
 static Config *
-config_new (NMPacrunnerManager *manager, char *tag, GVariant *args)
+config_new (NMPacrunnerManager *manager, GVariant *args)
 {
 	Config *config;
 
 	config = g_slice_new0 (Config);
 	config->manager = manager;
-	config->tag = tag;
 	config->args = g_variant_ref_sink (args);
 	config->refcount = 1;
 
@@ -106,7 +116,6 @@ config_unref (Config *config)
 	g_assert (config->refcount > 0);
 
 	if (config->refcount == 1) {
-		g_free (config->tag);
 		g_variant_unref (config->args);
 		g_free (config->path);
 		g_slice_free (Config, config);
@@ -232,13 +241,13 @@ pacrunner_send_done (GDBusProxy *proxy, GAsyncResult *res, gpointer user_data)
 	self = NM_PACRUNNER_MANAGER (config->manager);
 	priv = NM_PACRUNNER_MANAGER_GET_PRIVATE (self);
 
-	if (!variant) {
+	if (!variant)
-		_LOGD ("send config for '%s' failed: %s", config->tag, error->message);
+		_LOG2D (config, "sending failed: %s", error->message);
-	} else {
+	else {
 		g_variant_get (variant, "(&o)", &path);
 
 		config->path = g_strdup (path);
-		_LOGD ("successfully sent config for '%s'", config->tag);
+		_LOG2D (config, "sent");
 
 		if (config->removed) {
 			g_dbus_proxy_call (priv->pacrunner,
@@ -260,10 +269,7 @@ pacrunner_send_config (NMPacrunnerManager *self, Config *config)
 	NMPacrunnerManagerPrivate *priv = NM_PACRUNNER_MANAGER_GET_PRIVATE (self);
 
 	if (priv->pacrunner) {
-		gs_free char *args_str = NULL;
+		_LOG2T (config, "sending...");
-
-		_LOGT ("sending proxy config for '%s': %s", config->tag,
-		       (args_str = g_variant_print (config->args, FALSE)));
 
 		config_ref (config);
 		g_clear_pointer (&config->path, g_free);
@@ -280,16 +286,13 @@ pacrunner_send_config (NMPacrunnerManager *self, Config *config)
 }
 
 static void
-name_owner_changed (GObject *object,
+name_owner_changed (NMPacrunnerManager *self)
-                    GParamSpec *pspec,
-                    gpointer user_data)
 {
-	NMPacrunnerManager *self = NM_PACRUNNER_MANAGER (user_data);
 	NMPacrunnerManagerPrivate *priv = NM_PACRUNNER_MANAGER_GET_PRIVATE (self);
 	gs_free char *owner = NULL;
 	GList *iter = NULL;
 
-	owner = g_dbus_proxy_get_name_owner (G_DBUS_PROXY (object));
+	owner = g_dbus_proxy_get_name_owner (priv->pacrunner);
 	if (owner) {
 		_LOGD ("name owner appeared (%s)", owner);
 		for (iter = g_list_first (priv->configs); iter; iter = g_list_next (iter))
@@ -299,6 +302,14 @@ name_owner_changed (GObject *object,
 	}
 }
 
+static void
+name_owner_changed_cb (GObject *object,
+                       GParamSpec *pspec,
+                       gpointer user_data)
+{
+	name_owner_changed (user_data);
+}
+
 static void
 pacrunner_proxy_cb (GObject *source, GAsyncResult *res, gpointer user_data)
 {
@@ -321,22 +332,29 @@ pacrunner_proxy_cb (GObject *source, GAsyncResult *res, gpointer user_data)
 	nm_clear_g_cancellable (&priv->pacrunner_cancellable);
 
 	g_signal_connect (priv->pacrunner, "notify::g-name-owner",
-	                  G_CALLBACK (name_owner_changed), self);
+	                  G_CALLBACK (name_owner_changed_cb), self);
+	name_owner_changed (self);
 }
 
 /**
  * nm_pacrunner_manager_send:
  * @self: the #NMPacrunnerManager
  * @iface: the iface for the connection or %NULL
- * @tag: unique configuration identifier
  * @proxy_config: proxy config of the connection
  * @ip4_config: IP4 config of the connection to extract domain info from
  * @ip6_config: IP6 config of the connection to extract domain info from
+ *
+ * Returns: a #NMPacrunnerCallId call id. The function cannot
+ *  fail and always returns a non NULL pointer. The call-id may
+ *  be used to remove the configuration later via nm_pacrunner_manager_remove().
+ *  Note that the call-id does not keep the @self instance alive.
+ *  If you plan to remove the configuration later, you must keep
+ *  the instance alive long enough. You can remove the configuration
+ *  at most once using this call call-id.
  */
-void
+NMPacrunnerCallId *
 nm_pacrunner_manager_send (NMPacrunnerManager *self,
                            const char *iface,
-                           const char *tag,
                            NMProxyConfig *proxy_config,
                            NMIP4Config *ip4_config,
                            NMIP6Config *ip6_config)
@@ -348,8 +366,8 @@ nm_pacrunner_manager_send (NMPacrunnerManager *self,
 	GPtrArray *domains;
 	Config *config;
 
-	g_return_if_fail (NM_IS_PACRUNNER_MANAGER (self));
+	g_return_val_if_fail (NM_IS_PACRUNNER_MANAGER (self), NULL);
-	g_return_if_fail (proxy_config);
+	g_return_val_if_fail (proxy_config, NULL);
 
 	priv = NM_PACRUNNER_MANAGER_GET_PRIVATE (self);
 
@@ -401,15 +419,23 @@ nm_pacrunner_manager_send (NMPacrunnerManager *self,
 		}
 	}
 
-	config = config_new (self, g_strdup (tag),
+	config = config_new (self, g_variant_new ("(a{sv})", &proxy_data));
-	                     g_variant_new ("(a{sv})", &proxy_data));
 	priv->configs = g_list_append (priv->configs, config);
 
+	{
+		gs_free char *args_str = NULL;
+
+		_LOG2D (config, "send: new config %s",
+		        (args_str = g_variant_print (config->args, FALSE)));
+	}
+
 	/* Send if pacrunner is available on bus, otherwise
 	 * config has already been appended above to be
 	 * sent when pacrunner appears.
 	 */
 	pacrunner_send_config (self, config);
+
+	return config;
 }
 
 static void
@@ -429,9 +455,9 @@ pacrunner_remove_done (GDBusProxy *proxy, GAsyncResult *res, gpointer user_data)
 	self = NM_PACRUNNER_MANAGER (config->manager);
 
 	if (!ret)
-		_LOGD ("couldn't remove config for '%s': %s", config->tag, error->message);
+		_LOG2D (config, "remove failed: %s", error->message);
 	else
-		_LOGD ("successfully removed config for '%s'", config->tag);
+		_LOG2D (config, "removed");
 
 	config_unref (config);
 }
@@ -439,23 +465,27 @@ pacrunner_remove_done (GDBusProxy *proxy, GAsyncResult *res, gpointer user_data)
 /**
  * nm_pacrunner_manager_remove:
  * @self: the #NMPacrunnerManager
- * @iface: the iface for the connection to be removed
+ * @call_id: the call-id obtained from nm_pacrunner_manager_send()
- * from pacrunner
  */
 void
-nm_pacrunner_manager_remove (NMPacrunnerManager *self, const char *tag)
+nm_pacrunner_manager_remove (NMPacrunnerManager *self, NMPacrunnerCallId *call_id)
 {
-	NMPacrunnerManagerPrivate *priv = NM_PACRUNNER_MANAGER_GET_PRIVATE (self);
+	NMPacrunnerManagerPrivate *priv;
+	Config *config;
 	GList *list;
 
-	g_return_if_fail (tag);
+	g_return_if_fail (NM_IS_PACRUNNER_MANAGER (self));
+	g_return_if_fail (call_id);
 
-	_LOGT ("removing config for '%s'", tag);
+	config = call_id;
+	priv = NM_PACRUNNER_MANAGER_GET_PRIVATE (self);
 
-	for (list = g_list_first (priv->configs); list; list = g_list_next (list)) {
+	_LOG2T (config, "removing...");
-		Config *config = list->data;
+
+	list = g_list_find (priv->configs, config);
+	if (!list)
+		g_return_if_reached ();
 
-		if (nm_streq (config->tag, tag)) {
 	if (priv->pacrunner) {
 		if (!config->path) {
 			/* send() failed or is still pending. Mark the item as
@@ -477,11 +507,22 @@ nm_pacrunner_manager_remove (NMPacrunnerManager *self, const char *tag)
 	} else
 		config_unref (config);
 	priv->configs = g_list_delete_link (priv->configs, list);
-			return;
+}
-		}
+
-	}
+gboolean
-	/* bug, remove() should always match a previous send() for a given tag */
+nm_pacrunner_manager_remove_clear (NMPacrunnerManager *self,
-	g_return_if_reached ();
+                                   NMPacrunnerCallId **p_call_id)
+{
+	g_return_val_if_fail (p_call_id, FALSE);
+
+	/* if we have no call-id, allow for %NULL */
+	g_return_val_if_fail ((!self && !*p_call_id) || NM_IS_PACRUNNER_MANAGER (self), FALSE);
+
+	if (!*p_call_id)
+		return FALSE;
+	nm_pacrunner_manager_remove (self,
+	                             g_steal_pointer (p_call_id));
+	return TRUE;
 }
 
 /*****************************************************************************/

src/nm-pacrunner-manager.h

@@ -15,7 +15,8 @@
  * with this program; if not, write to the Free Software Foundation, Inc.,
  * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
  *
- * (C) Copyright 2016 Atul Anand <atulhjp@gmail.com>.
+ * Copyright 2016 Atul Anand <atulhjp@gmail.com>.
+ * Copyright 2016 - 2017 Red Hat, Inc.
  */
 
 #ifndef __NETWORKMANAGER_PACRUNNER_MANAGER_H__
@@ -30,17 +31,22 @@
 
 typedef struct _NMPacrunnerManagerClass NMPacrunnerManagerClass;
 
+typedef struct _NMPacrunnerCallId NMPacrunnerCallId;
+
 GType nm_pacrunner_manager_get_type (void);
 
 NMPacrunnerManager *nm_pacrunner_manager_get (void);
 
-void nm_pacrunner_manager_send (NMPacrunnerManager *self,
+NMPacrunnerCallId *nm_pacrunner_manager_send (NMPacrunnerManager *self,
                                               const char *iface,
-                                const char *tag,
                                               NMProxyConfig *proxy_config,
                                               NMIP4Config *ip4_config,
                                               NMIP6Config *ip6_config);
 
-void nm_pacrunner_manager_remove (NMPacrunnerManager *self, const char *tag);
+void nm_pacrunner_manager_remove (NMPacrunnerManager *self,
+                                  NMPacrunnerCallId *call_id);
+
+gboolean nm_pacrunner_manager_remove_clear (NMPacrunnerManager *self,
+                                            NMPacrunnerCallId **p_call_id);
 
 #endif /* __NETWORKMANAGER_PACRUNNER_MANAGER_H__ */

src/vpn/nm-vpn-connection.c

@@ -128,7 +128,8 @@ typedef struct {
 	GVariant *connect_hash;
 	guint connect_timeout;
 	NMProxyConfig *proxy_config;
-	gboolean proxy_config_sent;
+	NMPacrunnerManager *pacrunner_manager;
+	NMPacrunnerCallId *pacrunner_call_id;
 	gboolean has_ip4;
 	NMIP4Config *ip4_config;
 	guint32 ip4_internal_gw;
@@ -559,13 +560,18 @@ _set_vpn_state (NMVpnConnection *self,
 		                        NULL);
 
 		if (priv->proxy_config) {
-			nm_pacrunner_manager_send (nm_pacrunner_manager_get (),
+			nm_pacrunner_manager_remove_clear (priv->pacrunner_manager,
+			                                   &priv->pacrunner_call_id);
+			if (!priv->pacrunner_manager) {
+				/* the pending call doesn't keep NMPacrunnerManager alive.
+				 * Take a reference to it. */
+				priv->pacrunner_manager = g_object_ref (nm_pacrunner_manager_get ());
+			}
+			priv->pacrunner_call_id = nm_pacrunner_manager_send (priv->pacrunner_manager,
 			                                                     priv->ip_iface,
-			                           nm_connection_get_uuid (applied),
 			                                                     priv->proxy_config,
 			                                                     priv->ip4_config,
 			                                                     priv->ip6_config);
-			priv->proxy_config_sent = TRUE;
 		}
 		break;
 	case STATE_DEACTIVATING:
@@ -596,11 +602,8 @@ _set_vpn_state (NMVpnConnection *self,
 			}
 		}
 
-		if (priv->proxy_config_sent) {
+		nm_pacrunner_manager_remove_clear (priv->pacrunner_manager,
-			nm_pacrunner_manager_remove (nm_pacrunner_manager_get(),
+		                                   &priv->pacrunner_call_id);
-			                             nm_connection_get_uuid (applied));
-			priv->proxy_config_sent = FALSE;
-		}
 		break;
 	case STATE_FAILED:
 	case STATE_DISCONNECTED:
@@ -2649,6 +2652,10 @@ dispose (GObject *object)
 
 	fw_call_cleanup (self);
 
+	nm_pacrunner_manager_remove_clear (priv->pacrunner_manager,
+	                                   &priv->pacrunner_call_id);
+	g_clear_object (&priv->pacrunner_manager);
+
 	G_OBJECT_CLASS (nm_vpn_connection_parent_class)->dispose (object);
 }