[PATCH 0/2] ch: Two simple fixes
While working on emitting more events from the CH driver and playing with cloud-hypervisor itself, I've noticed one memleak and one forgotten call to unlock. Michal Prívozník (2): ch: Avoid memory leak in virCHProcessEvents() ch: Unlock domain in virCHEventStopProcess() on all exit paths src/ch/ch_events.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) -- 2.49.1
From: Michal Privoznik <mprivozn@redhat.com> The aim of virCHProcessEvents() is to read data (in JSON format) from CH monitor and then process them. To parse incoming data virJSONValueFromString() is used. But the corresponding virJSONValue is freed only when processing of an even succeeds. If processing an event fails, then the memory is not freed leading to a memory leak. 334 (24 direct, 310 indirect) bytes in 1 blocks are definitely lost in loss record 1,975 of 2,040 at 0x4919EF3: calloc (vg_replace_malloc.c:1675) by 0x4FEB249: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.8400.3) by 0x4A66162: virJSONValueNewObject (virjson.c:533) by 0x4A67E74: virJSONValueFromJsonC (virjson.c:1413) by 0x4A681A5: virJSONValueFromString (virjson.c:1484) by 0xB8CD9D7: virCHProcessEvents (ch_events.c:179) by 0xB8CDCDC: virCHReadProcessEvents (ch_events.c:251) by 0xB8CDEBB: virCHEventHandlerLoop (ch_events.c:284) by 0x4AC1EB4: virThreadHelper (virthread.c:256) by 0x5441DE3: start_thread (in /usr/lib64/libc.so.6) by 0x54C25F3: clone (in /usr/lib64/libc.so.6) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/ch/ch_events.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c index be572dfde3..25c7ecf90a 100644 --- a/src/ch/ch_events.c +++ b/src/ch/ch_events.c @@ -152,7 +152,6 @@ virCHProcessEvents(virCHMonitor *mon) virDomainObj *vm = mon->vm; char *buf = mon->event_buffer.buffer; ssize_t sz = mon->event_buffer.buf_fill_sz; - virJSONValue *obj = NULL; int blocks = 0; size_t i = 0; char *json_start; @@ -168,6 +167,8 @@ virCHProcessEvents(virCHMonitor *mon) } else if (buf[i] == '}' && blocks > 0) { blocks--; if (blocks == 0) { + g_autoptr(virJSONValue) obj = NULL; + /* valid json document */ end_index = i; @@ -182,7 +183,6 @@ virCHProcessEvents(virCHMonitor *mon) vm->def->name, json_start); return -1; } - virJSONValueFree(obj); } else { VIR_ERROR(_("%1$s: Invalid JSON event doc: %2$s"), vm->def->name, json_start); -- 2.49.1
From: Michal Privoznik <mprivozn@redhat.com> The aim of virCHEventStopProcess() is to clean up after stopped domain by calling virCHProcessStop(). But in order to do that it needs to acquire a job and in order to do that it needs to lock the domain object. Well, the object is not unlocked in all exit paths, i.e. when job acquiring fails the domain object is left locked. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/ch/ch_events.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/ch/ch_events.c b/src/ch/ch_events.c index 25c7ecf90a..cd2f92a493 100644 --- a/src/ch/ch_events.c +++ b/src/ch/ch_events.c @@ -55,13 +55,12 @@ virCHEventStopProcess(virDomainObj *vm, virDomainShutoffReason reason) { virCHDriver *driver = CH_DOMAIN_PRIVATE(vm)->driver; + VIR_LOCK_GUARD lock = virObjectLockGuard(vm); - virObjectLock(vm); if (virDomainObjBeginJob(vm, VIR_JOB_DESTROY)) return -1; virCHProcessStop(driver, vm, reason, VIR_CH_PROCESS_STOP_FORCE); virDomainObjEndJob(vm); - virObjectUnlock(vm); return 0; } -- 2.49.1
On a Thursday in 2025, Michal Privoznik via Devel wrote:
While working on emitting more events from the CH driver and playing with cloud-hypervisor itself, I've noticed one memleak and one forgotten call to unlock.
Michal Prívozník (2): ch: Avoid memory leak in virCHProcessEvents() ch: Unlock domain in virCHEventStopProcess() on all exit paths
src/ch/ch_events.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (2)
-
Ján Tomko -
Michal Privoznik