core/dbus: stop NMDBusManager and reject future method callsth/core-fixes-for-shutdown

During shutdown, we will need to still iterate the main loop
to do a coordinated shutdown. Currently we do not, and we just
exit, leaving a lot of objects hanging.

If we are going to fix that, we need during shutdown tell
NMDBusManager to reject all future operations.

Note that property getters and "GetManagerObjects" call is not
blocked. It continues to work.

Certainly for some operations, we want to allow them to be called even
during shutdown. However, these have to opt-in.

This also fixes an uglyness, where nm_dbus_manager_start() would
get the set-property-handler and the @manager as user-data. However,
NMDBusManager will always outlife NMManager, hence, after NMManager
is destroyed, the user-data would be a dangling pointer. Currently
that is not an issue, because
  - we always leak NMManager
  - we don't run the mainloop during shutdown

8 files changed, 52 insertions, 24 deletions

diff --git a/src/main.c b/src/main.c
index e8343208b7..c2f08f61b6 100644
--- a/src/main.c
+++ b/src/main.c
@@ -442,11 +442,6 @@ done:
 	 * it misses to update the state. */
 	nm_manager_write_device_state (manager);
 
-	/* FIXME(shutdown): we don't properly shut down on exit. That is a bug.
-	 * NMDBusObject have an assertion that they get unexported before disposing.
-	 * We need this workaround and disable the assertion during our leaky shutdown. */
-	nm_dbus_object_set_quitting ();
-
 	nm_manager_stop (manager);
 
 	nm_config_state_set (config, TRUE, TRUE);
