W3C home > Mailing lists > Public > www-lib@w3.org > July to September 2006

Re: Patch - plug a memory leak

From: Vic Bancroft <bancroft@america.net>
Date: Sun, 09 Jul 2006 20:58:04 -0400
Message-ID: <44B1A61C.5080209@america.net>
To: Sam Varshavchik <mrsam@courier-mta.com>
CC: www-lib@w3.org, Arthur Smith <apsmith@aps.org>

Sam Varshavchik wrote:

> valgrind comes back with a memory leak in the new version of 
> HTBound.c. When I rewrote HTBound.c last year, I copied some snippets 
> from the old version into the new version on HTBound.c, I guess 
> without fully understanding other parts of libwww.

No doubt, there is a lot to read in Library/src !

> Can't really say for sure, but the old HTBound.c might've also been 
> leaking in the same spot.  The analogous HTStreamStack call in the old 
> HTBound.c had the same parameters.

An HTStreamStack is constructed for format conversions, but most callers 
do not use the fancy HTMerge action like your invocation.  The only 
other usage of HTMerge is an interesting section in HTMIME when there is 
the explict handling of two feeds, one from the network and one from the 
cache.  It would appear that in the context of HTBound there is only one 
feed.  Egads, how did we not see this before :^?

> Anyway, this patch makes valgrind happy.  Perhaps someone who's more 
> familiar with HTMerge can share a comment, here.

Most excellent !  It is now in the HEAD as new revision: 2.16.  There 
were also some "#if 0" snippets in HTMIME.c which could use some 
trimming, giving us a new revision: 2.102 . . .

Taken together cvs diff looked like,

    Index: Library/src/HTBound.c
    ===================================================================
    RCS file: /sources/public/libwww/Library/src/HTBound.c,v
    retrieving revision 2.15
    diff -r2.15 HTBound.c
    412c412
    <                                          HTMerge(me->orig_target, 2),
    ---
     >                                          HTMerge(me->orig_target, 1),
    [bancroft@hilbert libwww]$ cvs diff lLibrary/src/HTMIME.c
    cvs [diff aborted]: no such directory `lLibrary/src'
    [bancroft@hilbert libwww]$ cvs diff Library/src/HTMIME.c
    Index: Library/src/HTMIME.c
    ===================================================================
    RCS file: /sources/public/libwww/Library/src/HTMIME.c,v
    retrieving revision 2.101
    diff -r2.101 HTMIME.c
    198,201d197
    < #if 0
    <           /* @@ JK: change */
    <           if (append) me->target = append;
    < #endif
    661c657
    <       HTTRACE(STREAM_TRACE, "Cache Flush. Flushing and freeing
    PIPE buffer\n");
    ---
     >       HTTRACE(STREAM_TRACE, "Cache Flush. Flushing PIPE buffer\n");
    663,667d658
    < #if 0
    <       /* @@ JK: flush converts the pipe to an open one, we shouldn't
    <          free it as we'll loose our references */
    <       (*pipe->isa->_free)(pipe);
    < #endif
    720,731d710
    < #if 0
    <     /* JK: this doesn't work because this work is repeated before */
    <     /*
    <     **  Create the cache append stream, and a Tee stream
    <     */
    <     {
    <       HTStream * append = HTStreamStack(WWW_CACHE_APPEND,
    output_format,
    <                                         output_stream, request, NO);
    <       if (append) me->target = HTTee(me->target, append, NULL);
    <     }
    < #endif
    <

more,
l8r,
v

BTW, is it time to revisit the release party idea as per ChangeLog 
revision 1.57 should we continue to worry over modification to address 
Ben's security concerns?  Along those lines,  did anyone come up with an 
appropriate Solaris box to construct a patch or vet a clean build as per 
the errors experienced by Arthur Smith ?

-- 
"The future is here. It's just not evenly distributed yet."
 -- William Gibson, quoted by Whitfield Diffie
Received on Monday, 10 July 2006 00:58:18 GMT

This archive was generated by hypermail 2.2.0+W3C-0.50 : Monday, 23 April 2007 18:18:45 GMT