Possible bug in __sfp() libc routine

Possible bug in __sfp() libc routine

[Kapania, Ashish]

Hi All,

In the __sfp() function in "libc/findfp.c" file, I see that if no free FILE object is found, one is allocated and put on a list in the global re-entrancy structure (_GLOBAL_REENT). This seems like a bug to me. I believe the FILE object should be put on a list in the thread specific reentrancy structure. If I create a thread, do a fopen, do a fwrite (invokes __sfp which in turn allocates the FILE object), do a fclose and then delete the thread, the FILE object allocated by __sfp() is not freed. If a do this sequence repeatedly, I see memory keeps leaking until my app runs out of heap. I have a separate re-entrancy structure for each thread but because the FILE object is not in a list on the local re-entrancy structure, it does not get freed when I delete the thread and run _reclaim_reent() on the local reentrancy structure.

Any thoughts ?

Best,
Ashish

Re: Possible bug in __sfp() libc routine

[Freddie Chopin]

On Fri, 2017-04-07 at 21:57 +0000, Kapania, Ashish wrote:
> Hi All,
> 
> In the __sfp() function in "libc/findfp.c" file, I see that if no
> free FILE object is found, one is allocated and put on a list in the
> global re-entrancy structure (_GLOBAL_REENT). This seems like a bug
> to me. I believe the FILE object should be put on a list in the
> thread specific reentrancy structure. If I create a thread, do a
> fopen, do a fwrite (invokes __sfp which in turn allocates the FILE
> object), do a fclose and then delete the thread, the FILE object
> allocated by __sfp() is not freed. If a do this sequence repeatedly,
> I see memory keeps leaking until my app runs out of heap. I have a
> separate re-entrancy structure for each thread but because the FILE
> object is not in a list on the local re-entrancy structure, it does
> not get freed when I delete the thread and run _reclaim_reent() on
> the local reentrancy structure.
> 
> Any thoughts ?

Hi!

In my understanding newlib just keeps all FILE objects in the
_GLOBAL_REENT but this should not lead to the behaviour you observe.

The objects are indeed shared, which means that they are -
unfortunately - never freed. But on the other hand in your scenario
this should work like this:

1. you start the first thread
2. it tries to open a file
3. __sfp() is called and sees there are no FILE objects, so some are
allocated and the first one is returned
4. when your thread closes the file, its FILE object is kept in
_GLOBAL_REENT, but is marked as free.
5. you start another thread
6. it tries to open a file
7. __sfp() is called, which searches for a free FILE object - it will
find the object which was closed in 4, which will be returned, so
nothing would be allocated
8. same as 4
...

However I must admit that for me the whole design of reentrancy in
newlib is not very consistent and optimal, which causes a lot of memory
to be wasted in a multithreaded application...

Regards,
FCh

Re: Possible bug in __sfp() libc routine

[Martin Velek]

What about this thread?

https://sourceware.org/ml/newlib/2015/msg00619.html . I think this
issue was not fixed or commented yet.

I have written some info related to newlib, reentrancy
(--enable-newlib-reent-small) and using it in FreeRTOS, however the
biggest issue is that it is written in Czech language with no
perspective to be translated into English. I have tried
translate.google.com and it is somehow cumbersome.
https://support.dce.felk.cvut.cz/mediawiki/images/1/17/Dp_2016_velek_martin.pdf

BR
Martin

On Fri, Apr 7, 2017 at 11:57 PM, Kapania, Ashish <akapania@ti.com> wrote:
> Hi All,
>
> In the __sfp() function in "libc/findfp.c" file, I see that if no free FILE object is found, one is allocated and put on a list in the global re-entrancy structure (_GLOBAL_REENT). This seems like a bug to me. I believe the FILE object should be put on a list in the thread specific reentrancy structure. If I create a thread, do a fopen, do a fwrite (invokes __sfp which in turn allocates the FILE object), do a fclose and then delete the thread, the FILE object allocated by __sfp() is not freed. If a do this sequence repeatedly, I see memory keeps leaking until my app runs out of heap. I have a separate re-entrancy structure for each thread but because the FILE object is not in a list on the local re-entrancy structure, it does not get freed when I delete the thread and run _reclaim_reent() on the local reentrancy structure.
>
> Any thoughts ?
>
> Best,
> Ashish

RE: Possible bug in __sfp() libc routine

[Kapania, Ashish]

Hi Freddie,

