2016-12-07 13:44:23

by Loic Poulain

Subject: [PATCH] shared/mainloop: Fix timeout data memleak

Valgrind reports "timeout_data" as definitely lost.
Fix this issue in timeout_destroy function.
---
src/shared/mainloop.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/src/shared/mainloop.c b/src/shared/mainloop.c
index eacb6a4..09c46a7 100644
--- a/src/shared/mainloop.c
+++ b/src/shared/mainloop.c
@@ -283,6 +283,8 @@ static void timeout_destroy(void *user_data)

if (data->destroy)
data->destroy(data->user_data);
+
+ free(data);
}

static void timeout_callback(int fd, uint32_t events, void *user_data)
--
1.9.1

2016-12-08 10:56:43

by Luiz Augusto von Dentz

Subject: Re: [PATCH] shared/mainloop: Fix timeout data memleak

Hi Loic,

On Wed, Dec 7, 2016 at 5:39 PM, <[email protected]> wrote:
> Hi Luiz,
>
>> Seems valid but can you add the backtrace, I wonder how we didn't spot
>> this before so perhaps the backtrace would explain it.
>>
>
> Actually, I included this useful shared code in a different project/poc,
> don't have backtrace in a bluez context.
> This could explain why you've never met this issue (timeout/timerfd seems
> only used in emulator).

Fair enough, applied, thanks.

--
Luiz Augusto von Dentz

2016-12-07 15:39:28

by Loic Poulain

Subject: Re: [PATCH] shared/mainloop: Fix timeout data memleak

Hi Luiz,

> Seems valid but can you add the backtrace, I wonder how we didn't spot
> this before so perhaps the backtrace would explain it.
>

Actually, I included this useful shared code in a different project/poc,
don't have backtrace in a bluez context.
This could explain why you've never met this issue (timeout/timerfd
seems only used in emulator).

Regards,
Loic

2016-12-07 15:11:42

by Luiz Augusto von Dentz

Subject: Re: [PATCH] shared/mainloop: Fix timeout data memleak

Hi Loic,

On Wed, Dec 7, 2016 at 3:44 PM, Loic Poulain <[email protected]> wrote:
> Valgrind reports "timeout_data" as definitely lost.
> Fix this issue in timeout_destroy function.
> ---
> src/shared/mainloop.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/src/shared/mainloop.c b/src/shared/mainloop.c
> index eacb6a4..09c46a7 100644
> --- a/src/shared/mainloop.c
> +++ b/src/shared/mainloop.c
> @@ -283,6 +283,8 @@ static void timeout_destroy(void *user_data)
>
> if (data->destroy)
> data->destroy(data->user_data);
> +
> + free(data);
> }

Seems valid but can you add the backtrace, I wonder how we didn't spot
this before so perhaps the backtrace would explain it.

--
Luiz Augusto von Dentz