Skip to content

Commit

Permalink
Thread-safety around get type/resolve type
Browse files Browse the repository at this point in the history
  • Loading branch information
@jmao-denver
jmao-denver committed Sep 17, 2024
1 parent 91e8567 commit 1049097
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 24 deletions.
38 changes: 19 additions & 19 deletions setup.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -73,30 +73,30 @@

# Python unit tests that just use Java runtime classes (rt.jar)
python_java_rt_tests = [
# os.path.join(src_test_py_dir, 'jpy_rt_test.py'),
# os.path.join(src_test_py_dir, 'jpy_mt_test.py'),
# os.path.join(src_test_py_dir, 'jpy_diag_test.py'),
# # os.path.join(src_test_py_dir, 'jpy_perf_test.py'),
os.path.join(src_test_py_dir, 'jpy_rt_test.py'),
os.path.join(src_test_py_dir, 'jpy_mt_test.py'),
os.path.join(src_test_py_dir, 'jpy_diag_test.py'),
os.path.join(src_test_py_dir, 'jpy_perf_test.py'),
]

# Python unit tests that require target/test-classes or target/classes
# available on the classpath
python_java_jpy_tests = [
# os.path.join(src_test_py_dir, 'jpy_array_test.py'),
# os.path.join(src_test_py_dir, 'jpy_field_test.py'),
# os.path.join(src_test_py_dir, 'jpy_retval_test.py'),
# os.path.join(src_test_py_dir, 'jpy_exception_test.py'),
# os.path.join(src_test_py_dir, 'jpy_overload_test.py'),
# os.path.join(src_test_py_dir, 'jpy_typeconv_test.py'),
# os.path.join(src_test_py_dir, 'jpy_typeconv_test_pyobj.py'),
# os.path.join(src_test_py_dir, 'jpy_typeres_test.py'),
# os.path.join(src_test_py_dir, 'jpy_modretparam_test.py'),
# os.path.join(src_test_py_dir, 'jpy_translation_test.py'),
# os.path.join(src_test_py_dir, 'jpy_gettype_test.py'),
# os.path.join(src_test_py_dir, 'jpy_reentrant_test.py'),
# os.path.join(src_test_py_dir, 'jpy_java_embeddable_test.py'),
# os.path.join(src_test_py_dir, 'jpy_obj_test.py'),
# os.path.join(src_test_py_dir, 'jpy_eval_exec_test.py'),
os.path.join(src_test_py_dir, 'jpy_array_test.py'),
os.path.join(src_test_py_dir, 'jpy_field_test.py'),
os.path.join(src_test_py_dir, 'jpy_retval_test.py'),
os.path.join(src_test_py_dir, 'jpy_exception_test.py'),
os.path.join(src_test_py_dir, 'jpy_overload_test.py'),
os.path.join(src_test_py_dir, 'jpy_typeconv_test.py'),
os.path.join(src_test_py_dir, 'jpy_typeconv_test_pyobj.py'),
os.path.join(src_test_py_dir, 'jpy_typeres_test.py'),
os.path.join(src_test_py_dir, 'jpy_modretparam_test.py'),
os.path.join(src_test_py_dir, 'jpy_translation_test.py'),
os.path.join(src_test_py_dir, 'jpy_gettype_test.py'),
os.path.join(src_test_py_dir, 'jpy_reentrant_test.py'),
os.path.join(src_test_py_dir, 'jpy_java_embeddable_test.py'),
os.path.join(src_test_py_dir, 'jpy_obj_test.py'),
os.path.join(src_test_py_dir, 'jpy_eval_exec_test.py'),
os.path.join(src_test_py_dir, 'jpy_mt_eval_exec_test.py'),
]

Expand Down
66 changes: 66 additions & 0 deletions src/main/c/jpy_jtype.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,54 @@
#include "jpy_conv.h"
#include "jpy_compat.h"

