External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-01-22 04:25:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix incorrect uses of g_clear_pointer() introduced in 8.1.0

Browse Source 
This is a partial revert of 87a43a907f0ad4897a28ad7c216bc70f37270b93

The change to use g_clear_pointer() in more places was accidentally
applied to cases involving vir_g_source_unref().

In some cases, the ordering of g_source_destroy() and
vir_g_source_unref() was reversed, which resulted in the source being
marked as destroyed, after it is already unreferenced. This
use-after-free case might work in many cases, but with versions of
glib older than 2.64.0 it may defer unref to run within the main
thread to avoid a race condition, which creates a large distance
between the g_source_unref() and g_source_destroy().

In some cases, the call to vir_g_source_unref() was replaced with a
second call to g_source_destroy(), leading to a memory leak or worse.

In our experience, the symptoms were that use of libvirt-python became
slower over time, with OpenStack nova-compute initially taking around
one second to periodically query the host PCI devices, and within an
hour it was taking over a minute to complete the same operation, until
it is was eventually running this query back-to-back, resulting in the
nova-compute process consuming 100% of one CPU thread, losing its
RabbitMQ connection frequently, and showing up as down to the control
plane.

Signed-off-by: Mark Mielke <mark.mielke@gmail.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Signed-off-by: Ján Tomko <jtomko@redhat.com>
This commit is contained in:
Mark Mielke 2022-06-12 15:16:50 -04:00 committed by Ján Tomko
parent f5c5b16d5d
commit 31b5ad06e3
3 changed files with 12 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
src/qemu/qemu_agent.c
View File

@ -452,8 +452,9 @@ static void
qemuAgentUnregister(qemuAgent *agent)
{
if (agent->watch) {
g_source_destroy(agent->watch);
vir_g_source_unref(agent->watch, agent->context);
g_clear_pointer(&agent->watch, g_source_destroy);
agent->watch = NULL;
}
}

3
src/qemu/qemu_monitor.c
View File

@ -794,8 +794,9 @@ void
qemuMonitorUnregister(qemuMonitor *mon)
{
if (mon->watch) {
g_source_destroy(mon->watch);
vir_g_source_unref(mon->watch, mon->context);
g_clear_pointer(&mon->watch, g_source_destroy);
mon->watch = NULL;
}
}

12
src/util/vireventglib.c
View File

@ -228,7 +228,8 @@ virEventGLibHandleUpdate(int watch,
VIR_DEBUG("Removed old handle source=%p", data->source);
g_source_destroy(data->source);
g_clear_pointer(&data->source, g_source_destroy);
vir_g_source_unref(data->source, NULL);
data->source = NULL;
data->events = 0;
}
@ -275,8 +276,9 @@ virEventGLibHandleRemove(int watch)
data, watch, data->fd);
if (data->source != NULL) {
g_source_destroy(data->source);
vir_g_source_unref(data->source, NULL);
g_clear_pointer(&data->source, g_source_destroy);
data->source = NULL;
data->events = 0;
}
@ -417,8 +419,9 @@ virEventGLibTimeoutUpdate(int timer,
if (data->source == NULL)
goto cleanup;
g_source_destroy(data->source);
vir_g_source_unref(data->source, NULL);
g_clear_pointer(&data->source, g_source_destroy);
data->source = NULL;
}
cleanup:
@ -465,8 +468,9 @@ virEventGLibTimeoutRemove(int timer)
data, timer);
if (data->source != NULL) {
g_source_destroy(data->source);
vir_g_source_unref(data->source, NULL);
g_clear_pointer(&data->source, g_source_destroy);
data->source = NULL;
}
/* since the actual timeout deletion is done asynchronously, a timeoutUpdate call may