diff --git a/src/nm-dbus-manager.c b/src/nm-dbus-manager.c
index 3e3691296f..62844a7848 100644
--- a/src/nm-dbus-manager.c
+++ b/src/nm-dbus-manager.c
@@ -83,6 +83,7 @@ typedef struct {
 	GDBusConnection *connection;
 	GDBusProxy *proxy;
 	guint objmgr_registration_id;
+	bool shutting_down:1;
 } NMDBusManagerPrivate;
 
 struct _NMDBusManager {
@@ -787,6 +788,8 @@ dbus_vtable_method_call (GDBusConnection *connection,
                          GDBusMethodInvocation *invocation,
                          gpointer user_data)
 {
+	NMDBusManager *self;
+	NMDBusManagerPrivate *priv;
 	RegistrationData *reg_data = user_data;
 	NMDBusObject *obj = reg_data->obj;
 	const NMDBusInterfaceInfoExtended *interface_info = _reg_data_get_interface_info (reg_data);
@@ -799,13 +802,14 @@ dbus_vtable_method_call (GDBusConnection *connection,
 	if (   !on_same_interface
 	    && nm_streq (interface_name, DBUS_INTERFACE_PROPERTIES)
 	    && nm_streq (method_name, "Set")) {
-		NMDBusManager *self = nm_dbus_object_get_manager (obj);
-		NMDBusManagerPrivate *priv = NM_DBUS_MANAGER_GET_PRIVATE (self);
 		const NMDBusPropertyInfoExtended *property_info = NULL;
 		const char *property_interface;
 		const char *property_name;
 		gs_unref_variant GVariant *value = NULL;
 
+		self = nm_dbus_object_get_manager (obj);
+		priv = NM_DBUS_MANAGER_GET_PRIVATE (self);
+
 		g_variant_get (parameters, "(&s&sv)", &property_interface, &property_name, &value);
 
 		nm_assert (nm_streq (property_interface, interface_info->parent.name));
@@ -850,6 +854,17 @@ dbus_vtable_method_call (GDBusConnection *connection,
 		return;
 	}
 
+	self = nm_dbus_object_get_manager (obj);
+	priv = NM_DBUS_MANAGER_GET_PRIVATE (self);
+	if (   priv->shutting_down
+	    && !method_info->allow_during_shutdown) {
+		g_dbus_method_invocation_return_error_literal (invocation,
+		                                               G_DBUS_ERROR,
+		                                               G_DBUS_ERROR_FAILED,
+		                                               "NetworkManager is exiting");
+		return;
+	}
+
 	method_info->handle (reg_data->obj,
 	                     interface_info,
 	                     method_info,
@@ -1563,6 +1578,27 @@ nm_dbus_manager_start (NMDBusManager *self,
 	return TRUE;
 }
 
+void
+nm_dbus_manager_stop (NMDBusManager *self)
+{
+	NMDBusManagerPrivate *priv = NM_DBUS_MANAGER_GET_PRIVATE (self);
+
+	priv->shutting_down = TRUE;
+
+	/* during shutdown we also clear the set-property-handler. It's no longer
+	 * possible to set a property, because doing so would require authorization,
+	 * which is async, which is just complicated to get right. No more property
+	 * setting from now on. */
+	priv->set_property_handler = NULL;
+	priv->set_property_handler_data = NULL;
+}
+
+gboolean
+nm_dbus_manager_is_stopping (NMDBusManager *self)
+{
+	return NM_DBUS_MANAGER_GET_PRIVATE (self)->shutting_down;
+}
+
 /*****************************************************************************/
 
 static void
diff --git a/src/nm-dbus-manager.h b/src/nm-dbus-manager.h
index 617f8a674c..277f8de2d4 100644
--- a/src/nm-dbus-manager.h
+++ b/src/nm-dbus-manager.h
@@ -53,6 +53,10 @@ gboolean nm_dbus_manager_start (NMDBusManager *self,
                                 NMDBusManagerSetPropertyHandler handler,
                                 gpointer handler_data);
 
+void nm_dbus_manager_stop (NMDBusManager *self);
+
+gboolean nm_dbus_manager_is_stopping (NMDBusManager *self);
+
 GDBusConnection *nm_dbus_manager_get_connection (NMDBusManager *self);
 
 NMDBusObject *nm_dbus_manager_lookup_object (NMDBusManager *self, const char *path);
diff --git a/src/nm-dbus-object.c b/src/nm-dbus-object.c
index 514fda4977..e92a8f5fa5 100644
--- a/src/nm-dbus-object.c
+++ b/src/nm-dbus-object.c
@@ -26,17 +26,6 @@
 
 /*****************************************************************************/
 
-static gboolean quitting = FALSE;
-
-void
-nm_dbus_object_set_quitting (void)
-{
-	nm_assert (!quitting);
-	quitting = TRUE;
-}
-
-/*****************************************************************************/
-
 enum {
 	EXPORTED_CHANGED,
 
@@ -291,7 +280,7 @@ dispose (GObject *object)
 	 * we are quitting, where many objects stick around until exit.
 	 */
 	if (self->internal.path) {
-		if (!quitting)
+		if (!nm_dbus_manager_is_stopping (nm_dbus_object_get_manager (self)))
 			g_warn_if_reached ();
 		nm_dbus_object_unexport (self);
 	}
diff --git a/src/nm-dbus-object.h b/src/nm-dbus-object.h
index 43613630bb..8b3ffdbded 100644
--- a/src/nm-dbus-object.h
+++ b/src/nm-dbus-object.h
@@ -28,10 +28,6 @@
 
 /*****************************************************************************/
 
-void nm_dbus_object_set_quitting (void);
-
-/*****************************************************************************/
-
 typedef struct {
 	const char *path;
 
diff --git a/src/nm-dbus-utils.h b/src/nm-dbus-utils.h
index e7e930e938..1f8e96c4c9 100644
--- a/src/nm-dbus-utils.h
+++ b/src/nm-dbus-utils.h
@@ -122,6 +122,7 @@ typedef struct _NMDBusMethodInfoExtended {
 	                const char *sender,
 	                GDBusMethodInvocation *invocation,
 	                GVariant *parameters);
+	bool allow_during_shutdown;
 } NMDBusMethodInfoExtended;
 
 #define NM_DEFINE_DBUS_METHOD_INFO_EXTENDED(parent_, ...) \
diff --git a/src/nm-iface-helper.c b/src/nm-iface-helper.c
index 601c72acac..58d766c124 100644
--- a/src/nm-iface-helper.c
+++ b/src/nm-iface-helper.c
@@ -635,6 +635,12 @@ nm_dbus_manager_get (void)
 	return NULL;
 }
 
+gboolean
+nm_dbus_manager_is_stopping (NMDBusManager *self)
+{
+	return FALSE;
+}
+
 void
 _nm_dbus_manager_obj_export (NMDBusObject *obj)
 {
diff --git a/src/nm-manager.c b/src/nm-manager.c
index fd341538a3..0ffc33f385 100644
--- a/src/nm-manager.c
+++ b/src/nm-manager.c
@@ -5918,7 +5918,6 @@ nm_manager_stop (NMManager *self)
 	NMDevice *device;
 
 	/* FIXME(shutdown): we don't do a proper shutdown yet:
-	 *  - need to tell NMDBusManager that all future D-Bus requests are rejected
 	 *  - need to ensure that all pending async operations are cancelled
 	 *    - e.g. operations in priv->async_op_lst_head
 	 *  - need to ensure that no more asynchronous requests are started,
@@ -5931,6 +5930,8 @@ nm_manager_stop (NMManager *self)
 	 *    - e.g. see comment at nm_auth_manager_force_shutdown()
 	 */
 
+	nm_dbus_manager_stop (nm_dbus_object_get_manager (NM_DBUS_OBJECT (self)));
+
 	while ((device = c_list_first_entry (&priv->devices_lst_head, NMDevice, devices_lst)))
 		remove_device (self, device, TRUE, TRUE);