#ifdef Py_GIL_DISABLED
typedef struct {
PyMutex lock;
PyThreadState* owner;
int recursion_level;
} ReentrantLock;

static void acquire_lock(ReentrantLock* self) {
PyThreadState* current_thread = PyThreadState_Get();

if (self->owner == current_thread) {
self->recursion_level++;
return;
}

PyMutex_Lock(&(self->lock));

self->owner = current_thread;
self->recursion_level = 1;
}

static void release_lock(ReentrantLock* self) {
if (self->owner != PyThreadState_Get()) {
PyErr_SetString(PyExc_RuntimeError, "Lock not owned by current thread");
return;
}

self->recursion_level--;
if (self->recursion_level == 0) {
self->owner = NULL;
PyMutex_Unlock(&(self->lock));
}
}

static ReentrantLock get_type_rlock = {{0}, NULL, 0};
static ReentrantLock resolve_type_rlock = {{0}, NULL, 0};

#define ACQUIRE_GET_TYPE_LOCK() acquire_lock(&get_type_rlock)
#define RELEASE_GET_TYPE_LOCK() release_lock(&get_type_rlock)
#define ACQUIRE_RESOLVE_TYPE_LOCK() acquire_lock(&resolve_type_rlock)
#define RELEASE_RESOLVE_TYPE_LOCK() release_lock(&resolve_type_rlock)

#else
#define ACQUIRE_GET_TYPE_LOCK()
#define RELEASE_GET_TYPE_LOCK()
#define ACQUIRE_RESOLVE_TYPE_LOCK()
#define RELEASE_RESOLVE_TYPE_LOCK()
#endif

JPy_JType* JType_New(JNIEnv* jenv, jclass classRef, jboolean resolve);
int JType_ResolveType(JNIEnv* jenv, JPy_JType* type);
Expand All @@ -52,6 +100,8 @@ static int JType_MatchVarArgPyArgAsFPType(const JPy_ParamDescriptor *paramDescri
static int JType_MatchVarArgPyArgIntType(const JPy_ParamDescriptor *paramDescriptor, PyObject *pyArg, int idx,
struct JPy_JType *expectedComponentType);



JPy_JType* JType_GetTypeForObject(JNIEnv* jenv, jobject objectRef, jboolean resolve)
{
JPy_JType* type;
Expand Down Expand Up @@ -151,6 +201,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
return NULL;
}

ACQUIRE_GET_TYPE_LOCK();
typeValue = PyDict_GetItem(JPy_Types, typeKey);
if (typeValue == NULL) {

Expand All @@ -160,6 +211,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
type = JType_New(jenv, classRef, resolve);
if (type == NULL) {
JPy_DECREF(typeKey);
RELEASE_GET_TYPE_LOCK();
return NULL;
}

Expand All @@ -184,6 +236,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
PyDict_DelItem(JPy_Types, typeKey);
JPy_DECREF(typeKey);
JPy_DECREF(type);
RELEASE_GET_TYPE_LOCK();
return NULL;
}

Expand All @@ -195,6 +248,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
PyDict_DelItem(JPy_Types, typeKey);
JPy_DECREF(typeKey);
JPy_DECREF(type);
RELEASE_GET_TYPE_LOCK();
return NULL;
}

Expand All @@ -206,6 +260,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
PyDict_DelItem(JPy_Types, typeKey);
JPy_DECREF(typeKey);
JPy_DECREF(type);
RELEASE_GET_TYPE_LOCK();
return NULL;
}

Expand All @@ -231,6 +286,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
"jpy internal error: attributes in 'jpy.%s' must be of type '%s', but found a '%s'",
JPy_MODULE_ATTR_NAME_TYPES, JType_Type.tp_name, Py_TYPE(typeValue)->tp_name);
JPy_DECREF(typeKey);
RELEASE_GET_TYPE_LOCK();
return NULL;
}

Expand All @@ -240,6 +296,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
}

