Fixed weakref leak in calling functions with kwargs by nulling out after call (in compiler). Fixes #1586. Bumped bytecode magic

jimbaker · jimbaker · commit 83d6bbbb7773 · 2010-04-26T14:27:38.000Z
diff --git a/Lib/test/test_weakref_jy.py b/Lib/test/test_weakref_jy.py
@@ -5,6 +5,7 @@
 import unittest
 import weakref
 from test import test_support
+from test_weakref import extra_collect
 
 class ReferencesTestCase(unittest.TestCase):
 
@@ -38,8 +39,45 @@ def __hash__(self):
         self.assertEqual(len(hash_called), 2)
 
 
+class ArgsTestCase(unittest.TestCase):
+
+    # XXX consider adding other tests for dict, list, etc
+
+    def test_python_fn_kwargs(self):
+
+        weakrefs = []
+        sentinel = []
+
+        def watch(obj, kwarg=True):
+            self.assertEqual(kwarg, True)
+            # log the death of the reference by appending to the sentinel
+            ref = weakref.ref(obj, sentinel.append)
+            weakrefs.append(ref)
+
+        self.assert_(not sentinel)
+
+        thunk1 = lambda: None
+        watch(thunk1)
+        self.assert_(not sentinel)
+
+        del thunk1
+        extra_collect()
+        self.assert_(sentinel)
+
+        del sentinel[:]
+
+        thunk2 = lambda: None
+        watch(thunk2, kwarg=True)  # <--- only difference: called with a kwarg
+        self.assert_(not sentinel)
+
+        del thunk2
+        extra_collect()
+        self.assert_(sentinel)
+
+
+
 def test_main():
-    test_support.run_unittest(ReferencesTestCase)
+    test_support.run_unittest(ReferencesTestCase, ArgsTestCase)
 
 
 if __name__ == '__main__':
diff --git a/src/org/python/compiler/CodeCompiler.java b/src/org/python/compiler/CodeCompiler.java
@@ -1804,27 +1804,26 @@ public Object visitCall(Call node) throws Exception {
 
             code.aload(argArray);
             code.aload(strArray);
-            code.freeLocal(argArray);
             code.freeLocal(strArray);
             code.dup2_x2();
             code.pop2();
 
             stackConsume(3); // target + starargs + kwargs
-
             code.invokevirtual(p(PyObject.class), "_callextra", sig(PyObject.class,
                     PyObject[].class, String[].class, PyObject.class, PyObject.class));
+            freeArray(argArray);
         } else if (keys.size() > 0) {
             loadThreadState();
             stackProduce(p(ThreadState.class));
             int argArray = makeArray(values);
             int strArray = makeStrings(code, keys);
             code.aload(argArray);
             code.aload(strArray);
-            code.freeLocal(argArray);
             code.freeLocal(strArray);
             stackConsume(2); // target + ts
             code.invokevirtual(p(PyObject.class), "__call__", sig(PyObject.class, ThreadState.class,
                     PyObject[].class, String[].class));
+            freeArray(argArray);
         } else {
             loadThreadState();
             stackProduce(p(ThreadState.class));
diff --git a/src/org/python/core/imp.java b/src/org/python/core/imp.java
@@ -21,7 +21,7 @@ public class imp {
 
     private static final String UNKNOWN_SOURCEFILE = "<unknown>";
 
-    private static final int APIVersion = 28;
+    private static final int APIVersion = 29;
 
     public static final int NO_MTIME = -1;
 
diff --git a/src/org/python/modules/thread/PyLocal.java b/src/org/python/modules/thread/PyLocal.java
@@ -59,7 +59,9 @@ final void _local___init__(PyObject[] args, String[] keywords) {
         if (where[0] == TYPE && args.length > 0) {
             throw Py.TypeError("Initialization arguments are not supported");
         }
-        this.args = args;
+        // caller zeros out `args`
+        this.args = new PyObject[args.length];
+        System.arraycopy(args, 0, this.args, 0, args.length);
         this.keywords = keywords;
     }
 

Original file line number	Diff line number	Diff line change
`@@ -59,7 +59,9 @@ final void _local___init__(PyObject[] args, String[] keywords) {`
`59`	`59`	`if (where[0] == TYPE && args.length > 0) {`
`60`	`60`	`throw Py.TypeError("Initialization arguments are not supported");`
`61`	`61`	`}`
`62`		`- this.args = args;`
	`62`	+ // caller zeros out `args`
	`63`	`+ this.args = new PyObject[args.length];`
	`64`	`+ System.arraycopy(args, 0, this.args, 0, args.length);`
`63`	`65`	`this.keywords = keywords;`
`64`	`66`	`}`
`65`	`67`