instant team mailing list archive
-
instant team
-
Mailing list archive
-
Message #00482
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