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

[Marc Hartmayer]

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