This patch was created by Thomas Haller for NetworkManager 1.4. It was
backported to NetworkManager 1.2 for Ubuntu by Sjors Gielen in July 2017.

https://bugzilla.gnome.org/show_bug.cgi?id=784636
https://bugs.launchpad.net/network-manager/+bug/1696415

--------

Date: Mon, 5 Sep 2016 12:32:40 +0200
Subject: dhcp: call synchronous Notify D-Bus method from nm-dhcp-helper

A D-Bus signal is asynchronous and it can happen that nm-dhcp-helper
emits the "Event" signal before the server is able to register a handler:

   NM_DHCP_HELPER=/usr/libexec/nm-dhcp-helper
   nmcli general logging level TRACE
   for i in `seq 1 500`; do $NM_DHCP_HELPER & done
   journalctl -u NetworkManager --since '1 min ago' | grep "didn't have associated interface" | wc -l
    499

Avoid that, by calling the synchronous D-Bus method "Notify".

Interestingly, this race seem to exist since 2007.

Actually, we called g_dbus_connection_signal_subscribe() from inside
GDBusServer:new-connection signal. So it is not clear how such a race
could exist. I was not able to reproduce it by putting a sleep
before g_dbus_connection_signal_subscribe(). On the other hand, there
is bug rh#1372854 and above reproducer which strongly indicates that
events can be lost under certain circumstances.
Now we instead g_dbus_connection_register_object() from the
new-connection signal. According to my tests there was no more race
as also backed by glib's documentation. Still, keep a simple retry-loop
in nm-dhcp-helper just to be sure.

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

--- a/src/dhcp-manager/nm-dhcp-helper.c
+++ b/src/dhcp-manager/nm-dhcp-helper.c
@@ -27,6 +27,11 @@
 
 #define NM_DHCP_CLIENT_DBUS_IFACE   "org.freedesktop.nm_dhcp_client"
 
+#define NM_DHCP_HELPER_SERVER_BUS_NAME          "org.freedesktop.nm_dhcp_server"
+#define NM_DHCP_HELPER_SERVER_OBJECT_PATH       "/org/freedesktop/nm_dhcp_server"
+#define NM_DHCP_HELPER_SERVER_INTERFACE_NAME    "org.freedesktop.nm_dhcp_server"
+#define NM_DHCP_HELPER_SERVER_METHOD_NOTIFY     "Notify"
+
 static const char * ignore[] = {"PATH", "SHLVL", "_", "PWD", "dhc_dbus", NULL};
 
 static GVariant *
@@ -70,7 +75,7 @@ build_signal_parameters (void)
 		g_free (name);
 	}
 
-	return g_variant_new ("(a{sv})", &builder);
+	return g_variant_ref_sink (g_variant_new ("(a{sv})", &builder));
 }
 
 static void
@@ -92,8 +97,12 @@ fatal_error (void)
 int
 main (int argc, char *argv[])
 {
-	GDBusConnection *connection;
+	GDBusConnection *connection = NULL;
 	GError *error = NULL;
+	GVariant *parameters = NULL;
+	GVariant *result = NULL;
+	guint try_count = 0;
+	gint64 time_end;
 
 	nm_g_type_init ();
 
@@ -108,17 +117,72 @@ main (int argc, char *argv[])
 		fatal_error ();
 	}
 
