Re: [libvirt] [PATCH 02/14] rpc: Fix memory leaks @virnetserverclient

On Fri, Dec 15, 2017 at 01:14 PM +0100, John Ferlan <jferlan(a)redhat.com&gt; 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(a)linux.vnet.ibm.com&gt; >> Reviewed-by: Boris Fiuczynski <fiuczy(a)linux.vnet.ibm.com&gt; >> --- >> 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?

> > 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(a)redhat.com&gt; > > 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