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

Sebastian Huber <[email protected]>
Newsgroups gmane.comp.lib.newlib
Message-ID <[email protected]>
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: [email protected]
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/
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.