libnm/client: don't reset properties when interface goes awaylr/object-removal

In case the D-Bus interfaces start dropping off (typically all off them go
one by one when the object is being deleted), don't reset all the properties.

In particular, keep most properties around, only tear down "o" and "ao",
so that the object dependencies get torn down, but we still get enough
properties around to identify what the dead object was its heyday.

One example of where this is not good is when the device-removed signal
is emmitted, the device no longer has the ifname:

  $ nmcli monitor
  <quit NetworkManager>
  (null): device removed
  (null): device removed
  ...

3 files changed, 23 insertions, 31 deletions

diff --git a/src/libnm-client-impl/nm-client.c b/src/libnm-client-impl/nm-client.c
index 0e7d957c80..7935b4bb81 100644
--- a/src/libnm-client-impl/nm-client.c
+++ b/src/libnm-client-impl/nm-client.c
@@ -2630,7 +2630,16 @@ _obj_handle_dbus_iface_changes(NMClient            *self,
 
     if (is_removed) {
         for (i_prop = 0; i_prop < db_iface_data->dbus_iface.meta->n_dbus_properties; i_prop++) {
-            _obj_handle_dbus_prop_changes(self, dbobj, db_iface_data, i_prop, NULL);
+            const GVariantType *dbus_type =
+                db_iface_data->dbus_iface.meta->dbus_properties[i_prop].dbus_type;
+
+            /* Unset properties that can potentially contain objects, to release them,
+             * but keep the rest around, because it might still make sense to know what
+             * they were (e.g. when a device has been removed we'd like know what interface
+             * name it had, or keep the state to avoid spurious state change into UNKNOWN). */
+            if (g_variant_type_is_array(dbus_type)
+                || g_variant_type_equal(dbus_type, G_VARIANT_TYPE_OBJECT_PATH))
+                _obj_handle_dbus_prop_changes(self, dbobj, db_iface_data, i_prop, NULL);
         }
     } else {
         while ((db_prop_data = c_list_first_entry(&db_iface_data->changed_prop_lst_head,
diff --git a/src/libnm-client-impl/tests/test-nm-client.c b/src/libnm-client-impl/tests/test-nm-client.c
index c97d9e263f..5de7c6a5b6 100644
--- a/src/libnm-client-impl/tests/test-nm-client.c
+++ b/src/libnm-client-impl/tests/test-nm-client.c
@@ -498,7 +498,8 @@ nm_running_changed(GObject *client, GParamSpec *pspec, gpointer user_data)
 {
     int *running_changed = user_data;
 
-    (*running_changed)++;
+    if (running_changed)
+        (*running_changed)++;
     g_main_loop_quit(gl.loop);
 }
 
@@ -865,25 +866,12 @@ test_activate_virtual(void)
 }
 
 static void
-_dev_eth0_1_state_changed_cb(NMDevice           *device,
-                             NMDeviceState       new_state,
-                             NMDeviceState       old_state,
-                             NMDeviceStateReason reason,
-                             int                *p_count_call)
+_client_dev_removed(NMClient *client, NMDevice *device, int *p_count_call)
 {
     const GPtrArray *arr;
 
-    g_assert(p_count_call);
-    g_assert_cmpint(*p_count_call, ==, 0);
-
     (*p_count_call)++;
 
-    g_assert(NM_IS_DEVICE_VLAN(device));
-
-    g_assert_cmpint(old_state, >=, NM_DEVICE_STATE_PREPARE);
-    g_assert_cmpint(old_state, <=, NM_DEVICE_STATE_ACTIVATED);
-    g_assert_cmpint(new_state, ==, NM_DEVICE_STATE_UNKNOWN);
-
     arr = nm_device_get_available_connections(device);
     g_assert(arr);
     g_assert_cmpint(arr->len, ==, 0);
@@ -899,7 +887,6 @@ test_activate_virtual_teardown(gconstpointer user_data)
     NMDevice                                      *dev_eth0_1;
     NMActiveConnection                            *ac;
     const GPtrArray                               *arr;
-    gulong                                         sig_id;
     int                                            call_count       = 0;
     gboolean                                       take_ref         = nmtst_get_rand_bool();
     gboolean                                       teardown_service = GPOINTER_TO_INT(user_data);
@@ -928,20 +915,21 @@ test_activate_virtual_teardown(gconstpointer user_data)
     ac = nm_device_get_active_connection(dev_eth0_1);
     g_assert(NM_IS_ACTIVE_CONNECTION(ac));
 
-    sig_id = g_signal_connect(dev_eth0_1,
-                              "state-changed",
-                              G_CALLBACK(_dev_eth0_1_state_changed_cb),
-                              &call_count);
+    g_signal_connect(client, "device-removed", G_CALLBACK(_client_dev_removed), &call_count);
 
     if (teardown_service) {
-        nmtstc_service_cleanup(g_steal_pointer(&sinfo));
-        nmtst_main_loop_run(gl.loop, 50);
+        g_signal_connect(client,
+                         "notify::" NM_CLIENT_NM_RUNNING,
+                         G_CALLBACK(nm_running_changed),
+                         NULL);
+        nm_clear_pointer(&sinfo, nmtstc_service_cleanup);
+        nmtst_main_loop_run(gl.loop, 1000);
+        g_assert_cmpint(call_count, ==, 2);
     } else {
         g_clear_object(&client);
+        g_assert_cmpint(call_count, ==, 0);
     }
 
-    g_assert_cmpint(call_count, ==, 1);
-
     if (take_ref) {
         arr = nm_device_get_available_connections(dev_eth0_1);
         g_assert(arr);
@@ -949,8 +937,6 @@ test_activate_virtual_teardown(gconstpointer user_data)
 
         g_assert(!nm_device_get_active_connection(dev_eth0_1));
 
-        nm_clear_g_signal_handler(dev_eth0_1, &sig_id);
-
         g_object_unref(dev_eth0_1);
     }
 }
diff --git a/src/tests/client/test-client.py b/src/tests/client/test-client.py
index 4ee64ecb86..f018fbf34a 100755
--- a/src/tests/client/test-client.py
+++ b/src/tests/client/test-client.py
@@ -1913,11 +1913,8 @@ class TestNmcli(NmTestBase):
         nmc = start_mon()
         self.srv.shutdown()
         self.srv = None
-        nmc.expect("\(null\): device removed")
+        nmc.expect("eth0: device removed")
         nmc.expect("con-1: connection profile removed")
-        nmc.expect("Hostname set to '\(null\)'")
-        nmc.expect("Networkmanager is now in the 'unknown' state")
-        nmc.expect("Connectivity is now 'unknown'")
         nmc.expect("NetworkManager is stopped")
         end_mon(nmc)