On 12/21/2017 06:32 AM, Marc Hartmayer wrote: > On Fri, Dec 15, 2017 at 01:14 PM +0100, John Ferlan <jferlan@xxxxxxxxxx> wrote: >> On 12/12/2017 06:36 AM, Marc Hartmayer wrote: >>> Direct leak of 104 byte(s) in 1 object(s) allocated from: >>> #0 0x7f904bfbe12b (/lib64/liblsan.so.0+0xe12b) >>> #1 0x7f904ba0ad67 in virAlloc ../../src/util/viralloc.c:144 >>> #2 0x7f904bbc11a4 in virNetMessageNew ../../src/rpc/virnetmessage.c:42 >>> #3 0x7f904bbb8e77 in virNetServerClientNewInternal ../../src/rpc/virnetserverclient.c:392 >>> #4 0x7f904bbb9921 in virNetServerClientNew ../../src/rpc/virnetserverclient.c:440 >>> #5 0x402ce5 in testIdentity ../../tests/virnetserverclienttest.c:55 >>> #6 0x403bed in virTestRun ../../tests/testutils.c:180 >>> #7 0x402c1e in mymain ../../tests/virnetserverclienttest.c:146 >>> #8 0x404c80 in virTestMain ../../tests/testutils.c:1119 >>> #9 0x4030d5 in main ../../tests/virnetserverclienttest.c:152 >>> #10 0x7f9047f7f889 in __libc_start_main (/lib64/libc.so.6+0x20889) >>> >>> Indirect leak of 4 byte(s) in 1 object(s) allocated from: >>> #0 0x7f904bfbe12b (/lib64/liblsan.so.0+0xe12b) >>> #1 0x7f904ba0adc7 in virAllocN ../../src/util/viralloc.c:191 >>> #2 0x7f904bbb8ec7 in virNetServerClientNewInternal ../../src/rpc/virnetserverclient.c:395 >>> #3 0x7f904bbb9921 in virNetServerClientNew ../../src/rpc/virnetserverclient.c:440 >>> #4 0x402ce5 in testIdentity ../../tests/virnetserverclienttest.c:55 >>> #5 0x403bed in virTestRun ../../tests/testutils.c:180 >>> #6 0x402c1e in mymain ../../tests/virnetserverclienttest.c:146 >>> #7 0x404c80 in virTestMain ../../tests/testutils.c:1119 >>> #8 0x4030d5 in main ../../tests/virnetserverclienttest.c:152 >>> #9 0x7f9047f7f889 in __libc_start_main (/lib64/libc.so.6+0x20889) >>> >>> SUMMARY: LeakSanitizer: 108 byte(s) leaked in 2 allocation(s). >>> >>> Signed-off-by: Marc Hartmayer <mhartmay@xxxxxxxxxxxxxxxxxx> >>> Reviewed-by: Boris Fiuczynski <fiuczy@xxxxxxxxxxxxxxxxxx> >>> --- >>> src/rpc/virnetserverclient.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c >>> index 6e086b7b4e2b..b454a3ff6992 100644 >>> --- a/src/rpc/virnetserverclient.c >>> +++ b/src/rpc/virnetserverclient.c >>> @@ -948,6 +948,9 @@ void virNetServerClientDispose(void *obj) >>> virObjectUnref(client->tlsCtxt); >>> #endif >>> virObjectUnref(client->sock); >>> + >>> + virNetMessageFree(client->rx); >>> + virNetMessageFree(client->tx); >> >> Makes me curious why virNetServerClientClose isn't doing the Free for >> the rx/tx. Oh, I see, the test program isn't calling it. >> >> I think this is the right idea, but the issue is in the test program >> doing the Unref before calling virNetServerClientClose. > > Yes. Will add an patch for this. Should I remove this patch then? > Yes probably should have been explicit - the R-B below was conditional on altering this patch to call virNetServerClientClose in the test program instead of in virNetServerClientDispose. John >> >> If you see how other tests/code handle this, the client usually gets >> added to the net server [n]clients list (virNetServerAddClient) and >> later when netserver is disposed the nclients is perused and calls >> virNetServerClientClose prior to the Unref as well for each client. >> >> So if you make the right call in tests/virnetserverclienttest.c, then >> you have my >> >> Reviewed-by: John Ferlan <jferlan@xxxxxxxxxx> >> >> John >> >>> } >>> >>> >>> >> > -- > Beste Grüße / Kind regards > Marc Hartmayer > > IBM Deutschland Research & Development GmbH > Vorsitzende des Aufsichtsrats: Martina Koederitz > Geschäftsführung: Dirk Wittkopp > Sitz der Gesellschaft: Böblingen > Registergericht: Amtsgericht Stuttgart, HRB 243294 > -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
