[OPEN-ILS-DEV] PATCH: osrf_json_object.c (miscellaneous)

[Mike Rylander]

On Dec 9, 2007 1:10 AM, Scott McKellar <mck9 at swbell.net> wrote:
> There are two similar memory leaks in osrf_json_object.c, at lines 640
> and 669.  In each case we pass the return value of jsonObjectClone()
> to osrfAppRespond(), and never free the clone.
>
> We could capture the clone pointers and then free them.  However it
> would be both simpler and more efficient just to eliminate the layer
> of cloning by passing the original jsonObject* directly.  Thus, for
> example:
>
>     osrfAppRespond( ctx, jsonObjectClone(cur->item) );
>
> ...becomes:
>
>     osrfAppRespond( ctx, cur->item );
>

Indeed.  Great minds, and all that.

I went through last night and swept out as many instances of that
pattern as I could find.  I'm not seeing any regular leaks now, either
on the unusedObj list or directly looking at the memory footprint.  I
think there are still a few pointers slipping through, but not the KBs
and KBs per request that we were seeing initially, just a pointer per
request, and only with certain classes of requests -- it looks like
now we leak only where we build default (fake) parameters for
simplicity.

Thanks again, Scott.

> Scott McKellar
> http://home.swbell.net/mck9/ct/
>
>
> --- Scott McKellar <mck9 at swbell.net> wrote:
>
> >
> > --- Mike Rylander <mrylander at gmail.com> wrote:
> >
> > > The other part that I'm not positive about, but I believe to be
> > true
> > > (and coded for),  is that osrfAppRespond() takes care of destroying
> > > the object it is passed.  Basically, I'm not freeing anything that
> > > comes out of the ctx object (hash, and all its descendants), nor
> > > anything I return to the client.
> >
> > If you are referring to the second parameter of osrfAppRespond(),
> > which is a const jsonObject*, the answer is no -- that object doesn't
> > get freed.  In fact osrfAppRespond can't free it, at least not
> > without
> > a loathsome cast, because of the constness.
> >
> > So I think this is indeed a leak.  We call osrfAppRespond in line
> > 2433:
> >
> >         osrfAppRespond( ctx, oilsMakeJSONFromResult( result ) );
> >
> > The second parameter is a newly allocated jsonObject created by
> > oilsMakeJSONFromResult().  That object never gets freed.  We need to
> > capture the pointer in a variable, pass the variable as a parameter,
> > and then free it via jsonObjectFree():
> >
> >         jsonObject * resultObj = oilsMakeJSONFromResult( result );
> >         osrfAppRespond( ctx, resultObj ):
> >         jsonObjectFree( resultObj );
> >
> > Scott McKellar
> > http://home.swbell.net/mck9/ct/
> >
> >
> >
> >
> >
> >
>
>



-- 
Mike Rylander
 | VP, Research and Design
 | Equinox Software, Inc. / The Evergreen Experts
 | phone:  1-877-OPEN-ILS (673-6457)
 | email:  miker at esilibrary.com
 | web:  http://www.esilibrary.com