From 98ac0f404ed8103324b7d03cf04c24946f4543ff 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 --- 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 c133b4152c..972d06f195 100644 --- a/src/settings/nm-settings-connection.c +++ b/src/settings/nm-settings-connection.c @@ -789,7 +789,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; } @@ -2183,15 +2183,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 ccd3708d51..e58183dc1a 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 dd179565a2..a4dfc4ebb1 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);