-	if (!g_dbus_connection_emit_signal (connection,
+	parameters = build_signal_parameters ();
+
+	time_end = g_get_monotonic_time () + (200 * 1000L); /* retry for at least 200 milliseconds */
+
+do_notify:
+	try_count++;
+	result = g_dbus_connection_call_sync (connection,
 	                                    NULL,
-	                                    "/",
-	                                    NM_DHCP_CLIENT_DBUS_IFACE,
-	                                    "Event",
-	                                    build_signal_parameters (),
-	                                    &error)) {
-		g_dbus_error_strip_remote_error (error);
-		g_printerr ("Error: Could not send DHCP Event signal: %s\n", error->message);
-		g_error_free (error);
-		fatal_error ();
+	                                    NM_DHCP_HELPER_SERVER_OBJECT_PATH,
+	                                    NM_DHCP_HELPER_SERVER_INTERFACE_NAME,
+	                                    NM_DHCP_HELPER_SERVER_METHOD_NOTIFY,
+	                                    parameters,
+	                                    NULL,
+	                                    G_DBUS_CALL_FLAGS_NONE,
+	                                    1000,
+	                                    NULL,
+	                                    &error);
+
+	if (!result) {
+		gs_free char *s_err = NULL;
+
+		s_err = g_dbus_error_get_remote_error (error);
+		if (NM_IN_STRSET (s_err, "org.freedesktop.DBus.Error.UnknownMethod")) {
+			gint64 remaining_time = time_end - g_get_monotonic_time ();
+
+			/* I am not sure that a race can actually happen, as we register the object
+			 * on the server side during GDBusServer:new-connection signal.
+			 *
+			 * However, there was also a race for subscribing to an event, so let's just
+			 * do some retry. */
+			if (remaining_time > 0) {
+				gint64 sleeptime = 100L * (1L << try_count);
+				if(sleeptime < 5000) {
+					sleeptime = 5000;
+				}
+				if(sleeptime > 25000) {
+					sleeptime = 25000;
+				}
+				if(sleeptime > remaining_time) {
+					sleeptime = remaining_time;
+				}
+				g_printerr ("failure to call notify: %s (retry %u)\n", error->message, try_count);
+				g_usleep (sleeptime);
+				g_clear_error (&error);
+				goto do_notify;
+			}
+		}
+		g_printerr ("failure to call notify: %s (try signal via Event)\n", error->message);
+		g_clear_error (&error);
+
+		/* for backward compatibilty, try to emit the signal. There is no stable
+		 * API between the dhcp-helper and NetworkManager. However, while upgrading
+		 * the NetworkManager package, a newer helper might want to notify an
+		 * older server, which still uses the "Event". */
+		if (!g_dbus_connection_emit_signal (connection,
+		                                    NULL,
+		                                    "/",
+		                                    NM_DHCP_CLIENT_DBUS_IFACE,
+		                                    "Event",
+		                                    parameters,
+		                                    &error)) {
+			g_dbus_error_strip_remote_error (error);
+			g_printerr ("could not send DHCP Event signal: %s\n", error->message);
+			g_error_free (error);
+			fatal_error ();
+		}
 	}
 
 	if (!g_dbus_connection_flush_sync (connection, NULL, &error)) {
--- a/src/dhcp-manager/nm-dhcp-listener.c
+++ b/src/dhcp-manager/nm-dhcp-listener.c
@@ -36,6 +36,11 @@
 #define PRIV_SOCK_PATH            NMRUNDIR "/private-dhcp"
 #define PRIV_SOCK_TAG             "dhcp"
 
+#define NM_DHCP_HELPER_SERVER_BUS_NAME          "org.freedesktop.nm_dhcp_server"
+#define NM_DHCP_HELPER_SERVER_OBJECT_PATH       "/org/freedesktop/nm_dhcp_server"
+#define NM_DHCP_HELPER_SERVER_INTERFACE_NAME    "org.freedesktop.nm_dhcp_server"
+#define NM_DHCP_HELPER_SERVER_METHOD_NOTIFY     "Notify"
+
 typedef struct {
 	NMBusManager *      dbus_mgr;
 	gulong              new_conn_id;
@@ -88,13 +93,14 @@ get_option (GVariant *options, const cha
 }
 
 static void
-handle_event (GDBusConnection  *connection,
-              const char       *sender_name,
-              const char       *object_path,
-              const char       *interface_name,
-              const char       *signal_name,
-              GVariant         *parameters,
-              gpointer          user_data)
+_method_call (GDBusConnection *connection,
+              const char *sender,
+              const char *object_path,
+              const char *interface_name,
+              const char *method_name,
+              GVariant *parameters,
+              GDBusMethodInvocation *invocation,
+              gpointer user_data)
 {
 	NMDhcpListener *self = NM_DHCP_LISTENER (user_data);
 	char *iface = NULL;
@@ -104,8 +110,12 @@ handle_event (GDBusConnection  *connecti
 	gboolean handled = FALSE;
 	GVariant *options;
 
+	if (!nm_streq0 (interface_name, NM_DHCP_HELPER_SERVER_INTERFACE_NAME))
+		g_return_if_reached ();
+	if (!nm_streq0 (method_name, NM_DHCP_HELPER_SERVER_METHOD_NOTIFY))
+		g_return_if_reached ();
 	if (!g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(a{sv})")))
-		return;
+		g_return_if_reached ();
 
 	g_variant_get (parameters, "(@a{sv})", &options);
 
@@ -142,6 +152,57 @@ out:
 	g_free (pid_str);
 	g_free (reason);
 	g_variant_unref (options);
+	g_dbus_method_invocation_return_value (invocation, NULL);
+}
+
+static guint
+_dbus_connection_register_object (NMDhcpListener *self,
+                                  GDBusConnection *connection,
+                                  GError **error)
+{
+	static GDBusArgInfo arg_info_notify_in = {
+		.ref_count = -1,
+		.name = "data",
+		.signature = "a{sv}",
+		.annotations = NULL,
+	};
+	static GDBusArgInfo *arg_infos_notify[] = {
+		&arg_info_notify_in,
+		NULL,
+	};
+	static GDBusMethodInfo method_info_notify = {
+		.ref_count = -1,
+		.name = NM_DHCP_HELPER_SERVER_METHOD_NOTIFY,
+		.in_args = arg_infos_notify,
+		.out_args = NULL,
+		.annotations = NULL,
+	};
+	static GDBusMethodInfo *method_infos[] = {
+		&method_info_notify,
+		NULL,
+	};
+	static GDBusInterfaceInfo interface_info = {
+		.ref_count = -1,
+		.name = NM_DHCP_HELPER_SERVER_INTERFACE_NAME,
+		.methods = method_infos,
+		.signals = NULL,
+		.properties = NULL,
+		.annotations = NULL,
+	};
+
+	static GDBusInterfaceVTable interface_vtable = {
+		.method_call = _method_call,
+		.get_property = NULL,
+		.set_property = NULL,
+	};
+
+	return g_dbus_connection_register_object (connection,
+	                                          NM_DHCP_HELPER_SERVER_OBJECT_PATH,
+	                                          &interface_info,
+	                                          &interface_vtable,
+	                                          self,
+	                                          NULL,
+	                                          error);
 }
 
 static void
@@ -151,17 +212,20 @@ new_connection_cb (NMBusManager *mgr,
                    NMDhcpListener *self)
 {
 	NMDhcpListenerPrivate *priv = NM_DHCP_LISTENER_GET_PRIVATE (self);
-	guint id;
+	guint registration_id;
+	GError *error = NULL;
+
+	/* it is important to register the object during the new-connection signal,
+	 * as this avoids races with the connecting object. */
+	registration_id = _dbus_connection_register_object (self, connection, &error);
+	if (!registration_id) {
+		g_printerr ("failure to register %s for connection %p: %s",
+			NM_DHCP_HELPER_SERVER_OBJECT_PATH, connection, error->message);
+		g_error_free (error);
+		return;
+	}
 
-	id = g_dbus_connection_signal_subscribe (connection,
-	                                         NULL,
-	                                         NM_DHCP_CLIENT_DBUS_IFACE,
-	                                         "Event",
-	                                         NULL,
-	                                         NULL,
-	                                         G_DBUS_SIGNAL_FLAGS_NONE,
-	                                         handle_event, self, NULL);
-	g_hash_table_insert (priv->signal_handlers, connection, GUINT_TO_POINTER (id));
+	g_hash_table_insert (priv->signal_handlers, connection, GUINT_TO_POINTER (registration_id));
 }
 
 static void
@@ -174,7 +238,7 @@ dis_connection_cb (NMBusManager *mgr,
 
 	id = GPOINTER_TO_UINT (g_hash_table_lookup (priv->signal_handlers, connection));
 	if (id) {
-		g_dbus_connection_signal_unsubscribe (connection, id);
+		g_dbus_connection_unregister_object (connection, id);
 		g_hash_table_remove (priv->signal_handlers, connection);
 	}
 }
--- a/src/nm-bus-manager.c
+++ b/src/nm-bus-manager.c
@@ -221,7 +221,11 @@ private_server_new_connection (GDBusServ
 
 	_LOGD ("(%s) accepted connection %p on private socket", s->tag, conn);
 
-	/* Emit this for the manager */
+	/* Emit this for the manager.
+	 *
+	 * It is essential to do this from the "new-connection" signal handler, as
+	 * at that point no messages from the connection are yet processed
+	 * (which avoids races with registering objects). */
 	g_signal_emit (s->manager,
 	               signals[PRIVATE_CONNECTION_NEW],
 	               s->detail,
