libnm/client: don't reset properties when interface goes away

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
  ...

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,

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);
     }
 }

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)