Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix SubInterpreter pyjmethod leak on static types. #579

Merged
merged 1 commit into from
Dec 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 30 additions & 11 deletions src/main/c/Objects/pyjtype.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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);
Expand Down
41 changes: 41 additions & 0 deletions src/test/java/jep/test/util/SubInterpreterThread.java
Original file line number Diff line number Diff line change
@@ -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;
}

}
14 changes: 14 additions & 0 deletions src/test/python/test_regressions.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__))