Make all jep types compatible with isolated interpreters. #594
Conversation
| @@ -449,6 +462,8 @@ PyMethodDef pyjclass_methods[] = { | |||
|
|
|||
|
|
|||
| static PyMemberDef pyjclass_members[] = { | |||
| /* TODO Consider moving to managed dict as described in the documentation for tp_dictoffset */ | |||
| {"__dictoffset__", T_PYSSIZET, offsetof(PyJClassObject, attr), READONLY}, | |||
There was a problem hiding this comment.
This is the equivalent of tp_dictoffset which was used previously. The documentation for PyType_Slot mentions that no slot is provided for tp_dictoffset and recommends switching to a managed dict, but I couldn't get that working I so followed the fallback recommendation of putting the offset in the members.
| static PyObject* pyjarray_iter(PyObject *); | ||
|
|
||
| static PyType_Slot slots[] = { | ||
| {Py_tp_doc, (void*) &list_doc}, |
There was a problem hiding this comment.
I'm not sure I agree with using list_doc, though I'm not entirely sure what this is used for. If you open the jep interpreter and make a pyjarray and do .doc on it, what does it say?
There was a problem hiding this comment.
This is the only place list_doc is used and I am pretty sure it is the same behavior as before. pyjarray.doc is not a valid attribute but doc is a string with list_doc in it. The main purpose of this is so that at an interactive interpreter help(jep.jarray) shows a nice help message.
src/main/c/Objects/pyjclass.c
Outdated
| }; | ||
|
|
||
| PyType_Spec PyJClass_Spec = { | ||
| .name = "java.lang.Class", |
There was a problem hiding this comment.
I take it this was intentional that you changed the name? Will that affect anything? I noticed below that PyJObject has java.lang.Object so this change makes it more consistent at least.
There was a problem hiding this comment.
It was not intentional, thanks for pointing it out. I am actually opposed to naming this java.lang.Class because class methods like getSuperclass() don't work on PyJClass so I think I should revert the name back to jep.PyJClass.
src/main/c/Objects/pyjmultimethod.c
Outdated
| @@ -239,7 +239,8 @@ PyTypeObject PyJMultiMethod_Type = { | |||
| 0, /* tp_getattro */ | |||
| 0, /* tp_setattro */ | |||
| 0, /* tp_as_buffer */ | |||
| Py_TPFLAGS_DEFAULT, /* tp_flags */ | |||
| Py_TPFLAGS_DEFAULT | | |||
| Py_TPFLAGS_IMMUTABLETYPE, /* tp_flags */ | |||
src/main/c/Objects/pyjtype.c
Outdated
| @@ -49,7 +43,7 @@ static int staticTypesInitialized = 0; | |||
| static PyTypeObject* addCustomTypeToTypeDict(JNIEnv *env, PyObject* fqnToPyType, | |||
| jclass class, PyTypeObject *type) | |||
| { | |||
| if (PyDict_SetItemString(fqnToPyType, type->tp_name, (PyObject*) type)) { | |||
| if (PyDict_SetItemString(fqnToPyType, type->tp_name, (PyObject * ) type)) { | |||
There was a problem hiding this comment.
Is there a reason there are spaces around the * now? You did it multiple times in this file.
There was a problem hiding this comment.
astyle automatically added that in. I am using the same astyle configuration I have been using for years that I think I originally got from you but it is a newer version of astyle so it might have a slightly different default. I like the old way better so I will look into it and see if that is something I can change in the astyle configuration.
There was a problem hiding this comment.
This seems to be a bug with astyle 3.4.13, I've also tried 3.6.6 and I see the same behavior. Reverting to astyle 3.3 produces better results so I am going with that for this change.
This change makes it so that all the jep types declared in c code are compatible with sub interpreters. PyJObject and its subclasses(pyjclass and pyjarray) are converted to heap types and each interpreter now has a separate instance of those types that is stored in the module state for the _jep module. All other static types created by jep are now flagged as immutable which allows them to be shared between isolated interpreters.
In my tests this change resolves the crash seen in #593.