fcgi2 icon indicating copy to clipboard operation
fcgi2 copied to clipboard

OS_LibShutdown is not called

Open Louson opened this issue 5 years ago • 10 comments

In unix at least : fcgiapp has a call to OS_LibInit() that allocates a structure (asyncIoTable) which is never freed. I guess OS_LibShutdown should be called in FCGX_Finish().

Louson avatar Oct 21 '19 15:10 Louson

See pull request https://github.com/FastCGI-Archives/fcgi2/pull/33

Louson avatar Oct 21 '19 15:10 Louson

i'not sure is usefull to do that, because is call in FCGIexit only when exit.

in FCGX_Accept is calling FCGX_Init only if libInitialized is initialized... only at the first request...

if you call at every FCGX_Finish you force at each FCGX_Accept to reinit at each Accept...

and os init activate the signal handler and i'm not sure they are correctly coded to reenabled at each accept...

mcarbonneaux avatar Jan 09 '20 21:01 mcarbonneaux

I can confirm the initial analysis. I'm getting this with Valgrind on my app running on embedded Linux with spawn-fcgi:

==818== Memcheck, a memory error detector
==818== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==818== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==818== Command: /var/www/htdocs/cgi-bin/iswebgui.cgi
==818== 
==818== 
==818== HEAP SUMMARY:
==818==     in use at exit: 448 bytes in 1 blocks
==818==   total heap usage: 16,925 allocs, 16,924 frees, 16,994,819 bytes allocated
==818== 
==818== 448 bytes in 1 blocks are still reachable in loss record 1 of 1
==818==    at 0x4841B04: calloc (vg_replace_malloc.c:752)
==818==    by 0x48A4BCF: OS_LibInit (os_unix.c:171)
==818==    by 0x48A4085: FCGX_Init (fcgiapp.c:2087)
==818==    by 0x48A40F1: FCGX_IsCGI (fcgiapp.c:1946)
==818==    by 0x48A4365: FCGI_Accept (fcgi_stdio.c:120)
==818==    by 0x10D415: main (iswebgui.c:100)
==818== 
==818== LEAK SUMMARY:
==818==    definitely lost: 0 bytes in 0 blocks
==818==    indirectly lost: 0 bytes in 0 blocks
==818==      possibly lost: 0 bytes in 0 blocks
==818==    still reachable: 448 bytes in 1 blocks
==818==         suppressed: 0 bytes in 0 blocks
==818== 
==818== For counts of detected and suppressed errors, rerun with: -v
==818== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

The code in main() is like an ordinary CGI, extended with FCGI stuff like this:

#include <fcgi_stdio.h>
…
void main( void )
{
    while ( FCGI_Accept() >= 0 )
    {
        /* all that classic CGI stuff */
    }
    FCGI_Finish();
    return 0;
}

LeSpocky avatar Mar 31 '20 08:03 LeSpocky

but FCGX_Accept is calling FCGX_Init only if libInitialized is initialized... not at each Accept...

in fcgiapp.c :

if (! libInitialized) {
        rc = FCGX_Init();
        if (rc) {
            return rc;
        }
    }

only one time along the live of the process... and i think not need to be release at finish because you can reopen FCGI before exit... eventualy when exit... you can explicitly call os_shutdown.... but the majority of what done in shutdown are automaticly release by the end of the process (when call FCGIexit)... ok is not very clean... but it's work...

mcarbonneaux avatar May 30 '20 19:05 mcarbonneaux

IMO executing OS_LibShutdown() right before return EXIT_SUCCESS; from process is the only correct solution for now. Unfortunately this is required in some cases like CI Valgrind integraiton, which yells for every single memory leak.

It would be great if OS_LibShutdown() would be exported to fcgiapp.c/h to something like FCGX_Deinit() :slightly_smiling_face:

xdevelnet avatar Oct 20 '21 01:10 xdevelnet

FCGIexit is for that i think... they call OS_LibShutdown()....

mcarbonneaux avatar Nov 06 '21 17:11 mcarbonneaux

FCGIexit does call OS_LibShutdown, but it also terminates the process when calling exit().

It would be great to have a similar function in declared in fcgiapp.h that returns a code without exiting the process.

In my case, I'm using fcgi in a side library along my main process, I am not creating a process for it.

Louson avatar Nov 07 '21 14:11 Louson

What is the current status of this topic? I agree to have OS_LibShutdown exported somewhere. The initial commit https://github.com/FastCGI-Archives/fcgi2/commit/217069dc84216956cd7bcddc7a3d218871343e09 makes sense since FCGX_Init and FCGX_Finish are opposites, and thus easy to understand. FCGIexit is not exported, and perhaps also not desirable because applications might want to have control of the exit by themselves. Any reason to not re-open that PR and accept this commit?

ThomasDevoogdt avatar Oct 10 '22 08:10 ThomasDevoogdt

On my side, I decided to fix it by freeing the memory manually :

  FCGX_Free(&request, 0);
  OS_LibShutdown();

This implies to include fcgios.h

I am not sure this issue can have an easy fix. I understand that my commit could cause some regression. I suggest the issue remains open with this hack.

Here is an example https://gitlab.com/louson.fr/grav/-/blob/master/pages/03.papers/connect-your-firmware-with-fastcgi/fcgi-server.c

Louson avatar Oct 10 '22 11:10 Louson

FCGX_Free(&request, 0);

normaly FCGX_Finish free the request data at fcgiapp.c:2045 but leave the connection open if request->keepConnection are true (depend on the FCGI_KEEP_CONN flag send from fcgi server).

This implies to include fcgios.h

effectively the OS_LibShutdown are exposed by this include at fcgios.h:104 and possibly can be called explicitly in that way.

FCGIexit does call OS_LibShutdown, but it also terminates the process when calling exit().

FCGIexit must be used in place of exit() is there role.

if fact idealy is to call FCGIexit event if the exit status are 0 (you must call FCGIexit(0) in this case), in this case OS_LibShutdown are systematicly call before exit.

Normaly you need to call OS_LibShutdown only before exit of the process. is the way is developped...

but effectively if you need to use in side library, you must call explicitly OS_LibShutdown in using fcgios.h include.

but you can call it at library shutdown to avoid to reinit each time.

mcarbonneaux avatar Oct 10 '22 17:10 mcarbonneaux