fdo-mirrors/NetworkManager
1
0
Fork 0
mirror of https://gitlab.freedesktop.org/NetworkManager/NetworkManager.git synced 2025-12-29 06:50:10 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions

ovs: add error code for callbacks to indicate NM is quitting

Browse source 
When NM quits it destroys all singletons including NMOvsdb, which
invokes callbacks for every pending method call. In the shutdown,
extra care must be taken to not access objects that are already in a
inconsistent state; for example here, the callback changes the device
state, and this causes an access to data that has already been
cleared:

 #0  _g_log_abort (breakpoint=breakpoint@entry=1) at gmessages.c:554
 #1  g_logv (log_domain=0x5635653b6817 "NetworkManager", log_level=G_LOG_LEVEL_CRITICAL, format=<optimized out>, args=args@entry=0x7fffb4b2c1e0) at gmessages.c:1362
 #2  g_log (log_domain=log_domain@entry=0x5635653b6817 "NetworkManager", log_level=log_level@entry=G_LOG_LEVEL_CRITICAL, format=format@entry=0x7fbb3f58fa4a "%s: assertion '%s' failed") at gmessages.c:1403
 #3  g_return_if_fail_warning (log_domain=log_domain@entry=0x5635653b6817 "NetworkManager", pretty_function=pretty_function@entry=0x5635653b6b00 <__func__.34463> "nm_device_factory_manager_find_factory_for_connection", expression=expression@entry=0x5635653b6719 "factories_by_setting") at gmessages.c:2702
 #4  nm_device_factory_manager_find_factory_for_connection (connection=connection@entry=0x56356627e0e0) at src/devices/nm-device-factory.c:243
 #5  nm_manager_get_connection_iface (self=0x563566241080 [NMManager], connection=connection@entry=0x56356627e0e0, out_parent=out_parent@entry=0x0, error=error@entry=0x0) at src/nm-manager.c:1458
 #6  check_connection_compatible (self=<optimized out>, connection=0x56356627e0e0) at src/devices/nm-device.c:4679
 #7  check_connection_compatible (device=0x56356647b1b0 [NMDeviceOvsInterface], connection=0x56356627e0e0) at src/devices/ovs/nm-device-ovs-interface.c:95
 #8  _nm_device_check_connection_available (self=0x56356647b1b0 [NMDeviceOvsInterface], connection=0x56356627e0e0, flags=NM_DEVICE_CHECK_CON_AVAILABLE_NONE, specific_object=0x0) at src/devices/nm-device.c:12102
 #9  nm_device_check_connection_available (self=self@entry=0x56356647b1b0 [NMDeviceOvsInterface], connection=0x56356627e0e0, flags=flags@entry=NM_DEVICE_CHECK_CON_AVAILABLE_NONE, specific_object=specific_object@entry=0x0) at src/devices/nm-device.c:12131
 #10 nm_device_recheck_available_connections (self=self@entry=0x56356647b1b0 [NMDeviceOvsInterface]) at src/devices/nm-device.c:12238
 #11 _set_state_full (self=self@entry=0x56356647b1b0 [NMDeviceOvsInterface], state=state@entry=NM_DEVICE_STATE_FAILED, reason=reason@entry=NM_DEVICE_STATE_REASON_OVSDB_FAILED, quitting=quitting@entry=0) at src/devices/nm-device.c:13065
 #12 nm_device_state_changed (self=self@entry=0x56356647b1b0 [NMDeviceOvsInterface], state=state@entry=NM_DEVICE_STATE_FAILED, reason=reason@entry=NM_DEVICE_STATE_REASON_OVSDB_FAILED) at src/devices/nm-device.c:13328
 #13 del_iface_cb (error=<optimized out>, user_data=0x56356647b1b0) at src/devices/ovs/nm-device-ovs-port.c:160
 #14 _transact_cb (self=self@entry=0x5635662b9ba0 [NMOvsdb], result=result@entry=0x0, error=0x563566259a10, user_data=user_data@entry=0x5635662ff320) at src/devices/ovs/nm-ovsdb.c:1449
 #15 ovsdb_disconnect (self=self@entry=0x5635662b9ba0 [NMOvsdb]) at src/devices/ovs/nm-ovsdb.c:1331
 #16 dispose (object=0x5635662b9ba0 [NMOvsdb]) at src/devices/ovs/nm-ovsdb.c:1558
 #17 g_object_unref (_object=0x5635662b9ba0) at gobject.c:3293
 #18 _nm_singleton_instance_destroy () at src/nm-core-utils.c:138
 #19 _dl_fini () at dl-fini.c:253
 #20 __run_exit_handlers (status=status@entry=0, listp=0x7fbb3e1ad6c8 <__exit_funcs>, run_list_atexit=run_list_atexit@entry=true) at exit.c:77
 #21 __GI_exit (status=status@entry=0) at exit.c:99
 #22 main (argc=1, argv=0x7fffb4b2cc38) at src/main.c:468

Add a new error code to indicate to callbacks that we are quitting and
no further action must be taken. This is preferable to having
additional references because it allows us to free the resources owned
by callbacks immediately, while references can easily create loops.

https://bugzilla.redhat.com/show_bug.cgi?id=1543871
This commit is contained in:
Beniamino Galvani 2018-02-15 17:58:28 +01:00
parent b9e22ece2d
commit cf79615169
2 changed files with 17 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

6
src/devices/ovs/nm-device-ovs-port.c
View file

