From 39e1c65494d489b2cd9d63140c98d4fb4185003a Mon Sep 17 00:00:00 2001 From: Beniamino Galvani Date: Mon, 18 Dec 2017 17:25:10 +0100 Subject: [PATCH] settings: avoid assertion when deleting connections If a volatile connection is deleted by user when it was already being deleted internally because the device vanished, we may hit the following failed assertion: file src/settings/nm-settings-connection.c: line 2196 (nm_settings_connection_signal_remove): should not be reached The @removed flag keeps track of whether we already signaled the connection removal. Instead of throwing an assertion if we try to emit the signal again, just return without action because this can happen in the situation described above. While at it, remove the @allow_reuse argument from nm_settings_connection_signal_remove(): we should never emit the signal twice. Instead, we should reset the @removed flag when the connection is added. Fixes: a9384452ed61ca3f1c6e1db175f499307da9c388 https://bugzilla.redhat.com/show_bug.cgi?id=1506552 (cherry picked from commit 98ac0f404ed8103324b7d03cf04c24946f4543ff) --- src/settings/nm-settings-connection.c | 23 +++++++++++++------ src/settings/nm-settings-connection.h | 4 +++- src/settings/nm-settings.c | 2 ++ .../plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c | 4 ++-- src/settings/plugins/ifnet/nms-ifnet-plugin.c | 4 ++-- .../plugins/keyfile/nms-keyfile-plugin.c | 2 +- 6 files changed, 26 insertions(+), 13 deletions(-) diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c index b6e894049f..1a34207e79 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -793,7 +793,7 @@ nm_settings_connection_delete (NMSettingsConnection *self, /* Remove connection from seen-bssids database file */ remove_entry_from_db (self, "seen-bssids"); - nm_settings_connection_signal_remove (self, FALSE); + nm_settings_connection_signal_remove (self); return TRUE; } @@ -2187,15 +2187,24 @@ impl_settings_connection_clear_secrets (NMSettingsConnection *self, /*****************************************************************************/ void -nm_settings_connection_signal_remove (NMSettingsConnection *self, gboolean allow_reuse) +nm_settings_connection_added (NMSettingsConnection *self) { NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); - if (!allow_reuse) { - if (priv->removed) - g_return_if_reached (); - priv->removed = TRUE; - } + /* FIXME: we should always dispose connections that are removed + * and not reuse them, but currently plugins keep alive unmanaged + * (e.g. NM_CONTROLLED=no) connections. */ + priv->removed = FALSE; +} + +void +nm_settings_connection_signal_remove (NMSettingsConnection *self) +{ + NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self); + + if (priv->removed) + return; + priv->removed = TRUE; g_signal_emit_by_name (self, NM_SETTINGS_CONNECTION_REMOVED); } diff --git a/src/settings/nm-settings-connection.h b/src/settings/nm-settings-connection.h index fc8ad1de1a..899c48dab4 100644 --- a/src/settings/nm-settings-connection.h +++ b/src/settings/nm-settings-connection.h @@ -190,7 +190,9 @@ void nm_settings_connection_recheck_visibility (NMSettingsConnection *self); gboolean nm_settings_connection_check_permission (NMSettingsConnection *self, const char *permission); -void nm_settings_connection_signal_remove (NMSettingsConnection *self, gboolean allow_reuse); +void nm_settings_connection_added (NMSettingsConnection *self); + +void nm_settings_connection_signal_remove (NMSettingsConnection *self); gboolean nm_settings_connection_get_unsaved (NMSettingsConnection *self); diff --git a/src/settings/nm-settings.c b/src/settings/nm-settings.c index 21fdf9e067..d54305c137 100644 --- a/src/settings/nm-settings.c +++ b/src/settings/nm-settings.c @@ -1004,6 +1004,8 @@ claim_connection (NMSettings *self, NMSettingsConnection *connection) /* Exported D-Bus signal */ g_signal_emit (self, signals[NEW_CONNECTION], 0, connection); } + + nm_settings_connection_added (connection); } static gboolean diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c index 04e74bbda6..8d222d31c2 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-plugin.c @@ -161,7 +161,7 @@ remove_connection (SettingsPluginIfcfg *self, NMIfcfgConnection *connection) g_object_ref (connection); g_hash_table_remove (priv->connections, nm_connection_get_uuid (NM_CONNECTION (connection))); if (!unmanaged && !unrecognized) - nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (connection), FALSE); + nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (connection)); g_object_unref (connection); /* Emit changes _after_ removing the connection */ @@ -331,7 +331,7 @@ update_connection (SettingsPluginIfcfg *self, /* Unexport the connection by telling the settings service it's * been removed. */ - nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (connection_by_uuid), TRUE); + nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (connection_by_uuid)); /* Remove the path so that claim_connection() doesn't complain later when * interface gets managed and connection is re-added. */ nm_connection_set_path (NM_CONNECTION (connection_by_uuid), NULL); diff --git a/src/settings/plugins/ifnet/nms-ifnet-plugin.c b/src/settings/plugins/ifnet/nms-ifnet-plugin.c index 38d23f306b..6fc7981cbc 100644 --- a/src/settings/plugins/ifnet/nms-ifnet-plugin.c +++ b/src/settings/plugins/ifnet/nms-ifnet-plugin.c @@ -262,7 +262,7 @@ reload_connections (NMSettingsPlugin *config) NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS)) { nm_log_info (LOGD_SETTINGS, "Auto refreshing %s", conn_name); - nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (old), FALSE); + nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (old)); track_new_connection (self, new); if (is_managed_plugin () && is_managed (conn_name)) g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_CONNECTION_ADDED, new); @@ -305,7 +305,7 @@ reload_connections (NMSettingsPlugin *config) */ if ( nm_ifnet_connection_get_conn_name (NM_IFNET_CONNECTION (candidate)) && !g_hash_table_lookup (new_connections, uuid)) { - nm_settings_connection_signal_remove (candidate, FALSE); + nm_settings_connection_signal_remove (candidate); g_hash_table_iter_remove (&iter); } } diff --git a/src/settings/plugins/keyfile/nms-keyfile-plugin.c b/src/settings/plugins/keyfile/nms-keyfile-plugin.c index bc64b3ca49..19ebbea8fc 100644 --- a/src/settings/plugins/keyfile/nms-keyfile-plugin.c +++ b/src/settings/plugins/keyfile/nms-keyfile-plugin.c @@ -107,7 +107,7 @@ remove_connection (NMSKeyfilePlugin *self, NMSKeyfileConnection *connection) g_signal_handlers_disconnect_by_func (connection, connection_removed_cb, self); removed = g_hash_table_remove (NMS_KEYFILE_PLUGIN_GET_PRIVATE (self)->connections, nm_connection_get_uuid (NM_CONNECTION (connection))); - nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (connection), FALSE); + nm_settings_connection_signal_remove (NM_SETTINGS_CONNECTION (connection)); g_object_unref (connection); g_return_if_fail (removed);