← Back to team overview

instant team mailing list archive

Re: [Fwd: [Branch ~instant-core/instant/main] Rev 288: Fix suspected leak in Instant.]

 

On sø., 2010-02-07 at 19:43 +0000, Garth N. Wells wrote:
> 
> Anders Logg wrote:
> > On Sun, Feb 07, 2010 at 12:34:52PM +0000, Garth N. Wells wrote:
> >> Instant gurus: Take a look at this change. I hope that it's not evil.
> >>
> >> Instant was inserting an object into the memory cache even though it was
> >>  already found in the cache. It was inserting the object based on its
> >> address, and not its signature. This meant that a element or form
> >> declared in a loop would be repeatedly inserted into the memory cache,
> >> despite the signature being the same.
> >>
> >> Garth
> > 
> > I don't know much about the internals of Instant, but did you consider
> > that there are two different "signatures"? One is the thing returned
> > by __hash__ in jitobject.py and the other is the thing returned by
> > signature in the same file. The first is used for in-memory cache and
> > the second for disk cache.
> > 
> 
> The problem is that there is some code that inserts things on the basis
> if the address. Some names in the code make it unclear what's intended.
> The below is from check_memory_cache. I've added some comments.
> 
>     moduleids = [moduleid]
>     module = memory_cached_module(moduleid)
>     if module: return module, moduleids
> 
>     if hasattr(moduleid, "signature"):
> 	# GNW: Why not signature = moduleid.signature()?	
>         moduleid = moduleid.signature()
>         module = memory_cached_module(moduleid)
>         if module:
> 	    # GNW: Is moduleid meant to be 'moduleid =
>             #      moduleid.signature()' or from 'moduleids =
>             #      [moduleid]'. The below uses the address of moduleid
>             #      as passed to the function.
>             for moduleid in moduleids:
>                 place_module_in_memory_cache(moduleid, module)
>             return module, moduleids
>         moduleids.append(moduleid)
> 
> Also, I don't see what a function called 'check_memory_cache' should be
> inserting something into the cache. It's confusing.
> 
> Garth
> 

Martin wrote this code so I have not looked at it before. 
Did it solve your problem ? 

There is a return statement earlier in this function that should prevent
insertion, shouldn't it ? 

Kent






References