openkylin
/
libvirt
mirror of https://gitee.com/openkylin/libvirt.git
9
0
Fork 0
Code Issues Projects Releases Wiki Activity

util: avoid crash due to race in glib event loop code 

There is a fairly long standing race condition bug in glib which can hit
if you call g_source_destroy or g_source_unref from a non-main thread:

  https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1358

Unfortunately it is really common for libvirt to call g_source_destroy
from a non-main thread. This glib bug is the cause of non-determinstic
crashes in eventtest, and probably in libvirtd too.

To work around the problem we need to ensure that we never release
the last reference on a GSource from a non-main thread. The previous
patch replaced our use of g_source_destroy with a pair of
g_source_remove and g_source_unref. We can now delay the g_source_unref
call by using a idle callback to invoke it from the main thread which
avoids the race condition.

Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2020-07-28 16:52:47 +01:00
parent da0a182708
commit 0db4743645
1 changed files with 24 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
src/util/vireventglib.c
View File

@ -185,6 +185,24 @@ virEventGLibHandleFind(int watch)
return NULL;
}
/*
* If the last refernce to a GSource is released in a non-main
* thread we're exposed to a race condition that causes a
* crash:
* https://gitlab.gnome.org/GNOME/glib/-/merge_requests/1358
* Thus we're using an idle func to release our ref
*/
static gboolean
virEventGLibSourceUnrefIdle(gpointer data)
{
GSource *src = data;
g_source_unref(src);
return FALSE;
}
static void
virEventGLibHandleUpdate(int watch,
int events)
@ -213,7 +231,7 @@ virEventGLibHandleUpdate(int watch,
if (data->source != NULL) {
VIR_DEBUG("Removed old handle source=%p", data->source);
g_source_destroy(data->source);
g_source_unref(data->source);
g_idle_add(virEventGLibSourceUnrefIdle, data->source);
}
data->source = virEventGLibAddSocketWatch(
@ -227,7 +245,7 @@ virEventGLibHandleUpdate(int watch,
VIR_DEBUG("Removed old handle source=%p", data->source);
g_source_destroy(data->source);
g_source_unref(data->source);
g_idle_add(virEventGLibSourceUnrefIdle, data->source);
data->source = NULL;
data->events = 0;
}
@ -276,7 +294,7 @@ virEventGLibHandleRemove(int watch)
if (data->source != NULL) {
g_source_destroy(data->source);
g_source_unref(data->source);
g_idle_add(virEventGLibSourceUnrefIdle, data->source);
data->source = NULL;
data->events = 0;
}
@ -409,7 +427,7 @@ virEventGLibTimeoutUpdate(int timer,
if (interval >= 0) {
if (data->source != NULL) {
g_source_destroy(data->source);
g_source_unref(data->source);
g_idle_add(virEventGLibSourceUnrefIdle, data->source);
}
data->interval = interval;
@ -419,7 +437,7 @@ virEventGLibTimeoutUpdate(int timer,
goto cleanup;
g_source_destroy(data->source);
g_source_unref(data->source);
g_idle_add(virEventGLibSourceUnrefIdle, data->source);
data->source = NULL;
}
@ -468,7 +486,7 @@ virEventGLibTimeoutRemove(int timer)
if (data->source != NULL) {
g_source_destroy(data->source);
g_source_unref(data->source);
g_idle_add(virEventGLibSourceUnrefIdle, data->source);
data->source = NULL;
}