Comment 9 for bug 1836558

Peter Maydell <email address hidden> writes:

> Ccing the QOM maintainers to make sure we have the
> QOM lifecycle operations right here...
>
> On Tue, 16 Jul 2019 at 15:02, Alex Bennée <email address hidden> wrote:
>>
>> When a CPU object is created it is parented during it's realize stage.
>> If we don't unparent before the "final" unref we will never finzalize
>> the object leading to a memory leak. For most setups you probably
>> won't notice but with anything that creates and destroys a lot of
>> threads this will add up. This goes especially for architectures which
>> allocate a lot of memory in their CPU structures.
>>
>> Fixes: https://bugs.launchpad.net/qemu/+bug/1836558
>> Cc: <email address hidden>
>> Signed-off-by: Alex Bennée <email address hidden>
>> ---
>> linux-user/syscall.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> index 39a37496fed..4c9313fd9d0 100644
>> --- a/linux-user/syscall.c
>> +++ b/linux-user/syscall.c
>> @@ -7183,6 +7183,7 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
>> NULL, NULL, 0);
>> }
>> thread_cpu = NULL;
>> + object_unparent(OBJECT(cpu));
>> object_unref(OBJECT(cpu));
>> g_free(ts);
>> rcu_unregister_thread();
>
> I think (as I mentioned on IRC) that we also need to unrealize
> the CPU object, because target/ppc at least does some freeing
> of memory in its unrealize method. I think we do that by
> setting the QOM "realize" property back to "false" -- but that
> might barf if we try it on a CPU that isn't hotpluggable...

I have tried:

             thread_cpu = NULL;
+ object_unparent(OBJECT(cpu));
+ object_property_set_bool(OBJECT(cpu), false, "realized", NULL);
             object_unref(OBJECT(cpu));

but it didn't manifestly change anything (i.e. both with and without
setting realized the thread allocated stuff is freed). Valgrind still
complains about:

==22483== 6,656 bytes in 26 blocks are possibly lost in loss record 1,639 of 1,654
==22483== at 0x483577F: malloc (vg_replace_malloc.c:299)
==22483== by 0x4D7F8D0: g_malloc (in /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.5800.3)
==22483== by 0x27D692: create_new_table (translate_init.inc.c:9252)
==22483== by 0x27D7CD: register_ind_in_table (translate_init.inc.c:9291)
==22483== by 0x27D975: register_dblind_insn (translate_init.inc.c:9337)
==22483== by 0x27DBBB: register_insn (translate_init.inc.c:9384)
==22483== by 0x27DE4E: create_ppc_opcodes (translate_init.inc.c:9449)
==22483== by 0x27E79C: ppc_cpu_realize (translate_init.inc.c:9818)
==22483== by 0x2C6FE8: device_set_realized (qdev.c:834)
==22483== by 0x2D1E3D: property_set_bool (object.c:2076)
==22483== by 0x2D00B3: object_property_set (object.c:1268)
==22483== by 0x2D3185: object_property_set_qobject (qom-qobject.c:26)

But I don't know if that is just because the final exit_group of the
main CPU just exits without attempting to clean-up. However it is better
than it was.

--
Alex Bennée