As it turns out, this was found (and fixed) by an engineer here a couple
months back, I just didn't remember it. I really need to get a new
relase out. Our fix is to correctly pass on the error from
cgiwrap_writevf with nerr_pass in cgiwrap_writef.

I hadn't noticed that all the calls to cgiwrap_writef don't check the
return code, so I'll have to fix that.

I really need to get a release out, I've been waiting on a few things
people have been working on before doing it, so I'll push on them.

Brandon

On 10/27/08 raphael.huck uttered the following other thing:
> I've found the problem a long time ago, and forgot to post it.
>
> I just stumbled on my old message, so I figured I'd post a followup :)
>
> In fact the problem comes from cgiwrap_writef in cgi/cgiwrap.c:
>
> NEOERR *cgiwrap_writef (const char *fmt, ...)
> {
> va_list ap;
>
> va_start (ap, fmt);
> cgiwrap_writevf (fmt, ap);
> va_end (ap);
> return STATUS_OK;
> }
>
> cgiwrap_writevf returns a NEOERR * which isn't checked in
> cgiwrap_writef. If cgiwrap_writevf returns STATUS_OK, everything is
> fine, if not, the allocated structure is never freed. That is the
> memory leak I was talking about in my previous mail.
>
> If we have a look at the code of cgiwrap_writevf, we can see:
>
> r = GlobalWrapper.writef_cb (GlobalWrapper.data, fmt, ap);
> if (r)
> return nerr_raise_errno (NERR_IO, "writef_cb returned %d", r);
>
> Thus, if the writef callback you have given to cgiwrap_init_emu
> doesn't return 0, an error structure will be allocated and returned,
> but never handled.
>
> The quick solution is to simply have the writef callback always return
> 0: cgiwrap_writevf will then never allocate an error structure, so no
> memory leak.
>
> So instead of doing this (which triggers the memory leak):
>
> int writef_cb(void *ptr, const char *format, va_list ap)
> {
> UNUSED(ptr);
>
> return FCGI_vprintf(format, ap);
> }
>
> do this:
>
> int writef_cb(void *ptr, const char *format, va_list ap)
> {
> UNUSED(ptr);
>
> FCGI_vprintf(format, ap);
>
> return 0;
> }
>
>
> The cleaner solution would be to modify cgiwrap_writef in order to
> handle the error returned by cgiwrap_writevf.
>
> But then each call to cgiwrap_writef should have to check the return
> value, and deal with the potential error structure, or there would be
> even more memory leaks.
>
> In cgi/cgi.c there are several calls to cgiwrap_writef which don't
> check the return value...
>
> So I don't know if the clean solution is a good idea.
>
> What do you think Brandon?
>
> Thanks in advance for your advice.
>
> --Raphaël
>
> >
> > > I'll try to find more information about the leaked memory.
> > >
> > > I forgot to mention that when I comment out the following lines:
> > >
> > > err = cgi_display(cgi, "test.cst");
> > > nerr_ignore(&err);
> > >
> > > there are no longer memory leaks, so it seems to come from
> cgi_display.
> >
> > I've found more information.
> >
> > I'm using the following wrappers for FastCGI:
> >
> > /* FCGI_fread wrapper for ClearSilver CGI */
> > int read_cb(void *ptr, char *data, int size)
> > {
> > UNUSED(ptr);
> >
> > return FCGI_fread(data, sizeof(char), size, FCGI_stdin);
> > }
> >
> > /* FCGI_vprintf wrapper for ClearSilver CGI */
> > int writef_cb(void *ptr, const char *format, va_list ap)
> > {
> > UNUSED(ptr);
> >
> > return FCGI_vprintf(format, ap);
> > }
> >
> > /* FCGI_fwrite wrapper for ClearSilver CGI */
> > int write_cb(void *ptr, const char *data, int size)
> > {
> > UNUSED(ptr);
> >
> > return FCGI_fwrite((void *) data, sizeof(char), size, FCGI_stdout);
> > }
> >
> > and then I call:
> >
> > cgiwrap_init_emu(NULL, &read_cb, &writef_cb, &write_cb, NULL, NULL,
> NULL);
> >
> > The error I get is:
> >
> > in cgiwrap.c:0192:cgiwrap_writevf - writef_cb returned 27: [134]
> > Transport endpoint is not connected
> >
> >
> > Then I had a look at cgi/fcgi_hello.c and saw this line (by the way, I
> > think 'environ' should be replaced by 'envp' in this line):
> >
> > cgiwrap_init_std(argc, argv, environ);
> >
> > and added this line to my code before the call to cgiwrap_init_emu, but
> > I still have the same error.
> >
> > Then I've tried using the following from cgi/fcgi_hello.c instead:
> >
> > int cs_printf(void *ctx, const char *s, va_list args)
> > {
> > UNUSED(ctx);
> >
> > return printf(s, args);
> > }
> >
> > int cs_write(void *ctx, const char *s, int n)
> > {
> > UNUSED(ctx);
> >
> > return fwrite(s, n, 1, FCGI_stdout);
> > }
> >
> > and calling this:
> >
> > cgiwrap_init_emu(NULL, NULL, cs_printf, cs_write, NULL, NULL, NULL);
> >
> > but it makes things worse as this gives me 2 errors then:
> >
> > in cgiwrap.c:0192:cgiwrap_writevf - writef_cb returned 27: [134]
> > Transport endpoint is not connected
> >
> > in cgiwrap.c:0210:cgiwrap_write - write_cb returned 1<2488: [134]
> > Transport endpoint is not connected
> >
> >
> > --Raphael
> >
> > >> On 08/02/07 Raphaël HUCK uttered the following other thing:
> > >>> I've made a simple example to try and understand where these
> leaks come
> > >>> from:
> > >>>
> > >>> void fcgi_begin(void)
> > >>> {
> > >>> NEOERR *err;
> > >>>
> > >>> err = cgi_init(&cgi, NULL);
> > >>> nerr_ignore(&err);
> > >>> }
> > >>>
> > >>> void fcgi_end(void)
> > >>> {
> > >>> cgi_destroy(&cgi);
> > >>> }
> > >>>
> > >>> void fcgi_dispatch(void)
> > >>> {
> > >>> NEOERR *err;
> > >>>
> > >>> err = hdf_read_file(cgi->hdf, "test.hdf");
> > >>> nerr_ignore(&err);
> > >>>
> > >>> err = cgi_display(cgi, "test.cst");
> > >>> nerr_ignore(&err);
> > >>>
> > >>> return;
> > >>> }
> > >>>
> > >>> int main(int argc, char **argv)
> > >>> {
> > >>> UNUSED(argc);
> > >>> UNUSED(argv);
> > >>>
> > >>> while (FCGI_Accept() >= 0)
> > >>> {
> > >>> #ifdef DMALLOC
> > >>> unsigned long mark;
> > >>> /* get the current dmalloc position */
> > >>> mark = dmalloc_mark();
> > >>> #endif
> > >>>
> > >>> fcgi_begin();
> > >>> fcgi_dispatch();
> > >>> fcgi_end();
> > >>>
> > >>> #ifdef DMALLOC
> > >>> /*
> > >>> * log unfreed pointers that have been added to
> > >>> * the heap since mark
> > >>> */
> > >>> dmalloc_log_changed(mark,
> > >>> 1 /* log unfreed pointers */,
> > >>> 0 /* do not log freed pointers */,
> > >>> 1 /* log each pnt otherwise summary */);
> > >>> #endif
> > >>> }
> > >>>
> > >>> return 0;
> > >>> }
> > >>>
> > >>> And I get the following results with dmalloc:
> > >>>
> > >>> $ dmalloc_summarize.pl fastcgi < fastcgi.3393
> > >>> size count gross function
> > >>> 37 10036 total
> > >>> 284 35 9940 ra=0x419748
> > >>> 80 1 80 _err_alloc+64> and ends at
> 0x41974c
> > >>> <_err_alloc [neo_err.c:59]
> > >>> 16 1 16 check_resize+76> and ends at
> 0x41b8d8
> > >>> <check_resize [ulist.c:37]
> > >>>
> > >>>
> > >>>
> > >>> --Raphael
> > >>>
> > >>>> Anything that returns a NEOERR is allocating memory on error.
> You must
> > >>>> check the return value and call one of the methods to free the
> data,
> > >>>> usually one of:
> > >>>>
> > >>>> nerr_log_error
> > >>>> nerr_ignore
> > >>>> nerr_handle
> > >>>>
> > >>>> See util/neo_err.h for details.
> > >>>>
> > >>>> The uListInit one is probably from nerr_init, which allocates a
> very
> > >>>> small amount of memory once.
> > >>>>
> > >>>> Brandon
> > >>>>
> > >>>> On 07/31/07 raphael.huck uttered the following other thing:
> > >>>>> Hi,
> > >>>>>
> > >>>>> Here's what I've found with DMALLOC:
> > >>>>>
> > >>>>> $ dmalloc_summarize.pl fastcgi < fastcgi.10983
> > >>>>> size count gross function
> > >>>>> 1494 401044 total
> > >>>>> 284 1364 387376 ra=0x436818
> > >>>>> 1176 9 10584 _err_alloc+64> and ends at
> 0x43681c
> > >>>>> <_err_alloc [neo_err.c:59]
> > >>>>> 240 2 480 hcreate_r
> > >>>>> 16 4 64 check_resize+76> and ends at
> > >>>>> 0x4389a8 <check_resize [ulist.c:37]
> > >>>>> 20 3 60 uListInit+68> and ends at
> 0x438a5c
> > >>>>> <uListInit [ulist.c:61]
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> The part which leaks the most memory is around 0x436818, which is:
> > >>>>>
> > >>>>> $ mips-linux-objdump -S fastcgi | grep -A 21 -B 25 436818
> > >>>>> static NEOERR *_err_alloc(void)
> > >>>>> {
> > >>>>> 4367d0: 3c1c0fbd lui gp,0xfbd
> > >>>>> 4367d4: 279c2c10 addiu gp,gp,11280
> > >>>>> 4367d8: 0399e021 addu gp,gp,t9
> > >>>>> 4367dc: 27bdffe0 addiu sp,sp,-32
> > >>>>> 4367e0: afbf0018 sw ra,24(sp)
> > >>>>> 4367e4: afbc0010 sw gp,16(sp)
> > >>>>> NEOERR *err;
> > >>>>>
> > >>>>> if (!UseFreeList || FreeList == NULL)
> > >>>>> 4367e8: 8f828018 lw v0,-32744(gp)
> > >>>>> 4367ec: 8f878018 lw a3,-32744(gp)
> > >>>>> {
> > >>>>> err = (NEOERR *)calloc (1, sizeof (NEOERR));
> > >>>>> 4367f0: 8f9988dc lw t9,-30500(gp)
> > >>>>> 4367f4: 8c4325d0 lw v1,9680(v0)
> > >>>>> 4367f8: 2405011c li a1,284
> > >>>>> 4367fc: 10600004 beqz v1,436810 <_err_alloc+0x40>
> > >>>>> 436800: 24040001 li a0,1
> > >>>>> 436804: 8ce625cc lw a2,9676(a3)
> > >>>>> 436808: 14c0000d bnez a2,436840 <_err_alloc+0x70>
> > >>>>> 43680c: 8fbf0018 lw ra,24(sp)
> > >>>>> 436810: 0320f809 jalr t9
> > >>>>> 436814: 00000000 nop
> > >>>>> 436818: 8fbc0010 lw gp,16(sp)
> > >>>>> if (err == NULL)
> > >>>>> {
> > >>>>> ne_warn ("INTERNAL ERROR: Unable to allocate memory for
> > >>>>> NEOERR");
> > >>>>> return INTERNAL_ERR;
> > >>>>> }
> > >>>>> return err;
> > >>>>> 43681c: 00402821 move a1,v0
> > >>>>> 436820: 8f848024 lw a0,-32732(gp)
> > >>>>> 436824: 8f998aa4 lw t9,-30044(gp)
> > >>>>> 436828: 1040000f beqz v0,436868 <_err_alloc+0x98>
> > >>>>> 43682c: 24844130 addiu a0,a0,16688
> > >>>>> }
> > >>>>> else
> > >>>>> {
> > >>>>> err = FreeList;
> > >>>>> FreeList = FreeList->next;
> > >>>>> }
> > >>>>> err->flags |= NE_IN_USE;
> > >>>>> err->next = NULL;
> > >>>>> return err;
> > >>>>> }
> > >>>>>
> > >>>>> These seem to be memory leaks. Can anyone confirm?
> > >>>>>
> > >>>>> --Raphaël HUCK
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>>
> > >>>>> Yahoo! Groups Links
> > >>>>>
> > >>>>>
> > >>>>>
> > >
> > >
> > >
> > >
> > > Yahoo! Groups Links
> > >
> > >
> > >
> >
>
>
>
> ------------------------------------
>
> Yahoo! Groups Links
>
>
>
--
"The only thing nude playing cards are good for is playing solitaire."
-- Harry Anderson, PI 1/7/98
http://www.fiction.net/blong/