Re: AW: [lwip-devel] Memory leak for timeouts

[Jonathan Larmour]

Goldschmidt Simon wrote:
>> As a consequence, we have many threads created. By doing some tests,
> we've found a memory leak around timeouts. 
>> Indeed, sys_timeouts should provide memory local to each thread. On
> L4, we use TLS (thread local
>> storage) : in each thread, if no TLS corresponding to sys_timeouts has
> been referenced, a malloc is done and is 
>> then referenced into TLS.
>> But, this little chunk of memory is newer freed.
> 
>> Today, we decided to fix this by creating a sys_thread_exit function
> which free the sys_timeout structure
>> referenced into TLS. Is there another way we didn't find in lwip ?
>> Should I propose a patch (sys.h) ? a bug ?
> 
> This is indeed a problem if you create and destroy threads at runtime.
> However, the lwIP sys layer is designed to never end a running thread
> (as most embedded systems do), so this is not a problem for most users.

Actually that's not true if you use select(), which can call
sys_sem_wait_timeout(), which calls sys_timeout().

Marc, in case it's helpful, in my port I made a sneaky optimisation.
sys_arch_timeouts() is expected to return a "struct sys_timeouts *". My
per-thread data is one word. Strictly I should malloc a "struct
sys_timeout", put its address in the per-thread data, and clear it up when
the thread gets deleted.

However in practice, sys.c uses the return value of sys_arch_timeouts()
only ever by referencing its "next" member which is the first member of the
struct. So if I return the _address_ of the per-thread data, then that
still works, and nothing needs to be cleaned up.

This trick may work for you and others. It may not work if someone changes
sys.c in some way that would break it, but I would like to argue that they
don't :-).

Jifl
-- 
eCosCentric Limited      http://www.eCosCentric.com/     The eCos experts
Barnwell House, Barnwell Drive, Cambridge, UK.       Tel: +44 1223 245571
Registered in England and Wales: Reg No 4422071.
------["The best things in life aren't things."]------      Opinions==mine