The steps you suggested makes sense. The FILE object should be marked as free on fclose and should be reused the next time around. In my experiments, it gets reused if I do fopen, fwrite & fclose in a loop. However, when I delete the thread and create a new thread to repeat the sequence, the very first time fwrite is called a new FILE object gets allocated instead of reusing the FILE object created by the now deleted thread. I will step through the fwrite & fclose code and try to figure out what is happening.

Best,
Ashish

> -----Original Message-----
> From: Freddie Chopin [mailto:freddie_chopin@op.pl]
> Sent: Saturday, April 08, 2017 2:04 PM
> To: Kapania, Ashish; newlib@sourceware.org
> Subject: Re: Possible bug in __sfp() libc routine
> 
> On Fri, 2017-04-07 at 21:57 +0000, Kapania, Ashish wrote:
> > Hi All,
> >
> > In the __sfp() function in "libc/findfp.c" file, I see that if no free
> > FILE object is found, one is allocated and put on a list in the global
> > re-entrancy structure (_GLOBAL_REENT). This seems like a bug to me. I
> > believe the FILE object should be put on a list in the thread specific
> > reentrancy structure. If I create a thread, do a fopen, do a fwrite
> > (invokes __sfp which in turn allocates the FILE object), do a fclose
> > and then delete the thread, the FILE object allocated by __sfp() is
> > not freed. If a do this sequence repeatedly, I see memory keeps
> > leaking until my app runs out of heap. I have a separate re-entrancy
> > structure for each thread but because the FILE object is not in a list
> > on the local re-entrancy structure, it does not get freed when I
> > delete the thread and run _reclaim_reent() on the local reentrancy
> > structure.
> >
> > Any thoughts ?
> 
> Hi!
> 
> In my understanding newlib just keeps all FILE objects in the _GLOBAL_REENT
> but this should not lead to the behaviour you observe.
> 
> The objects are indeed shared, which means that they are - unfortunately -
> never freed. But on the other hand in your scenario this should work like this:
> 
> 1. you start the first thread
> 2. it tries to open a file
> 3. __sfp() is called and sees there are no FILE objects, so some are allocated and
> the first one is returned 4. when your thread closes the file, its FILE object is kept
> in _GLOBAL_REENT, but is marked as free.
> 5. you start another thread
> 6. it tries to open a file
> 7. __sfp() is called, which searches for a free FILE object - it will find the object
> which was closed in 4, which will be returned, so nothing would be allocated 8.
> same as 4 ...
> 
> However I must admit that for me the whole design of reentrancy in newlib is
> not very consistent and optimal, which causes a lot of memory to be wasted in a
> multithreaded application...
> 
> Regards,
> FCh

AW: Possible bug in __sfp() libc routine

[onkel.jack]

The described behaviour seems is correct my opinion.
File descriptors as well as FILE objects are not bound to a specific thread.
In a posix-like environment, file descriptors and related stuff are always at process scope.
Threads are running inside of a process, they share all the resources the process owns, 
file descriptors, global variables heap space and so on.
Terminating a thread would NOT close files, since they are owned by the process, not by the thread.
It also would make it impossible i.e. use files shared by multiple threads.
As far as I have seen in newlib code, the implementation follows these rules same way as glibc does.
So the correct implementation would be: if a thread does open a file it has to close it before terminating (or hand it over to another thread who take care of closing it ....). 
 

-----Original-Nachricht-----
Betreff: RE: Possible bug in __sfp() libc routine
Datum: 2017-04-10T21:53:19+0200
Von: "Kapania, Ashish" <akapania@ti.com>
An: "Freddie Chopin" <freddie_chopin@op.pl>, "newlib@sourceware.org" <newlib@sourceware.org>

Hi Freddie,

The steps you suggested makes sense. The FILE object should be marked as free on fclose and should be reused the next time around. In my experiments, it gets reused if I do fopen, fwrite & fclose in a loop. However, when I delete the thread and create a new thread to repeat the sequence, the very first time fwrite is called a new FILE object gets allocated instead of reusing the FILE object created by the now deleted thread. I will step through the fwrite & fclose code and try to figure out what is happening.

Best,
Ashish

