From 7144ef5c3c11f369bf0d63edff2828ce1fb3718a Mon Sep 17 00:00:00 2001 From: Ben Steffensmeier Date: Wed, 11 Dec 2024 19:34:46 -0600 Subject: [PATCH] Fix SubInterpreter pyjmethod leak on static types. --- src/main/c/Objects/pyjtype.c | 41 ++++++++++++++----- .../jep/test/util/SubInterpreterThread.java | 41 +++++++++++++++++++ src/test/python/test_regressions.py | 14 +++++++ 3 files changed, 85 insertions(+), 11 deletions(-) create mode 100644 src/test/java/jep/test/util/SubInterpreterThread.java diff --git a/src/main/c/Objects/pyjtype.c b/src/main/c/Objects/pyjtype.c index 82948e75..716ec42c 100644 --- a/src/main/c/Objects/pyjtype.c +++ b/src/main/c/Objects/pyjtype.c @@ -32,6 +32,13 @@ static PyTypeObject PyJType_Type; static PyTypeObject* pyjtype_get_cached(JNIEnv*, PyObject*, jclass); static int addMethods(JNIEnv*, PyObject*, jclass); +/* +* Flag to indicate if methods have been added to static types. Most types are +* reinitialized for each interpreter but static types are shared between +* interpreters and must be initialized only once. +*/ +static int staticTypesInitialized = 0; + /* * Populate pyjmethods for a type and add it to the cache. This is for custom * types with extra logic in c since we are not able to add pyjmethods before @@ -127,12 +134,24 @@ static int populateCustomTypeDict(JNIEnv *env, PyObject* fqnToPyType) &PyJObject_Type)) { return -1; } - /* TODO In python 3.8 buffer protocol was added to spec so pybuffer type can use a spec */ - if (!addCustomTypeToTypeDict(env, fqnToPyType, JBUFFER_TYPE, &PyJBuffer_Type)) { - return -1; - } - if (!addCustomTypeToTypeDict(env, fqnToPyType, JOBJECT_TYPE, &PyJObject_Type)) { - return -1; + if (staticTypesInitialized) { + if (PyDict_SetItemString(fqnToPyType, PyJBuffer_Type.tp_name, + (PyObject * ) &PyJBuffer_Type)) { + return -1; + } + if (PyDict_SetItemString(fqnToPyType, PyJObject_Type.tp_name, + (PyObject * ) &PyJObject_Type)) { + return -1; + } + } else { + /* TODO In python 3.8 buffer protocol was added to spec so pybuffer type can use a spec */ + if (!addCustomTypeToTypeDict(env, fqnToPyType, JBUFFER_TYPE, &PyJBuffer_Type)) { + return -1; + } + if (!addCustomTypeToTypeDict(env, fqnToPyType, JOBJECT_TYPE, &PyJObject_Type)) { + return -1; + } + staticTypesInitialized = 1; } return 0; } @@ -297,10 +316,10 @@ static int addMethods(JNIEnv* env, PyObject* dict, jclass clazz) if (isAbstract) { if (oneAbstractPyJMethod) { /* - * If there is already one abstract method and this method is also - * abstract then this isn't a functional interface and there is no need - * to keep track of abstract methods. - */ + * If there is already one abstract method and this method is also + * abstract then this isn't a functional interface and there is no need + * to keep track of abstract methods. + */ functionalInterface = JNI_FALSE; oneAbstractPyJMethod = NULL; } else { @@ -440,7 +459,7 @@ static PyTypeObject* pyjtype_get_new(JNIEnv *env, PyObject *fqnToPyType, * See https://docs.python.org/3/library/functions.html#type */ type = (PyTypeObject*) PyObject_CallFunctionObjArgs((PyObject*) &PyJType_Type, - shortName, bases, dict, NULL); + shortName, bases, dict, NULL); } Py_DECREF(bases); Py_DECREF(dict); diff --git a/src/test/java/jep/test/util/SubInterpreterThread.java b/src/test/java/jep/test/util/SubInterpreterThread.java new file mode 100644 index 00000000..3689ed19 --- /dev/null +++ b/src/test/java/jep/test/util/SubInterpreterThread.java @@ -0,0 +1,41 @@ +package jep.test.util; + +import jep.Interpreter; +import jep.JepException; +import jep.SubInterpreter; + +/** + * This is just a thread that runs some Python statements in a subinterpreter. + * It can be run from a Python unittest to verify SubInterpreter behavior + * without writing Java code. + */ +public class SubInterpreterThread extends Thread { + + private final String[] pythonStatements; + + private JepException exception; + + public SubInterpreterThread(String... pythonStatements) { + this.pythonStatements = pythonStatements; + } + + @Override + public void run() { + try (Interpreter interpreter = new SubInterpreter()) { + for (String statement : this.pythonStatements) { + interpreter.exec(statement); + } + } catch (JepException e) { + this.exception = e; + } + } + + public boolean hasException() { + return exception != null; + } + + public JepException getException() { + return exception; + } + +} diff --git a/src/test/python/test_regressions.py b/src/test/python/test_regressions.py index 4402b56b..05ee162a 100644 --- a/src/test/python/test_regressions.py +++ b/src/test/python/test_regressions.py @@ -35,3 +35,17 @@ def test_pyjtype_mro(self): self.assertEqual('ParentClassWithMethod', ChildTestingMethodInheritance().checkPrecedence()) ClassInheritingDefault = jep.findClass('jep.test.TestPyJType$ClassInheritingDefault') self.assertEqual('InterfaceWithDefault', ClassInheritingDefault().checkPrecedence()) + + def test_object_method_count(self): + # There was a bug where each new sub-interpreter would add all the + # Object methods onto multimethods on the Object type so the + # multimethod would grow with each new sub-interpreter. + from java.lang import Object + self.assertEqual(3, len(Object.wait.__methods__)) + for i in range(3): + SubInterpreterThread = jep.findClass('jep.test.util.SubInterpreterThread') + thread = SubInterpreterThread(); + thread.start(); + thread.join(); + from java.lang import Object + self.assertEqual(3, len(Object.wait.__methods__)) \ No newline at end of file