[PATCH] virtlogd: solve some memory leaks
We used asan to find some memory leaks in virtlogd. In the virThreadPoolFree function, When job->data is of type virNetServerJobPtr, the following memory leak problem exists. 1. job->data is not released Direct leak of 24 byte(s) in 1 object(s) allocated from: #0 0x7f14ab932560 in calloc (/usr/local/gcc-6-4/lib64/libasan.so.3+0xc7560) ??:? #1 0x55ab07088853 in virAlloc (/usr/sbin/virtlogd+0x31853) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/viralloc.c:144 #2 0x55ab0707a515 (/usr/sbin/virtlogd+0x23515) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserver.c:209 #3 0x55ab07076d87 (/usr/sbin/virtlogd+0x1fd87) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:1374 #4 0x55ab070770e2 (/usr/sbin/virtlogd+0x200e2) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:1563 #5 0x55ab0709c67f in virEventPollRunOnce (/usr/sbin/virtlogd+0x4567f) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/vireventpoll.c:508 #6 0x55ab0709ad30 in virEventRunDefaultImpl (/usr/sbin/virtlogd+0x43d30) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/virevent.c:314 #7 0x55ab07079f6c in virNetDaemonRun (/usr/sbin/virtlogd+0x22f6c) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetdaemon.c:847 #8 0x55ab070714db in main (/usr/sbin/virtlogd+0x1a4db) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/logging/log_daemon.c:1162 #9 0x7f14aa3c2c56 in __libc_start_main (/usr/lib64/libc.so.6+0x25c56) ??:? #10 0x55ab07072639 in _start (/usr/sbin/virtlogd+0x1b639) ??:? 2. job->data->msg->buffer was not released Indirect leak of 152 byte(s) in 1 object(s) allocated from: #0 0x7f14ab932750 in realloc (/usr/local/gcc-6-4/lib64/libasan.so.3+0xc7750) ??:? #1 0x55ab0708894d in virReallocN (/usr/sbin/virtlogd+0x3194d) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/viralloc.c:245 #2 0x55ab0707d830 in virNetMessageDecodeLength (/usr/sbin/virtlogd+0x26830) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetmessage.c:157 #3 0x55ab07076b36 (/usr/sbin/virtlogd+0x1fb36) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:1269 #4 0x55ab070770e2 (/usr/sbin/virtlogd+0x200e2) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:1563 #5 0x55ab0709c67f in virEventPollRunOnce (/usr/sbin/virtlogd+0x4567f) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/vireventpoll.c:508 #6 0x55ab0709ad30 in virEventRunDefaultImpl (/usr/sbin/virtlogd+0x43d30) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/virevent.c:314 #7 0x55ab07079f6c in virNetDaemonRun (/usr/sbin/virtlogd+0x22f6c) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetdaemon.c:847 #8 0x55ab070714db in main (/usr/sbin/virtlogd+0x1a4db) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/logging/log_daemon.c:1162 #9 0x7f14aa3c2c56 in __libc_start_main (/usr/lib64/libc.so.6+0x25c56) ??:? #10 0x55ab07072639 in _start (/usr/sbin/virtlogd+0x1b639) ??:? 3. job->data->msg was not released Indirect leak of 104 byte(s) in 1 object(s) allocated from: #0 0x7f14ab932560 in calloc (/usr/local/gcc-6-4/lib64/libasan.so.3+0xc7560) ??:? #1 0x55ab07088853 in virAlloc (/usr/sbin/virtlogd+0x31853) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/viralloc.c:144 #2 0x55ab0707d541 in virNetMessageNew (/usr/sbin/virtlogd+0x26541) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetmessage.c:42 #3 0x55ab07076618 (/usr/sbin/virtlogd+0x1f618) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:416 #4 0x55ab0707755c in virNetServerClientNew (/usr/sbin/virtlogd+0x2055c) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:467 #5 0x55ab0707a9df (/usr/sbin/virtlogd+0x239df) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserver.c:319 #6 0x55ab07075ad9 (/usr/sbin/virtlogd+0x1ead9) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverservice.c:88 #7 0x55ab0709c67f in virEventPollRunOnce (/usr/sbin/virtlogd+0x4567f) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/vireventpoll.c:508 #8 0x55ab0709ad30 in virEventRunDefaultImpl (/usr/sbin/virtlogd+0x43d30) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/virevent.c:314 #9 0x55ab07079f6c in virNetDaemonRun (/usr/sbin/virtlogd+0x22f6c) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetdaemon.c:847 #10 0x55ab070714db in main (/usr/sbin/virtlogd+0x1a4db) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/logging/log_daemon.c:1162 #11 0x7f14aa3c2c56 in __libc_start_main (/usr/lib64/libc.so.6+0x25c56) ??:? #12 0x55ab07072639 in _start (/usr/sbin/virtlogd+0x1b639) ??:? 4. job->data->prog was not released Indirect leak of 64 byte(s) in 1 object(s) allocated from: #0 0x7f14ab932560 in calloc (/usr/local/gcc-6-4/lib64/libasan.so.3+0xc7560) ??:? #1 0x55ab07088d34 in virAllocVar (/usr/sbin/virtlogd+0x31d34) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/viralloc.c:560 #2 0x55ab070ac45c in virObjectNew (/usr/sbin/virtlogd+0x5545c) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/virobject.c:200 #3 0x55ab07074f06 in virNetServerProgramNew (/usr/sbin/virtlogd+0x1df06) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverprogram.c:80 #4 0x55ab07071432 in main (/usr/sbin/virtlogd+0x1a432) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/logging/log_daemon.c:1131 #5 0x7f14aa3c2c56 in __libc_start_main (/usr/lib64/libc.so.6+0x25c56) ??:? #6 0x55ab07072639 in _start (/usr/sbin/virtlogd+0x1b639) ??:? Signed-off-by: Jian wang <wangjian161@huawei.com> --- src/libvirt_private.syms | 1 + src/rpc/virnetserver.c | 12 ++++++++++++ src/util/virthreadpool.c | 9 +++++++++ src/util/virthreadpool.h | 2 ++ 4 files changed, 24 insertions(+) diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a6af44f..b23fc33 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3316,6 +3316,7 @@ virThreadPoolGetMaxWorkers; virThreadPoolGetMinWorkers; virThreadPoolGetPriorityWorkers; virThreadPoolNewFull; +virThreadPoolSetFreeFunc; virThreadPoolSendJob; virThreadPoolSetParameters; diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index e0a2386..66f0fb6 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -188,6 +188,15 @@ virNetServerGetProgramLocked(virNetServerPtr srv, return NULL; } +static void virNetServerHandleFree(void *jobOpaque) +{ + virNetServerJobPtr job = jobOpaque; + + virObjectUnref(job->prog); + virNetMessageFree(job->msg); + VIR_FREE(job); +} + static void virNetServerDispatchNewMessage(virNetServerClientPtr client, virNetMessagePtr msg, @@ -376,6 +385,9 @@ virNetServerPtr virNetServerNew(const char *name, srv))) goto error; + if (srv->workers) + virThreadPoolSetFreeFunc(srv->workers, virNetServerHandleFree); + srv->name = g_strdup(name); srv->next_client_id = next_client_id; diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c index 379d236..f885c04 100644 --- a/src/util/virthreadpool.c +++ b/src/util/virthreadpool.c @@ -54,6 +54,7 @@ struct _virThreadPool { bool quit; virThreadPoolJobFunc jobFunc; + virThreadPoolFreeFunc freeFunc; const char *jobName; void *jobOpaque; virThreadPoolJobList jobList; @@ -271,6 +272,12 @@ virThreadPoolNewFull(size_t minWorkers, } +void virThreadPoolSetFreeFunc(virThreadPoolPtr pool, + virThreadPoolFreeFunc func) +{ + pool->freeFunc = func; +} + void virThreadPoolFree(virThreadPoolPtr pool) { virThreadPoolJobPtr job; @@ -293,6 +300,8 @@ void virThreadPoolFree(virThreadPoolPtr pool) while ((job = pool->jobList.head)) { pool->jobList.head = pool->jobList.head->next; + if (pool->freeFunc) + pool->freeFunc(job->data); VIR_FREE(job); } diff --git a/src/util/virthreadpool.h b/src/util/virthreadpool.h index c97d9b3..d2e24f7 100644 --- a/src/util/virthreadpool.h +++ b/src/util/virthreadpool.h @@ -27,6 +27,7 @@ typedef struct _virThreadPool virThreadPool; typedef virThreadPool *virThreadPoolPtr; typedef void (*virThreadPoolJobFunc)(void *jobdata, void *opaque); +typedef void (*virThreadPoolFreeFunc)(void *jobdata); #define virThreadPoolNew(min, max, prio, func, opaque) \ virThreadPoolNewFull(min, max, prio, func, #func, opaque) @@ -46,6 +47,7 @@ size_t virThreadPoolGetFreeWorkers(virThreadPoolPtr pool); size_t virThreadPoolGetJobQueueDepth(virThreadPoolPtr pool); void virThreadPoolFree(virThreadPoolPtr pool); +void virThreadPoolSetFreeFunc(virThreadPoolPtr pool, virThreadPoolFreeFunc func); int virThreadPoolSendJob(virThreadPoolPtr pool, unsigned int priority, -- 2.23.0
On Tue, 2020-06-16 at 14:26 +0800, wangjian wrote:
We used asan to find some memory leaks in virtlogd. In the virThreadPoolFree function, When job->data is of type virNetServerJobPtr, the following memory leak problem exists.
Please don't CC random developers when posting patches, or send the same patch twice. See [1] for more information. If your patch has been sitting on the list for more than a week or so, you can ping the list by replying to the original message (or the cover letter for a multi-patch series). I'll leave the actual review to someone with more experience in this area of libvirt. [1] https://libvirt.org/submitting-patches.html -- Andrea Bolognani / Red Hat / Virtualization
On 6/16/20 8:26 AM, wangjian wrote:
We used asan to find some memory leaks in virtlogd. In the virThreadPoolFree function, When job->data is of type virNetServerJobPtr, the following memory leak problem exists.
1. job->data is not released Direct leak of 24 byte(s) in 1 object(s) allocated from: #0 0x7f14ab932560 in calloc (/usr/local/gcc-6-4/lib64/libasan.so.3+0xc7560) ??:? #1 0x55ab07088853 in virAlloc (/usr/sbin/virtlogd+0x31853) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/viralloc.c:144 #2 0x55ab0707a515 (/usr/sbin/virtlogd+0x23515) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserver.c:209 #3 0x55ab07076d87 (/usr/sbin/virtlogd+0x1fd87) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:1374 #4 0x55ab070770e2 (/usr/sbin/virtlogd+0x200e2) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:1563 #5 0x55ab0709c67f in virEventPollRunOnce (/usr/sbin/virtlogd+0x4567f) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/vireventpoll.c:508 #6 0x55ab0709ad30 in virEventRunDefaultImpl (/usr/sbin/virtlogd+0x43d30) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/virevent.c:314 #7 0x55ab07079f6c in virNetDaemonRun (/usr/sbin/virtlogd+0x22f6c) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetdaemon.c:847 #8 0x55ab070714db in main (/usr/sbin/virtlogd+0x1a4db) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/logging/log_daemon.c:1162 #9 0x7f14aa3c2c56 in __libc_start_main (/usr/lib64/libc.so.6+0x25c56) ??:? #10 0x55ab07072639 in _start (/usr/sbin/virtlogd+0x1b639) ??:?
Hey, firstly, I'm interested how you used asan. By providing -fsanitize=address to CFLAGS? Secondly, can you test the latest master? The memleaks you attach show libvirt-3.2.0. I've ran valgrind over the latest master but was unable to reproduce any leak: ==107012== definitely lost: 0 bytes in 0 blocks ==107012== indirectly lost: 0 bytes in 0 blocks ==107012== possibly lost: 1,368 bytes in 19 blocks What do I need to do to reproduce? Michal
1. Yes, our testers opened some compilation options, such as -fsanitizer=leak. 2. The version of libvirt we are using is 3.2.0. Our tester deployed a long-lived environment and did many operations (he didn't remember what he did), such as restarting virtlogd, restarting libvirtd, creating virtual machines, kill -9 virtlogd, libvirtd, etc. -----Original Message----- From: Michal Privoznik [mailto:mprivozn@redhat.com] Sent: Tuesday, June 16, 2020 9:58 PM To: wangjian (AN) <wangjian161@huawei.com> Cc: libvir-list@redhat.com Subject: Re: [PATCH] virtlogd: solve some memory leaks On 6/16/20 8:26 AM, wangjian wrote:
We used asan to find some memory leaks in virtlogd. In the virThreadPoolFree function, When job->data is of type virNetServerJobPtr, the following memory leak problem exists.
1. job->data is not released Direct leak of 24 byte(s) in 1 object(s) allocated from: #0 0x7f14ab932560 in calloc (/usr/local/gcc-6-4/lib64/libasan.so.3+0xc7560) ??:? #1 0x55ab07088853 in virAlloc (/usr/sbin/virtlogd+0x31853) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/viralloc.c:144 #2 0x55ab0707a515 (/usr/sbin/virtlogd+0x23515) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserver.c:209 #3 0x55ab07076d87 (/usr/sbin/virtlogd+0x1fd87) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:1374 #4 0x55ab070770e2 (/usr/sbin/virtlogd+0x200e2) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:1563 #5 0x55ab0709c67f in virEventPollRunOnce (/usr/sbin/virtlogd+0x4567f) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/vireventpoll.c:508 #6 0x55ab0709ad30 in virEventRunDefaultImpl (/usr/sbin/virtlogd+0x43d30) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/virevent.c:314 #7 0x55ab07079f6c in virNetDaemonRun (/usr/sbin/virtlogd+0x22f6c) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetdaemon.c:847 #8 0x55ab070714db in main (/usr/sbin/virtlogd+0x1a4db) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/logging/log_daemon.c:1162 #9 0x7f14aa3c2c56 in __libc_start_main (/usr/lib64/libc.so.6+0x25c56) ??:? #10 0x55ab07072639 in _start (/usr/sbin/virtlogd+0x1b639) ??:?
Hey, firstly, I'm interested how you used asan. By providing -fsanitize=address to CFLAGS? Secondly, can you test the latest master? The memleaks you attach show libvirt-3.2.0. I've ran valgrind over the latest master but was unable to reproduce any leak: ==107012== definitely lost: 0 bytes in 0 blocks ==107012== indirectly lost: 0 bytes in 0 blocks ==107012== possibly lost: 1,368 bytes in 19 blocks What do I need to do to reproduce? Michal
On 6/17/20 9:58 AM, wangjian (AN) wrote:
1. Yes, our testers opened some compilation options, such as -fsanitizer=leak. 2. The version of libvirt we are using is 3.2.0. Our tester deployed a long-lived environment and did many operations (he didn't remember what he did), such as restarting virtlogd, restarting libvirtd, creating virtual machines, kill -9 virtlogd, libvirtd, etc.
I've tried -fsanitize=leak but the only issues it found were some tests failing because of unexpected retvals from virsh (because asan changes that if a memleak was found). Anyway, I haven't found any leaks inside the virtlogd. The thing is, if you found memleaks in 3.2.0 that's a shame, but it's likely that those are fixed upstream already. I mean, 3.2.0 is three years old. Therefore, we need to prove that these memleaks can happen with more recent versions and it's not obvious from the code, sorry. Michal
On a Tuesday in 2020, wangjian wrote:
We used asan to find some memory leaks in virtlogd. In the virThreadPoolFree function, When job->data is of type virNetServerJobPtr, the following memory leak problem exists.
1. job->data is not released Direct leak of 24 byte(s) in 1 object(s) allocated from: #0 0x7f14ab932560 in calloc (/usr/local/gcc-6-4/lib64/libasan.so.3+0xc7560) ??:? #1 0x55ab07088853 in virAlloc (/usr/sbin/virtlogd+0x31853) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/viralloc.c:144 #2 0x55ab0707a515 (/usr/sbin/virtlogd+0x23515) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserver.c:209 #3 0x55ab07076d87 (/usr/sbin/virtlogd+0x1fd87) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:1374 #4 0x55ab070770e2 (/usr/sbin/virtlogd+0x200e2) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:1563 #5 0x55ab0709c67f in virEventPollRunOnce (/usr/sbin/virtlogd+0x4567f) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/vireventpoll.c:508 #6 0x55ab0709ad30 in virEventRunDefaultImpl (/usr/sbin/virtlogd+0x43d30) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/virevent.c:314 #7 0x55ab07079f6c in virNetDaemonRun (/usr/sbin/virtlogd+0x22f6c) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetdaemon.c:847 #8 0x55ab070714db in main (/usr/sbin/virtlogd+0x1a4db) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/logging/log_daemon.c:1162 #9 0x7f14aa3c2c56 in __libc_start_main (/usr/lib64/libc.so.6+0x25c56) ??:? #10 0x55ab07072639 in _start (/usr/sbin/virtlogd+0x1b639) ??:?
2. job->data->msg->buffer was not released Indirect leak of 152 byte(s) in 1 object(s) allocated from: #0 0x7f14ab932750 in realloc (/usr/local/gcc-6-4/lib64/libasan.so.3+0xc7750) ??:? #1 0x55ab0708894d in virReallocN (/usr/sbin/virtlogd+0x3194d) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/viralloc.c:245 #2 0x55ab0707d830 in virNetMessageDecodeLength (/usr/sbin/virtlogd+0x26830) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetmessage.c:157 #3 0x55ab07076b36 (/usr/sbin/virtlogd+0x1fb36) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:1269 #4 0x55ab070770e2 (/usr/sbin/virtlogd+0x200e2) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:1563 #5 0x55ab0709c67f in virEventPollRunOnce (/usr/sbin/virtlogd+0x4567f) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/vireventpoll.c:508 #6 0x55ab0709ad30 in virEventRunDefaultImpl (/usr/sbin/virtlogd+0x43d30) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/virevent.c:314 #7 0x55ab07079f6c in virNetDaemonRun (/usr/sbin/virtlogd+0x22f6c) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetdaemon.c:847 #8 0x55ab070714db in main (/usr/sbin/virtlogd+0x1a4db) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/logging/log_daemon.c:1162 #9 0x7f14aa3c2c56 in __libc_start_main (/usr/lib64/libc.so.6+0x25c56) ??:? #10 0x55ab07072639 in _start (/usr/sbin/virtlogd+0x1b639) ??:?
3. job->data->msg was not released Indirect leak of 104 byte(s) in 1 object(s) allocated from: #0 0x7f14ab932560 in calloc (/usr/local/gcc-6-4/lib64/libasan.so.3+0xc7560) ??:? #1 0x55ab07088853 in virAlloc (/usr/sbin/virtlogd+0x31853) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/viralloc.c:144 #2 0x55ab0707d541 in virNetMessageNew (/usr/sbin/virtlogd+0x26541) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetmessage.c:42 #3 0x55ab07076618 (/usr/sbin/virtlogd+0x1f618) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:416 #4 0x55ab0707755c in virNetServerClientNew (/usr/sbin/virtlogd+0x2055c) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverclient.c:467 #5 0x55ab0707a9df (/usr/sbin/virtlogd+0x239df) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserver.c:319 #6 0x55ab07075ad9 (/usr/sbin/virtlogd+0x1ead9) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverservice.c:88 #7 0x55ab0709c67f in virEventPollRunOnce (/usr/sbin/virtlogd+0x4567f) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/vireventpoll.c:508 #8 0x55ab0709ad30 in virEventRunDefaultImpl (/usr/sbin/virtlogd+0x43d30) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/virevent.c:314 #9 0x55ab07079f6c in virNetDaemonRun (/usr/sbin/virtlogd+0x22f6c) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetdaemon.c:847 #10 0x55ab070714db in main (/usr/sbin/virtlogd+0x1a4db) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/logging/log_daemon.c:1162 #11 0x7f14aa3c2c56 in __libc_start_main (/usr/lib64/libc.so.6+0x25c56) ??:? #12 0x55ab07072639 in _start (/usr/sbin/virtlogd+0x1b639) ??:?
4. job->data->prog was not released Indirect leak of 64 byte(s) in 1 object(s) allocated from: #0 0x7f14ab932560 in calloc (/usr/local/gcc-6-4/lib64/libasan.so.3+0xc7560) ??:? #1 0x55ab07088d34 in virAllocVar (/usr/sbin/virtlogd+0x31d34) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/viralloc.c:560 #2 0x55ab070ac45c in virObjectNew (/usr/sbin/virtlogd+0x5545c) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/util/virobject.c:200 #3 0x55ab07074f06 in virNetServerProgramNew (/usr/sbin/virtlogd+0x1df06) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/rpc/virnetserverprogram.c:80 #4 0x55ab07071432 in main (/usr/sbin/virtlogd+0x1a432) /usr/src/debug/libvirt-3.2.0-529.x86_64/src/logging/log_daemon.c:1131 #5 0x7f14aa3c2c56 in __libc_start_main (/usr/lib64/libc.so.6+0x25c56) ??:? #6 0x55ab07072639 in _start (/usr/sbin/virtlogd+0x1b639) ??:?
Signed-off-by: Jian wang <wangjian161@huawei.com> --- src/libvirt_private.syms | 1 + src/rpc/virnetserver.c | 12 ++++++++++++ src/util/virthreadpool.c | 9 +++++++++ src/util/virthreadpool.h | 2 ++ 4 files changed, 24 insertions(+)
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a6af44f..b23fc33 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -3316,6 +3316,7 @@ virThreadPoolGetMaxWorkers; virThreadPoolGetMinWorkers; virThreadPoolGetPriorityWorkers; virThreadPoolNewFull; +virThreadPoolSetFreeFunc; virThreadPoolSendJob; virThreadPoolSetParameters;
diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index e0a2386..66f0fb6 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -188,6 +188,15 @@ virNetServerGetProgramLocked(virNetServerPtr srv, return NULL; }
+static void virNetServerHandleFree(void *jobOpaque)
virNetServerJobFree, no need for two verbs. Also, using a free function in one place and open-coding it in HandleJob is incosistent.
+{ + virNetServerJobPtr job = jobOpaque; + + virObjectUnref(job->prog); + virNetMessageFree(job->msg); + VIR_FREE(job); +} + static void virNetServerDispatchNewMessage(virNetServerClientPtr client, virNetMessagePtr msg, @@ -376,6 +385,9 @@ virNetServerPtr virNetServerNew(const char *name, srv))) goto error;
+ if (srv->workers) + virThreadPoolSetFreeFunc(srv->workers, virNetServerHandleFree);
If there is a need for a free function, it should be a parameter to virThreadPoolNewFull, the other callers might need it too.
+ srv->name = g_strdup(name);
srv->next_client_id = next_client_id; diff --git a/src/util/virthreadpool.c b/src/util/virthreadpool.c index 379d236..f885c04 100644 --- a/src/util/virthreadpool.c +++ b/src/util/virthreadpool.c @@ -54,6 +54,7 @@ struct _virThreadPool { bool quit;
virThreadPoolJobFunc jobFunc; + virThreadPoolFreeFunc freeFunc; const char *jobName; void *jobOpaque; virThreadPoolJobList jobList; @@ -271,6 +272,12 @@ virThreadPoolNewFull(size_t minWorkers,
}
+void virThreadPoolSetFreeFunc(virThreadPoolPtr pool, + virThreadPoolFreeFunc func) +{ + pool->freeFunc = func; +} + void virThreadPoolFree(virThreadPoolPtr pool) { virThreadPoolJobPtr job; @@ -293,6 +300,8 @@ void virThreadPoolFree(virThreadPoolPtr pool)
while ((job = pool->jobList.head)) { pool->jobList.head = pool->jobList.head->next; + if (pool->freeFunc) + pool->freeFunc(job->data);
I was not able to hit this code path in my testing - it seems all the jobs get handled for me. A simple reproducer would really help. And in virThreadPoolWorker we also call jobFunc and expect it to free the job data. Calling jobFunc and freeFunc separately would look cleaner to me. Jano
VIR_FREE(job); }
diff --git a/src/util/virthreadpool.h b/src/util/virthreadpool.h index c97d9b3..d2e24f7 100644 --- a/src/util/virthreadpool.h +++ b/src/util/virthreadpool.h @@ -27,6 +27,7 @@ typedef struct _virThreadPool virThreadPool; typedef virThreadPool *virThreadPoolPtr;
typedef void (*virThreadPoolJobFunc)(void *jobdata, void *opaque); +typedef void (*virThreadPoolFreeFunc)(void *jobdata);
#define virThreadPoolNew(min, max, prio, func, opaque) \ virThreadPoolNewFull(min, max, prio, func, #func, opaque) @@ -46,6 +47,7 @@ size_t virThreadPoolGetFreeWorkers(virThreadPoolPtr pool); size_t virThreadPoolGetJobQueueDepth(virThreadPoolPtr pool);
void virThreadPoolFree(virThreadPoolPtr pool); +void virThreadPoolSetFreeFunc(virThreadPoolPtr pool, virThreadPoolFreeFunc func);
int virThreadPoolSendJob(virThreadPoolPtr pool, unsigned int priority, -- 2.23.0
participants (5)
-
Andrea Bolognani -
Ján Tomko -
Michal Privoznik -
wangjian -
wangjian (AN)