JPy_DIAG_PRINT(JPy_DIAG_F_TYPE, "JType_GetType: javaName=\"%s\", found=%d, resolve=%d, resolved=%d, type=%p\n", type->javaName, found, resolve, type->isResolved, type);
RELEASE_GET_TYPE_LOCK();

if (!type->isResolved && resolve) {
if (JType_ResolveType(jenv, type) < 0) {
Expand Down Expand Up @@ -968,7 +1025,10 @@ int JType_ResolveType(JNIEnv* jenv, JPy_JType* type)
{
PyTypeObject* typeObj;

ACQUIRE_RESOLVE_TYPE_LOCK();

if (type->isResolved || type->isResolving) {
RELEASE_RESOLVE_TYPE_LOCK();
return 0;
}

Expand All @@ -980,6 +1040,7 @@ int JType_ResolveType(JNIEnv* jenv, JPy_JType* type)
if (!baseType->isResolved) {
if (JType_ResolveType(jenv, baseType) < 0) {
type->isResolving = JNI_FALSE;
RELEASE_RESOLVE_TYPE_LOCK();
return -1;
}
}
Expand All @@ -988,24 +1049,29 @@ int JType_ResolveType(JNIEnv* jenv, JPy_JType* type)
//printf("JType_ResolveType 1\n");
if (JType_ProcessClassConstructors(jenv, type) < 0) {
type->isResolving = JNI_FALSE;
RELEASE_RESOLVE_TYPE_LOCK();
return -1;
}

//printf("JType_ResolveType 2\n");
if (JType_ProcessClassMethods(jenv, type) < 0) {
type->isResolving = JNI_FALSE;
RELEASE_RESOLVE_TYPE_LOCK();
return -1;
}

//printf("JType_ResolveType 3\n");
if (JType_ProcessClassFields(jenv, type) < 0) {
type->isResolving = JNI_FALSE;
RELEASE_RESOLVE_TYPE_LOCK();
return -1;
}

//printf("JType_ResolveType 4\n");
type->isResolving = JNI_FALSE;
type->isResolved = JNI_TRUE;

RELEASE_RESOLVE_TYPE_LOCK();
return 0;
}

Expand Down
10 changes: 5 additions & 5 deletions src/test/python/jpy_mt_eval_exec_test.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,30 @@

jpyutil.init_jvm(jvm_maxmem='512M', jvm_classpath=['target/classes', 'target/test-classes'])
import jpy
jpy.diag.flags = jpy.diag.F_TYPE
# jpy.diag.flags = jpy.diag.F_TYPE

NUM_THREADS = 8
NUM_THREADS = 20


class MultiThreadedTestEvalExec(unittest.TestCase):
def setUp(self):
self.fixture = jpy.get_type("org.jpy.fixtures.MultiThreadedEvalTestFixture")
self.assertIsNotNone(self.fixture)

def atest_inc_baz(self):
def test_inc_baz(self):
baz = 15
self.fixture.script("baz = baz + 1; self.assertGreater(baz, 15)", NUM_THREADS)
# note: this *is* correct wrt python semantics w/ exec(code, globals(), locals())
# https://bugs.python.org/issue4831 (Note: it's *not* a bug, is working as intended)
self.assertEqual(baz, 15)

def atest_exec_import(self):
def test_exec_import(self):
import sys
self.assertTrue("json" not in sys.modules)
self.fixture.script("import json", NUM_THREADS)
self.assertTrue("json" in sys.modules)

def atest_java_threading_jpy_get_type(self):
def test_java_threading_jpy_get_type(self):
self.fixture.script("j_child1_class = jpy.get_type(\"org.jpy.fixtures.CyclicReferenceChild1\");j_child2_class ="
"jpy.get_type(\"org.jpy.fixtures.CyclicReferenceChild2\")", NUM_THREADS)

Expand Down

0 comments on commit 1049097

Please sign in to comment.