[PATCH nbdkit nbd] nbd: Unsuccessful attempt to fix memory leak.
Hi Eric, There's a memory leak in the nbd client. The message (below) is not very useful because somehow debuginfo is missing in the plugin. However it's easily reproducible by doing: make check-valgrind TESTS=test-nbd I tried the attached patch to fix what I thought was the bug, but sadly the fix doesn't work for me :-( So I guess something else is going on, but it does look as if the transactions list also needs to be freed. Rich. ==20307== ==20307== HEAP SUMMARY: ==20307== in use at exit: 2,267 bytes in 23 blocks ==20307== total heap usage: 2,298 allocs, 2,275 frees, 307,606 bytes allocated ==20307== ==20307== 96 bytes in 3 blocks are definitely lost in loss record 4 of 10 ==20307== at 0x4C2FA1E: calloc (vg_replace_malloc.c:711) ==20307== by 0x77EBBAF: ??? ==20307== by 0x77EBD17: ??? ==20307== by 0x40595C: handle_request (connections.c:894) ==20307== by 0x40595C: recv_request_send_reply (connections.c:1061) ==20307== by 0x405AE7: connection_worker (connections.c:200) ==20307== by 0x55DC55A: start_thread (in /usr/lib64/libpthread-2.26.9000.so) ==20307== by 0x58E85AE: clone (in /usr/lib64/libc-2.26.9000.so) ==20307== ==20307== 320 bytes in 10 blocks are definitely lost in loss record 9 of 10 ==20307== at 0x4C2FA1E: calloc (vg_replace_malloc.c:711) ==20307== by 0x77EBBAF: ??? ==20307== by 0x77EBD98: ??? ==20307== by 0x405986: handle_request (connections.c:884) ==20307== by 0x405986: recv_request_send_reply (connections.c:1061) ==20307== by 0x405AE7: connection_worker (connections.c:200) ==20307== by 0x55DC55A: start_thread (in /usr/lib64/libpthread-2.26.9000.so) ==20307== by 0x58E85AE: clone (in /usr/lib64/libc-2.26.9000.so) ==20307== ==20307== LEAK SUMMARY: ==20307== definitely lost: 416 bytes in 13 blocks ==20307== indirectly lost: 0 bytes in 0 blocks ==20307== possibly lost: 0 bytes in 0 blocks ==20307== still reachable: 411 bytes in 5 blocks ==20307== suppressed: 1,440 bytes in 5 blocks ==20307== Reachable blocks (those to which a pointer was found) are not shown. ==20307== To see them, rerun with: --leak-check=full --show-leak-kinds=all ==20307== ==20307== For counts of detected and suppressed errors, rerun with: -v ==20307== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 3 from 3)
==20307== ==20307== HEAP SUMMARY: ==20307== in use at exit: 2,267 bytes in 23 blocks ==20307== total heap usage: 2,298 allocs, 2,275 frees, 307,606 bytes allocated ==20307== ==20307== 96 bytes in 3 blocks are definitely lost in loss record 4 of 10 ==20307== at 0x4C2FA1E: calloc (vg_replace_malloc.c:711) ==20307== by 0x77EBBAF: ??? ==20307== by 0x77EBD17: ??? ==20307== by 0x40595C: handle_request (connections.c:894) ==20307== by 0x40595C: recv_request_send_reply (connections.c:1061) ==20307== by 0x405AE7: connection_worker (connections.c:200) ==20307== by 0x55DC55A: start_thread (in /usr/lib64/libpthread-2.26.9000.so) ==20307== by 0x58E85AE: clone (in /usr/lib64/libc-2.26.9000.so) ==20307== ==20307== 320 bytes in 10 blocks are definitely lost in loss record 9 of 10 ==20307== at 0x4C2FA1E: calloc (vg_replace_malloc.c:711) ==20307== by 0x77EBBAF: ??? ==20307== by 0x77EBD98: ??? ==20307== by 0x405986: handle_request (connections.c:884) ==20307== by 0x405986: recv_request_send_reply (connections.c:1061) ==20307== by 0x405AE7: connection_worker (connections.c:200) ==20307== by 0x55DC55A: start_thread (in /usr/lib64/libpthread-2.26.9000.so) ==20307== by 0x58E85AE: clone (in /usr/lib64/libc-2.26.9000.so) ==20307== ==20307== LEAK SUMMARY: ==20307== definitely lost: 416 bytes in 13 blocks ==20307== indirectly lost: 0 bytes in 0 blocks ==20307== possibly lost: 0 bytes in 0 blocks ==20307== still reachable: 411 bytes in 5 blocks ==20307== suppressed: 1,440 bytes in 5 blocks ==20307== Reachable blocks (those to which a pointer was found) are not shown. ==20307== To see them, rerun with: --leak-check=full --show-leak-kinds=all ==20307== ==20307== For counts of detected and suppressed errors, rerun with: -v ==20307== ERROR SUMMARY: 2 errors from 2 contexts (suppressed: 3 from 3) --- plugins/nbd/nbd.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/plugins/nbd/nbd.c b/plugins/nbd/nbd.c index b844bf5..4f5e027 100644 --- a/plugins/nbd/nbd.c +++ b/plugins/nbd/nbd.c @@ -536,6 +536,7 @@ static void nbd_close (void *handle) { struct handle *h = handle; + struct transaction *trans, *trans_next; if (!h->dead) nbd_request_raw (h, NBD_CMD_DISC, 0, 0, 0, NULL); @@ -544,6 +545,10 @@ nbd_close (void *handle) nbdkit_debug ("failed to join reader thread: %m"); pthread_mutex_destroy (&h->write_lock); pthread_mutex_destroy (&h->trans_lock); + for (trans = h->trans; trans != NULL; trans = trans_next) { + trans_next = trans->next; + free (trans); + } free (h); } -- 2.13.2
On 12/02/2017 09:27 AM, Richard W.M. Jones wrote:
Hi Eric,
There's a memory leak in the nbd client. The message (below) is not very useful because somehow debuginfo is missing in the plugin.
I couldn't quickly figure out why that's missing, either. Actually, maybe I can; plugins/nbd/Makefile.am has (via copy-and-paste from other .c plugins): nbdkit_nbd_plugin_la_CFLAGS = \ $(WARNINGS_CFLAGS) which isn't quite right - the moment you override CFLAGS, you have to remember to include $(AM_CFLAGS) for the shared library to be built with the user's CFLAGS. I'll have to test and provide a patch for that later.
However it's easily reproducible by doing:
make check-valgrind TESTS=test-nbd
I tried the attached patch to fix what I thought was the bug, but sadly the fix doesn't work for me :-( So I guess something else is going on, but it does look as if the transactions list also needs to be freed.
Spot on, just the wrong place for trying to free it. The correct place is as items are removed from the list in the reader loop; patch sent separately. -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
participants (2)
-
Eric Blake -
Richard W.M. Jones