From 5701119515029b594349639fe5ef50aa1e34ff6b Mon Sep 17 00:00:00 2001 From: Ben Steffensmeier Date: Mon, 17 Jun 2024 11:41:05 -0500 Subject: [PATCH 1/2] Fix leaking attributes in java PyObject. --- src/main/c/Jep/python/jep_object.c | 1 + src/test/java/jep/test/TestGetJPyObject.java | 21 ++++++++++++++++++++ 2 files changed, 22 insertions(+) diff --git a/src/main/c/Jep/python/jep_object.c b/src/main/c/Jep/python/jep_object.c index 793bb8dd..4f239e36 100644 --- a/src/main/c/Jep/python/jep_object.c +++ b/src/main/c/Jep/python/jep_object.c @@ -109,6 +109,7 @@ JNIEXPORT void JNICALL Java_jep_python_PyObject_setAttr goto EXIT; } ret = PyObject_SetAttrString(pyObject, attrName, pyAttr); + Py_DECREF(pyAttr); if (ret == -1) { process_py_exception(env); } diff --git a/src/test/java/jep/test/TestGetJPyObject.java b/src/test/java/jep/test/TestGetJPyObject.java index a0065aed..9e92f3cd 100644 --- a/src/test/java/jep/test/TestGetJPyObject.java +++ b/src/test/java/jep/test/TestGetJPyObject.java @@ -3,6 +3,7 @@ import java.lang.reflect.UndeclaredThrowableException; import java.util.Arrays; import java.util.Deque; +import java.util.concurrent.atomic.AtomicInteger; import jep.Interpreter; import jep.JepException; @@ -113,6 +114,25 @@ public static void testSetAttr(Interpreter interp) throws JepException { } } + public static void testSetAttrRefCount(Interpreter interp) throws JepException { + AtomicInteger delCount = new AtomicInteger(); + interp.set("delCount", delCount); + interp.exec("class Simple:\n" + + " def __del__(self):\n" + + " delCount.getAndIncrement()" + ); + PyCallable simple = interp.getValue("Simple", PyCallable.class); + PyObject obj1 = simple.callAs(PyObject.class); + PyObject obj2 = simple.callAs(PyObject.class); + obj1.setAttr("member", obj2); + obj2.close(); + obj1.close(); + /* When a PyObject is deleted any attributes without other references should also be deleted. */ + if (delCount.get() != 2) { + throw new IllegalStateException("Expecting 2 deletions but got " + delCount.get()); + } + } + public static void testDelAttr(Interpreter interp) throws JepException { interp.eval(buildTestClassPython()); interp.eval("t = testclass(1, 'A String', None)"); @@ -402,6 +422,7 @@ public static void main(String[] args) throws Exception { testIdentity(interp); testGetAttr(interp); testSetAttr(interp); + testSetAttrRefCount(interp); testDelAttr(interp); testJPyCallable(interp); testToString(interp); From 7462705d46548a5ee595a266a7acd8c65f6860a2 Mon Sep 17 00:00:00 2001 From: Ben Steffensmeier Date: Sat, 13 Jul 2024 15:59:46 -0500 Subject: [PATCH 2/2] JPyObject cleanup: check for leaks in equals and consistently XDECREF is setAttr. --- src/main/c/Jep/python/jep_object.c | 2 +- src/test/java/jep/test/TestGetJPyObject.java | 15 +++++++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/main/c/Jep/python/jep_object.c b/src/main/c/Jep/python/jep_object.c index 4f239e36..7777c28d 100644 --- a/src/main/c/Jep/python/jep_object.c +++ b/src/main/c/Jep/python/jep_object.c @@ -109,12 +109,12 @@ JNIEXPORT void JNICALL Java_jep_python_PyObject_setAttr goto EXIT; } ret = PyObject_SetAttrString(pyObject, attrName, pyAttr); - Py_DECREF(pyAttr); if (ret == -1) { process_py_exception(env); } EXIT: + Py_XDECREF(pyAttr); PyEval_ReleaseThread(jepThread->tstate); release_utf_char(env, str, attrName); } diff --git a/src/test/java/jep/test/TestGetJPyObject.java b/src/test/java/jep/test/TestGetJPyObject.java index 9e92f3cd..98a16a64 100644 --- a/src/test/java/jep/test/TestGetJPyObject.java +++ b/src/test/java/jep/test/TestGetJPyObject.java @@ -114,7 +114,7 @@ public static void testSetAttr(Interpreter interp) throws JepException { } } - public static void testSetAttrRefCount(Interpreter interp) throws JepException { + public static void testRefCount(Interpreter interp) throws JepException { AtomicInteger delCount = new AtomicInteger(); interp.set("delCount", delCount); interp.exec("class Simple:\n" + @@ -122,6 +122,7 @@ public static void testSetAttrRefCount(Interpreter interp) throws JepException { " delCount.getAndIncrement()" ); PyCallable simple = interp.getValue("Simple", PyCallable.class); + /* Check for leaks in setAttr() */ PyObject obj1 = simple.callAs(PyObject.class); PyObject obj2 = simple.callAs(PyObject.class); obj1.setAttr("member", obj2); @@ -131,6 +132,16 @@ public static void testSetAttrRefCount(Interpreter interp) throws JepException { if (delCount.get() != 2) { throw new IllegalStateException("Expecting 2 deletions but got " + delCount.get()); } + delCount.set(0); + /* Check for leaks in equals() */ + obj1 = simple.callAs(PyObject.class); + obj2 = simple.callAs(PyObject.class); + obj1.equals(obj2); + obj2.close(); + obj1.close(); + if (delCount.get() != 2) { + throw new IllegalStateException("Expecting 2 deletions but got " + delCount.get()); + } } public static void testDelAttr(Interpreter interp) throws JepException { @@ -422,7 +433,7 @@ public static void main(String[] args) throws Exception { testIdentity(interp); testGetAttr(interp); testSetAttr(interp); - testSetAttrRefCount(interp); + testRefCount(interp); testDelAttr(interp); testJPyCallable(interp); testToString(interp);