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

From: "Garth N. Wells" <[email-masked]>
Date: Sun, 07 Feb 2010 12:34:52 +0000

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

-------- Original Message --------
Subject: [Branch ~instant-core/instant/main] Rev 288: Fix suspected
leak in	Instant.
Date: Sun, 07 Feb 2010 12:01:13 -0000
From: [email-masked]
Reply-To: [email-masked]
To: Garth Wells <[email-masked]>

------------------------------------------------------------
revno: 288
committer: Garth N. Wells <[email-masked]>
branch nick: instant
timestamp: Sun 2010-02-07 11:51:12 +0000
message:
  Fix suspected leak in Instant.
modified:
  src/instant/cache.py

--
lp:instant
https://code.launchpad.net/~instant-core/instant/main

Your team Instant Core Team is subscribed to branch lp:instant.
To unsubscribe from this branch go to
https://code.launchpad.net/~instant-core/instant/main/+edit-subscription.

=== modified file 'src/instant/cache.py'
--- src/instant/cache.py	2009-03-20 14:10:01 +0000
+++ src/instant/cache.py	2010-02-07 11:51:12 +0000
@@ -83,8 +83,8 @@
                       "'%s' from moduleid.signature()." % moduleid)
         module = memory_cached_module(moduleid)
         if module:
-            for moduleid in moduleids:
-                place_module_in_memory_cache(moduleid, module)
+            #for moduleid in moduleids:
+            #    place_module_in_memory_cache(moduleid, module)
             return module, moduleids
         moduleids.append(moduleid)

---

From: Anders Logg <[email-masked]>
Date: Sun, 7 Feb 2010 20:35:34 +0100

> despite the signature being the same.

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.

--
Anders

> _______________________________________________
> Mailing list: https://launchpad.net/~instant
> Post to     : [email-masked]
> Unsubscribe : https://launchpad.net/~instant
> More help   : https://help.launchpad.net/ListHelp

---

From: "Garth N. Wells" <[email-masked]>
Date: Sun, 07 Feb 2010 19:43:45 +0000

> 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

---

From: Kent Andre <[email-masked]>
Date: Mon, 08 Feb 2010 22:17:30 +0100

> inserting something into the cache. It's confusing.

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