[v1] target/riscv: Fix memory leak in riscv_trigger_unrealize()

[PATCH] target/riscv: Fix memory leak in riscv_trigger_unrealize()

Posted by Zeng Chi 1 day, 20 hours ago

From: Zeng Chi <zengchi@kylinos.cn>

In riscv_trigger_unrealize(), the per-trigger QEMUTimer objects are
created in riscv_trigger_realize() using timer_new_ns().  However,
unrealize only calls timer_del() to cancel them, but never frees the
timer objects themselves. This results in a memory leak every time a
CPU instance is unrealized (e.g., during hot-unplug or machine teardown).

Fix it by calling timer_free() for each timer after timer_del().

Fixes: 820552a92e32 ("target/riscv: dynamic alloc of debug trigger arrays")
Signed-off-by: Zeng Chi <zengchi@kylinos.cn>
---
 target/riscv/tcg/debug.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/target/riscv/tcg/debug.c b/target/riscv/tcg/debug.c
index 3c0fe70101..ffbdc17023 100644
--- a/target/riscv/tcg/debug.c
+++ b/target/riscv/tcg/debug.c
@@ -1081,6 +1081,7 @@ void riscv_trigger_unrealize(CPURISCVState *env)
 
     for (int i = 0; i < env->num_triggers; i++) {
         timer_del(env->itrigger_timer[i]);
+        timer_free(env->itrigger_timer[i]);
     }
     g_free(env->itrigger_timer);
 }
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

Re: [PATCH] target/riscv: Fix memory leak in riscv_trigger_unrealize()

Posted by Daniel Henrique Barboza 1 day, 19 hours ago


On 7/23/2026 6:59 AM, Zeng Chi wrote:
> From: Zeng Chi <zengchi@kylinos.cn>
> 
> In riscv_trigger_unrealize(), the per-trigger QEMUTimer objects are
> created in riscv_trigger_realize() using timer_new_ns().  However,
> unrealize only calls timer_del() to cancel them, but never frees the
> timer objects themselves. This results in a memory leak every time a
> CPU instance is unrealized (e.g., during hot-unplug or machine teardown).
> 
> Fix it by calling timer_free() for each timer after timer_del().

I made the assumption that timer_del() calls g_free() under the hood.  Seems
like the best assumption is always to not make assumptions ...

> 
> Fixes: 820552a92e32 ("target/riscv: dynamic alloc of debug trigger arrays")
> Signed-off-by: Zeng Chi <zengchi@kylinos.cn>
> ---

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>

>   target/riscv/tcg/debug.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/target/riscv/tcg/debug.c b/target/riscv/tcg/debug.c
> index 3c0fe70101..ffbdc17023 100644
> --- a/target/riscv/tcg/debug.c
> +++ b/target/riscv/tcg/debug.c
> @@ -1081,6 +1081,7 @@ void riscv_trigger_unrealize(CPURISCVState *env)
>   
>       for (int i = 0; i < env->num_triggers; i++) {
>           timer_del(env->itrigger_timer[i]);
> +        timer_free(env->itrigger_timer[i]);
>       }
>       g_free(env->itrigger_timer);
>   }

Re: [PATCH] target/riscv: Fix memory leak in riscv_trigger_unrealize()

Posted by Philippe Mathieu-Daudé 1 day, 19 hours ago

On 23/7/26 13:12, Daniel Henrique Barboza wrote:
> 
> 
> On 7/23/2026 6:59 AM, Zeng Chi wrote:
>> From: Zeng Chi <zengchi@kylinos.cn>
>>
>> In riscv_trigger_unrealize(), the per-trigger QEMUTimer objects are
>> created in riscv_trigger_realize() using timer_new_ns().  However,
>> unrealize only calls timer_del() to cancel them, but never frees the
>> timer objects themselves. This results in a memory leak every time a
>> CPU instance is unrealized (e.g., during hot-unplug or machine teardown).
>>
>> Fix it by calling timer_free() for each timer after timer_del().
> 
> I made the assumption that timer_del() calls g_free() under the hood.  
> Seems
> like the best assumption is always to not make assumptions ...