> -----Original Message-----
> From: Freddie Chopin [mailto:freddie_chopin@op.pl]
> Sent: Saturday, April 08, 2017 2:04 PM
> To: Kapania, Ashish; newlib@sourceware.org
> Subject: Re: Possible bug in __sfp() libc routine
> 
> On Fri, 2017-04-07 at 21:57 +0000, Kapania, Ashish wrote:
> > Hi All,
> >
> > In the __sfp() function in "libc/findfp.c" file, I see that if no free
> > FILE object is found, one is allocated and put on a list in the global
> > re-entrancy structure (_GLOBAL_REENT). This seems like a bug to me. I
> > believe the FILE object should be put on a list in the thread specific
> > reentrancy structure. If I create a thread, do a fopen, do a fwrite
> > (invokes __sfp which in turn allocates the FILE object), do a fclose
> > and then delete the thread, the FILE object allocated by __sfp() is
> > not freed. If a do this sequence repeatedly, I see memory keeps
> > leaking until my app runs out of heap. I have a separate re-entrancy
> > structure for each thread but because the FILE object is not in a list
> > on the local re-entrancy structure, it does not get freed when I
> > delete the thread and run _reclaim_reent() on the local reentrancy
> > structure.
> >
> > Any thoughts ?
> 
> Hi!
> 
> In my understanding newlib just keeps all FILE objects in the _GLOBAL_REENT
> but this should not lead to the behaviour you observe.
> 
> The objects are indeed shared, which means that they are - unfortunately -
> never freed. But on the other hand in your scenario this should work like this:
> 
> 1. you start the first thread
> 2. it tries to open a file
> 3. __sfp() is called and sees there are no FILE objects, so some are allocated and
> the first one is returned 4. when your thread closes the file, its FILE object is kept
> in _GLOBAL_REENT, but is marked as free.
> 5. you start another thread
> 6. it tries to open a file
> 7. __sfp() is called, which searches for a free FILE object - it will find the object
> which was closed in 4, which will be returned, so nothing would be allocated 8.
> same as 4 ...
> 
> However I must admit that for me the whole design of reentrancy in newlib is
> not very consistent and optimal, which causes a lot of memory to be wasted in a
> multithreaded application...
> 
> Regards,
> FCh


RE: Possible bug in __sfp() libc routine

[Kapania, Ashish]

Reviving an old thread...

I recently ran into this issue again where my app was creating and deleting RTOS threads (that do file IO) and that was causing a memory leak. After doing some debugging, I can conclude that there is no bug in __sfp() code but the issue Martin points to in the below email is what I am seeing. Newlib nano does not close the stdio/stdout/stderr descriptors when _reclaim_reent() is called on my RTOS thread's re-entrancy structure. Therefore, every new thread I create ends up allocating a new file descriptor instead of re-using the old ones. To work around this, I am explicitly calling fclose on the thread's stdio/stdout/stderr descriptors in the thread delete hook. I think this should be done in the cleanup function by newlib. Do the maintainers and other newlib experts agree ?

Best,
Ashish

-----Original Message-----
From: Martin Velek [mailto:martin.velek@gmail.com] 
Sent: Sunday, April 9, 2017 2:23 AM
To: Kapania, Ashish
Cc: newlib@sourceware.org
Subject: Re: Possible bug in __sfp() libc routine

What about this thread?

https://sourceware.org/ml/newlib/2015/msg00619.html . I think this issue was not fixed or commented yet.

I have written some info related to newlib, reentrancy
(--enable-newlib-reent-small) and using it in FreeRTOS, however the biggest issue is that it is written in Czech language with no perspective to be translated into English. I have tried translate.google.com and it is somehow cumbersome.
https://support.dce.felk.cvut.cz/mediawiki/images/1/17/Dp_2016_velek_martin.pdf

BR
Martin

On Fri, Apr 7, 2017 at 11:57 PM, Kapania, Ashish <akapania@ti.com> wrote:
> Hi All,
>
> In the __sfp() function in "libc/findfp.c" file, I see that if no free FILE object is found, one is allocated and put on a list in the global re-entrancy structure (_GLOBAL_REENT). This seems like a bug to me. I believe the FILE object should be put on a list in the thread specific reentrancy structure. If I create a thread, do a fopen, do a fwrite (invokes __sfp which in turn allocates the FILE object), do a fclose and then delete the thread, the FILE object allocated by __sfp() is not freed. If a do this sequence repeatedly, I see memory keeps leaking until my app runs out of heap. I have a separate re-entrancy structure for each thread but because the FILE object is not in a list on the local re-entrancy structure, it does not get freed when I delete the thread and run _reclaim_reent() on the local reentrancy structure.
>
> Any thoughts ?
>
> Best,
> Ashish