Re: [PATCH] Fix stdio memory leaks with _REENT_SMALL + _LITE_EXIT

[Sebastian Huber <sebastian.huber@embedded-brains.de>]

On 12/04/2022 15:23, Volodymyr Medvid wrote:
> When a thread calls stdio functions (say, printf) and then dies,
> _reclaim_reent() runs cleanup_stdio() to free the file buffers
> and descriptors created for this thread. This is causing multiple
> memory leaks when newlib is configured with _REENT_SMALL
> and _LITE_EXIT - this is the standard configuration for
> newlib-nano provided with GNU Arm Embedded Toolchain.
> 
> 1. While __sfp() allocates the FILE objects in GLOBAL_REENT glue chain,
>     stdio_cleanup walks through the thread-specific glue chain
>     to run the cleanup_func. Therefore, the FILE objects are never
>     freed. This leaks ~428 bytes per thread (glue_with_file + 3 x FILE).

Yes, this is the Newlib design. FILE objects are never freed. If you 
close a FILE object, then it is basically placed on a free list and 
recycled once another FILE object is opened.

> 
>     To fix this, update __sfp() to use the per-thread glue chain for
>     stdio descriptors.

This is definitely not the right way to address this issue.

Why don't you use the _REENT_GLOBAL_STDIO_STREAMS Newlib configuration 
option?

> 
> 2. With _LITE_EXIT enabled, _fflush_r is used as cleanup_func
>     instead of _fclose_r - as a result, the I/O buffer memory allocated
>     by __smakebuf_r is never freed - this leaks another 1024 bytes.
>     To fix this, update cleanup_stdio to always use _fclose_r.
> 
> This is a follow-up patch forhttps://ecos.sourceware.org/ml/newlib/current/017697.html
> ---
>   newlib/libc/stdio/findfp.c | 9 ++-------
>   1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/newlib/libc/stdio/findfp.c b/newlib/libc/stdio/findfp.c
> index 1370b63b8..2799980f3 100644
> --- a/newlib/libc/stdio/findfp.c
> +++ b/newlib/libc/stdio/findfp.c
> @@ -153,7 +153,7 @@ __sfp (struct _reent *d)
>   
>     if (_GLOBAL_REENT->__cleanup == NULL)
>       __sinit (_GLOBAL_REENT);
> -  for (g = &_GLOBAL_REENT->__sglue;; g = g->_next)
> +  for (g = &d->__sglue;; g = g->_next)
>       {
>         for (fp = g->_iobs, n = g->_niobs; --n >= 0; fp++)
>   	if (fp->_flags == 0)

This patch would break at least RTEMS.

-- 
embedded brains GmbH
Herr Sebastian HUBER
Dornierstr. 4
82178 Puchheim
Germany
email: sebastian.huber@embedded-brains.de
phone: +49-89-18 94 741 - 16
fax:   +49-89-18 94 741 - 08

Registergericht: Amtsgericht München
Registernummer: HRB 157899
Vertretungsberechtigte Geschäftsführer: Peter Rasmussen, Thomas Dörfler
Unsere Datenschutzerklärung finden Sie hier:
https://embedded-brains.de/datenschutzerklaerung/