Commit 98ac0f40 authored by Beniamino Galvani's avatar Beniamino Galvani

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: a9384452

https://bugzilla.redhat.com/show_bug.cgi?id=1506552
parent 2499d3bd
...@@ -789,7 +789,7 @@ nm_settings_connection_delete (NMSettingsConnection *self, ...@@ -789,7 +789,7 @@ nm_settings_connection_delete (NMSettingsConnection *self,
/* Remove connection from seen-bssids database file */ /* Remove connection from seen-bssids database file */
remove_entry_from_db (self, "seen-bssids"); remove_entry_from_db (self, "seen-bssids");
nm_settings_connection_signal_remove (self, FALSE); nm_settings_connection_signal_remove (self);
return TRUE; return TRUE;
} }
...@@ -2183,15 +2183,24 @@ impl_settings_connection_clear_secrets (NMSettingsConnection *self, ...@@ -2183,15 +2183,24 @@ impl_settings_connection_clear_secrets (NMSettingsConnection *self,
/*****************************************************************************/ /*****************************************************************************/
void 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); NMSettingsConnectionPrivate *priv = NM_SETTINGS_CONNECTION_GET_PRIVATE (self);
if (!allow_reuse) { /* FIXME: we should always dispose connections that are removed
if (priv->removed) * and not reuse them, but currently plugins keep alive unmanaged
g_return_if_reached (); * (e.g. NM_CONTROLLED=no) connections. */
priv->removed = TRUE; 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); g_signal_emit_by_name (self, NM_SETTINGS_CONNECTION_REMOVED);
} }
......
...@@ -190,7 +190,9 @@ void nm_settings_connection_recheck_visibility (NMSettingsConnection *self); ...@@ -190,7 +190,9 @@ void nm_settings_connection_recheck_visibility (NMSettingsConnection *self);
gboolean nm_settings_connection_check_permission (NMSettingsConnection *self, gboolean nm_settings_connection_check_permission (NMSettingsConnection *self,
const char *permission); 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); gboolean nm_settings_connection_get_unsaved (NMSettingsConnection *self);
......
...@@ -1004,6 +1004,8 @@ claim_connection (NMSettings *self, NMSettingsConnection *connection) ...@@ -1004,6 +1004,8 @@ claim_connection (NMSettings *self, NMSettingsConnection *connection)
/* Exported D-Bus signal */ /* Exported D-Bus signal */
g_signal_emit (self, signals[NEW_CONNECTION], 0, connection); g_signal_emit (self, signals[NEW_CONNECTION], 0, connection);
} }
nm_settings_connection_added (connection);
} }
static gboolean static gboolean
......
...@@ -161,7 +161,7 @@ remove_connection (SettingsPluginIfcfg *self, NMIfcfgConnection *connection) ...@@ -161,7 +161,7 @@ remove_connection (SettingsPluginIfcfg *self, NMIfcfgConnection *connection)
g_object_ref (connection); g_object_ref (connection);
g_hash_table_remove (priv->connections, nm_connection_get_uuid (NM_CONNECTION (connection))); g_hash_table_remove (priv->connections, nm_connection_get_uuid (NM_CONNECTION (connection)));
if (!unmanaged && !unrecognized) 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); g_object_unref (connection);
/* Emit changes _after_ removing the connection */ /* Emit changes _after_ removing the connection */
...@@ -331,7 +331,7 @@ update_connection (SettingsPluginIfcfg *self, ...@@ -331,7 +331,7 @@ update_connection (SettingsPluginIfcfg *self,
/* Unexport the connection by telling the settings service it's /* Unexport the connection by telling the settings service it's
* been removed. * 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 /* Remove the path so that claim_connection() doesn't complain later when
* interface gets managed and connection is re-added. */ * interface gets managed and connection is re-added. */
nm_connection_set_path (NM_CONNECTION (connection_by_uuid), NULL); nm_connection_set_path (NM_CONNECTION (connection_by_uuid), NULL);
......
...@@ -262,7 +262,7 @@ reload_connections (NMSettingsPlugin *config) ...@@ -262,7 +262,7 @@ reload_connections (NMSettingsPlugin *config)
NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS)) { NM_SETTING_COMPARE_FLAG_IGNORE_NOT_SAVED_SECRETS)) {
nm_log_info (LOGD_SETTINGS, "Auto refreshing %s", conn_name); 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); track_new_connection (self, new);
if (is_managed_plugin () && is_managed (conn_name)) if (is_managed_plugin () && is_managed (conn_name))
g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_CONNECTION_ADDED, new); g_signal_emit_by_name (self, NM_SETTINGS_PLUGIN_CONNECTION_ADDED, new);
...@@ -305,7 +305,7 @@ reload_connections (NMSettingsPlugin *config) ...@@ -305,7 +305,7 @@ reload_connections (NMSettingsPlugin *config)
*/ */
if ( nm_ifnet_connection_get_conn_name (NM_IFNET_CONNECTION (candidate)) if ( nm_ifnet_connection_get_conn_name (NM_IFNET_CONNECTION (candidate))
&& !g_hash_table_lookup (new_connections, uuid)) { && !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); g_hash_table_iter_remove (&iter);
} }
} }
......
...@@ -107,7 +107,7 @@ remove_connection (NMSKeyfilePlugin *self, NMSKeyfileConnection *connection) ...@@ -107,7 +107,7 @@ remove_connection (NMSKeyfilePlugin *self, NMSKeyfileConnection *connection)
g_signal_handlers_disconnect_by_func (connection, connection_removed_cb, self); g_signal_handlers_disconnect_by_func (connection, connection_removed_cb, self);
removed = g_hash_table_remove (NMS_KEYFILE_PLUGIN_GET_PRIVATE (self)->connections, removed = g_hash_table_remove (NMS_KEYFILE_PLUGIN_GET_PRIVATE (self)->connections,
nm_connection_get_uuid (NM_CONNECTION (connection))); 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_object_unref (connection);
g_return_if_fail (removed); g_return_if_fail (removed);
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment