Richard W.M. Jones
2017-Dec-02 15:27 UTC
[Libguestfs] [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)
Richard W.M. Jones
2017-Dec-02 15:27 UTC
[Libguestfs] [PATCH nbdkit nbd] nbd: Unsuccessful attempt to fix memory leak.
==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
Eric Blake
2017-Dec-02 17:56 UTC
[Libguestfs] [PATCH nbdkit nbd] nbd: Unsuccessful attempt to fix memory leak.
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 -------------- next part -------------- A non-text attachment was scrubbed... Name: signature.asc Type: application/pgp-signature Size: 619 bytes Desc: OpenPGP digital signature URL: <http://listman.redhat.com/archives/libguestfs/attachments/20171202/c563b5e5/attachment.sig>