@ -114,7 +114,8 @@ add_iface_cb (GError *error, gpointer user_data)
{
NMDevice *slave = user_data;
if (error) {
if ( error
&& !g_error_matches (error, NM_UTILS_ERROR, NM_UTILS_ERROR_CANCELLED_DISPOSING)) {
nm_log_warn (LOGD_DEVICE, "device %s could not be added to a ovs port: %s",
nm_device_get_iface (slave), error->message);
nm_device_state_changed (slave,
@ -153,7 +154,8 @@ del_iface_cb (GError *error, gpointer user_data)
{
NMDevice *slave = user_data;
if (error) {
if ( error
&& !g_error_matches (error, NM_UTILS_ERROR, NM_UTILS_ERROR_CANCELLED_DISPOSING)) {
nm_log_warn (LOGD_DEVICE, "device %s could not be removed from a ovs port: %s",
nm_device_get_iface (slave), error->message);
nm_device_state_changed (slave,

26
src/devices/ovs/nm-ovsdb.c
View file

@ -102,7 +102,7 @@ NM_DEFINE_SINGLETON_GETTER (NMOvsdb, nm_ovsdb_get, NM_TYPE_OVSDB);
/*****************************************************************************/
static void ovsdb_try_connect (NMOvsdb *self);
static void ovsdb_disconnect (NMOvsdb *self);
static void ovsdb_disconnect (NMOvsdb *self, gboolean is_disposing);
static void ovsdb_read (NMOvsdb *self);
static void ovsdb_write (NMOvsdb *self);
static void ovsdb_next_command (NMOvsdb *self);
@ -1086,7 +1086,7 @@ ovsdb_got_msg (NMOvsdb *self, json_t *msg)
"result", &result,
"error", &error) == -1) {
_LOGW ("couldn't grok the message: %s", json_error.text);
ovsdb_disconnect (self);
ovsdb_disconnect (self, FALSE);
return;
}
@ -1097,7 +1097,7 @@ ovsdb_got_msg (NMOvsdb *self, json_t *msg)
/* It's a method call! */
if (!params) {
_LOGW ("a method call with no params: '%s'", method);
ovsdb_disconnect (self);
ovsdb_disconnect (self, FALSE);
return;
}
@ -1117,13 +1117,13 @@ ovsdb_got_msg (NMOvsdb *self, json_t *msg)
/* This is a response to a method call. */
if (!priv->calls->len) {
_LOGE ("there are no queued calls expecting response %" G_GUINT64_FORMAT, id);
ovsdb_disconnect (self);
ovsdb_disconnect (self, FALSE);
return;
}
call = &g_array_index (priv->calls, OvsdbMethodCall, 0);
if (call->id != id) {
_LOGE ("expected a response to call %" G_GUINT64_FORMAT ", not %" G_GUINT64_FORMAT, call->id, id);
ovsdb_disconnect (self);
ovsdb_disconnect (self, FALSE);
return;
}
/* Cool, we found a corresponsing call. Finish it. */
@ -1202,7 +1202,7 @@ ovsdb_read_cb (GObject *source_object, GAsyncResult *res, gpointer user_data)
if (size == -1) {
_LOGW ("short read from ovsdb: %s", error->message);
g_clear_error (&error);
ovsdb_disconnect (self);
ovsdb_disconnect (self, FALSE);
return;
}
@ -1250,7 +1250,7 @@ ovsdb_write_cb (GObject *source_object, GAsyncResult *res, gpointer user_data)
if (size == -1) {
_LOGW ("short write to ovsdb: %s", error->message);
g_clear_error (&error);
ovsdb_disconnect (self);
ovsdb_disconnect (self, FALSE);
return;
}
@ -1293,7 +1293,7 @@ ovsdb_write (NMOvsdb *self)
* puts us back in sync.
*/
static void
ovsdb_disconnect (NMOvsdb *self)
ovsdb_disconnect (NMOvsdb *self, gboolean is_disposing)
{
NMOvsdbPrivate *priv = NM_OVSDB_GET_PRIVATE (self);
OvsdbMethodCall *call;
@ -1306,7 +1306,7 @@ ovsdb_disconnect (NMOvsdb *self)
while (priv->calls->len) {
error = NULL;
call = &g_array_index (priv->calls, OvsdbMethodCall, priv->calls->len - 1);
g_set_error_literal (&error, G_IO_ERROR, G_IO_ERROR_CANCELLED, "Cancelled");
nm_utils_error_set_cancelled (&error, is_disposing, "NMOvsdb");
callback = call->callback;
user_data = call->user_data;
@ -1326,9 +1326,9 @@ static void
_monitor_bridges_cb (NMOvsdb *self, json_t *result, GError *error, gpointer user_data)
{
if (error) {
if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) {
if (!nm_utils_error_is_cancelled (error, TRUE)) {
_LOGI ("%s", error->message);
ovsdb_disconnect (self);
ovsdb_disconnect (self, FALSE);
}
g_clear_error (&error);
@ -1354,7 +1354,7 @@ _client_connect_cb (GObject *source_object, GAsyncResult *res, gpointer user_dat
if (!g_error_matches (error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
_LOGI ("%s", error->message);
ovsdb_disconnect (self);
ovsdb_disconnect (self, FALSE);
g_clear_error (&error);
return;
}
@ -1538,7 +1538,7 @@ dispose (GObject *object)
NMOvsdb *self = NM_OVSDB (object);
NMOvsdbPrivate *priv = NM_OVSDB_GET_PRIVATE (self);
ovsdb_disconnect (self);
ovsdb_disconnect (self, TRUE);
g_string_free (priv->input, TRUE);
priv->input = NULL;