Skip to content

Commit

Permalink
More thread-safety change/refactoring
Browse files Browse the repository at this point in the history
  • Loading branch information
jmao-denver committed Sep 23, 2024
1 parent a8008de commit 6ed6cb6
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 9 deletions.
24 changes: 21 additions & 3 deletions src/main/c/jni/org_jpy_PyLib.c
Original file line number Diff line number Diff line change
Expand Up @@ -499,12 +499,18 @@ void dumpDict(const char* dictName, PyObject* dict)

size = PyDict_Size(dict);
printf(">> dumpDict: %s.size = %ld\n", dictName, size);
#ifdef Py_GIL_DISABLED
Py_BEGIN_CRITICAL_SECTION(dict);
#endif
while (PyDict_Next(dict, &pos, &key, &value)) {
const char* name;
name = JPy_AS_UTF8(key);
printf(">> dumpDict: %s[%ld].name = '%s'\n", dictName, i, name);
i++;
}
#ifdef Py_GIL_DISABLED
Py_END_CRITICAL_SECTION();
#endif
}

/**
Expand All @@ -521,7 +527,7 @@ PyObject *getMainGlobals() {
}

pyGlobals = PyModule_GetDict(pyMainModule); // borrowed ref
JPy_INCREF(pyGlobals);
JPy_XINCREF(pyGlobals);

return pyGlobals;
}
Expand Down Expand Up @@ -557,7 +563,7 @@ JNIEXPORT jobject JNICALL Java_org_jpy_PyLib_getCurrentGlobals

JPy_BEGIN_GIL_STATE

#if PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION <= 12
#if PY_VERSION_HEX < 0x030D0000
globals = PyEval_GetGlobals(); // borrowed ref
JPy_XINCREF(globals);
#else
Expand Down Expand Up @@ -588,7 +594,7 @@ JNIEXPORT jobject JNICALL Java_org_jpy_PyLib_getCurrentLocals

JPy_BEGIN_GIL_STATE

#if PY_MAJOR_VERSION == 3 && PY_MINOR_VERSION <= 12
#if PY_VERSION_HEX < 0x030D0000
locals = PyEval_GetLocals(); // borrowed ref
JPy_XINCREF(locals);
#else
Expand Down Expand Up @@ -1124,7 +1130,11 @@ JNIEXPORT void JNICALL Java_org_jpy_PyLib_incRef
if (Py_IsInitialized()) {
JPy_BEGIN_GIL_STATE

#if PY_VERSION_HEX < 0x030D0000
refCount = pyObject->ob_refcnt;
#else
refCount = Py_REFCNT(pyObject);
#endif
JPy_DIAG_PRINT(JPy_DIAG_F_MEM, "Java_org_jpy_PyLib_incRef: pyObject=%p, refCount=%d, type='%s'\n", pyObject, refCount, Py_TYPE(pyObject)->tp_name);
JPy_INCREF(pyObject);

Expand All @@ -1150,7 +1160,11 @@ JNIEXPORT void JNICALL Java_org_jpy_PyLib_decRef
if (Py_IsInitialized()) {
JPy_BEGIN_GIL_STATE

#if PY_VERSION_HEX < 0x030D0000
refCount = pyObject->ob_refcnt;
#else
refCount = Py_REFCNT(pyObject);
#endif
if (refCount <= 0) {
JPy_DIAG_PRINT(JPy_DIAG_F_ALL, "Java_org_jpy_PyLib_decRef: error: refCount <= 0: pyObject=%p, refCount=%d\n", pyObject, refCount);
} else {
Expand Down Expand Up @@ -1183,7 +1197,11 @@ JNIEXPORT void JNICALL Java_org_jpy_PyLib_decRefs
buf = (*jenv)->GetLongArrayElements(jenv, objIds, &isCopy);
for (i = 0; i < len; i++) {
pyObject = (PyObject*) buf[i];
#if PY_VERSION_HEX < 0x030D0000
refCount = pyObject->ob_refcnt;
#else
refCount = Py_REFCNT(pyObject);
#endif
if (refCount <= 0) {
JPy_DIAG_PRINT(JPy_DIAG_F_ALL, "Java_org_jpy_PyLib_decRefs: error: refCount <= 0: pyObject=%p, refCount=%d\n", pyObject, refCount);
} else {
Expand Down
6 changes: 2 additions & 4 deletions src/main/c/jpy_jmethod.c
Original file line number Diff line number Diff line change
Expand Up @@ -799,8 +799,7 @@ JPy_JMethod* JOverloadedMethod_FindMethod0(JNIEnv* jenv, JPy_JOverloadedMethod*
overloadedMethod->declaringClass->javaName, JPy_AS_UTF8(overloadedMethod->name), overloadCount, argCount);

for (i = 0; i < overloadCount; i++) {
// borrowed reference but no need to replace it with PyList_GetItemRef(), because the list won't be
// changed concurrently
// borrowed ref, no need to replace with PyList_GetItemRef() because the list won't be changed concurrently
currMethod = (JPy_JMethod*) PyList_GetItem(overloadedMethod->methodList, i);

if (currMethod->isVarArgs && matchValueMax > 0 && !bestMethod->isVarArgs) {
Expand Down Expand Up @@ -952,8 +951,7 @@ int JOverloadedMethod_AddMethod(JPy_JOverloadedMethod* overloadedMethod, JPy_JMe
// we need to insert this before the first varargs method
Py_ssize_t size = PyList_Size(overloadedMethod->methodList);
for (ii = 0; ii < size; ii++) {
// borrowed reference but no need to replace it with PyList_GetItemRef(), because the list won't be
// changed concurrently
// borrowed ref, no need to replace with PyList_GetItemRef() because the list won't be changed concurrently
PyObject *check = PyList_GetItem(overloadedMethod->methodList, ii);
if (((JPy_JMethod *) check)->isVarArgs) {
// this is the first varargs method, so we should go before it
Expand Down
14 changes: 13 additions & 1 deletion src/main/c/jpy_jobj.c
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,28 @@ PyObject* JObj_FromType(JNIEnv* jenv, JPy_JType* type, jobject objectRef)

// we check the type translations dictionary for a callable for this java type name,
// and apply the returned callable to the wrapped object
callable = PyDict_GetItemString(JPy_Type_Translations, type->javaName);
#ifdef Py_GIL_DISABLED
// if return is 1, callable is new reference
if (PyDict_GetItemStringRef(JPy_Type_Translations, type->javaName, &callable) != 1) {
callable = NULL;
}
#else
callable = PyDict_GetItemString(JPy_Type_Translations, type->javaName); // borrowed reference
JPy_XINCREF(callable);
#endif

if (callable != NULL) {
if (PyCallable_Check(callable)) {
callableResult = PyObject_CallFunction(callable, "OO", type, obj);
JPy_XDECREF(callable);
if (callableResult == NULL) {
Py_RETURN_NONE;
} else {
return callableResult;
}
}
}
JPy_XDECREF(callable);

return (PyObject *)obj;
}
Expand All @@ -103,6 +114,7 @@ int JObj_init_internal(JNIEnv* jenv, JPy_JObj* self, PyObject* args, PyObject* k

type = ((PyObject*) self)->ob_type;

// borrowed ref, no need to replace with PyDict_GetItemStringRef because tp_dict won't be changed concurrently
constructor = PyDict_GetItemString(type->tp_dict, JPy_JTYPE_ATTR_NAME_JINIT);
if (constructor == NULL) {
PyErr_SetString(PyExc_RuntimeError, "no constructor found (missing JType attribute '" JPy_JTYPE_ATTR_NAME_JINIT "')");
Expand Down
15 changes: 14 additions & 1 deletion src/main/c/jpy_jtype.c
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ JPy_JType* JType_GetType(JNIEnv* jenv, jclass classRef, jboolean resolve)
}

ACQUIRE_GET_TYPE_LOCK();
// borrowed ref, no need to replace with PyDict_GetItemRef because it protected by the lock
typeValue = PyDict_GetItem(JPy_Types, typeKey);
if (typeValue == NULL) {

Expand Down Expand Up @@ -1081,19 +1082,29 @@ jboolean JType_AcceptMethod(JPy_JType* declaringClass, JPy_JMethod* method)
PyObject* callableResult;

//printf("JType_AcceptMethod: javaName='%s'\n", overloadedMethod->declaringClass->javaName);
#ifdef Py_GIL_DISABLED
// if return is 1, callable is new reference
if (PyDict_GetItemStringRef(JPy_Type_Callbacks, declaringClass->javaName, &callable) != 1) {
callable = NULL;
}
#else
callable = PyDict_GetItemString(JPy_Type_Callbacks, declaringClass->javaName); // borrowed reference
JPy_XINCREF(callable);
#endif

callable = PyDict_GetItemString(JPy_Type_Callbacks, declaringClass->javaName);
if (callable != NULL) {
if (PyCallable_Check(callable)) {
callableResult = PyObject_CallFunction(callable, "OO", declaringClass, method);
if (callableResult == Py_None || callableResult == Py_False) {
JPy_XDECREF(callable);
return JNI_FALSE;
} else if (callableResult == NULL) {
JPy_DIAG_PRINT(JPy_DIAG_F_TYPE, "JType_AcceptMethod: warning: failed to invoke callback on method addition\n");
// Ignore this problem and continue
}
}
}
JPy_XDECREF(callable);

return JNI_TRUE;
}
Expand Down Expand Up @@ -1493,6 +1504,7 @@ int JType_AddMethod(JPy_JType* type, JPy_JMethod* method)
return -1;
}

// borrowed ref, no need to replace with PyDict_GetItemRef because typeDict won't be changed concurrently
methodValue = PyDict_GetItem(typeDict, method->name);
if (methodValue == NULL) {
overloadedMethod = JOverloadedMethod_New(type, method->name, method);
Expand Down Expand Up @@ -1520,6 +1532,7 @@ PyObject* JType_GetOverloadedMethod(JNIEnv* jenv, JPy_JType* type, PyObject* met
return NULL;
}

// borrowed ref, no need to replace with PyDict_GetItemRef because typeDict won't be changed concurrently
methodValue = PyDict_GetItem(typeDict, methodName);
if (methodValue == NULL) {
if (useSuperClass) {
Expand Down

0 comments on commit 6ed6cb6

Please sign in to comment.