Same, I always gets confused by that method. I once asked on IRC whether
to rename timer_del() to timer_cancel() or not but don't remember the
outcome of the discussion, I suppose there was a reason for not doing
the mechanical change, like "delete" is the appropriate action to cancel
a timer?

> 
>>
>> Fixes: 820552a92e32 ("target/riscv: dynamic alloc of debug trigger 
>> arrays")
>> Signed-off-by: Zeng Chi <zengchi@kylinos.cn>
>> ---
> 
> Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com>
> 
>>   target/riscv/tcg/debug.c | 1 +
>>   1 file changed, 1 insertion(+)
>>
>> diff --git a/target/riscv/tcg/debug.c b/target/riscv/tcg/debug.c
>> index 3c0fe70101..ffbdc17023 100644
>> --- a/target/riscv/tcg/debug.c
>> +++ b/target/riscv/tcg/debug.c
>> @@ -1081,6 +1081,7 @@ void riscv_trigger_unrealize(CPURISCVState *env)
>>       for (int i = 0; i < env->num_triggers; i++) {
>>           timer_del(env->itrigger_timer[i]);
>> +        timer_free(env->itrigger_timer[i]);

timer_free() also cancel/delete the timer. Could we simply replace
timer_del() -> timer_free()?

>>       }
>>       g_free(env->itrigger_timer);
>>   }
> 
>

[PATCH v2] target/riscv: Fix memory leak in riscv_trigger_unrealize()

Posted by Zeng Chi 1 day ago

From: Zeng Chi <zengchi@kylinos.cn>

In riscv_trigger_unrealize(), the per-trigger QEMUTimer objects are
created in riscv_trigger_realize() using timer_new_ns().  However,
unrealize only calls timer_del() to cancel them, but never frees the
timer objects themselves. This results in a memory leak every time a
CPU instance is unrealized (e.g., during hot-unplug or machine teardown).

Fix it by replacing timer_del() with timer_free(), which internally
cancels the timer and frees its memory.  The separate timer_del() call
is no longer needed.

Fixes: 820552a92e32 ("target/riscv: dynamic alloc of debug trigger arrays")
Signed-off-by: Zeng Chi <zengchi@kylinos.cn>
---
 target/riscv/tcg/debug.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/target/riscv/tcg/debug.c b/target/riscv/tcg/debug.c
index 3c0fe70101..5ab29f6945 100644
--- a/target/riscv/tcg/debug.c
+++ b/target/riscv/tcg/debug.c
@@ -1080,7 +1080,7 @@ void riscv_trigger_unrealize(CPURISCVState *env)
     g_free(env->cpu_watchpoint);
 
     for (int i = 0; i < env->num_triggers; i++) {
-        timer_del(env->itrigger_timer[i]);
+        timer_free(env->itrigger_timer[i]);
     }
     g_free(env->itrigger_timer);
 }
-- 
2.25.1


No virus found
		Checked by Hillstone Network AntiVirus

Re: [PATCH v2] target/riscv: Fix memory leak in riscv_trigger_unrealize()

Posted by Philippe Mathieu-Daudé 23 hours ago

On 24/7/26 08:39, Zeng Chi wrote:
> From: Zeng Chi <zengchi@kylinos.cn>
> 
> In riscv_trigger_unrealize(), the per-trigger QEMUTimer objects are
> created in riscv_trigger_realize() using timer_new_ns().  However,
> unrealize only calls timer_del() to cancel them, but never frees the
> timer objects themselves. This results in a memory leak every time a
> CPU instance is unrealized (e.g., during hot-unplug or machine teardown).
> 
> Fix it by replacing timer_del() with timer_free(), which internally
> cancels the timer and frees its memory.  The separate timer_del() call
> is no longer needed.
> 
> Fixes: 820552a92e32 ("target/riscv: dynamic alloc of debug trigger arrays")
> Signed-off-by: Zeng Chi <zengchi@kylinos.cn>
> ---
>   target/riscv/tcg/debug.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Daniel Henrique Barboza <daniel.barboza@oss.qualcomm.com> 
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>