Status of JSR223 support

[fabrice-ducos]

Hi everyone, thumbs up for the great work.

I am wondering if JPy is currently JSR223 compliant.

The ScriptEngine and ScriptEngineFactory implementations seem to be there. But no JPy engine is detected by the ScriptEngineManager when one tries to load it (I know it, because I can detect many other script engines without problem).

When checking at the jpy-0.10.0-SNAPSHOT.jar content, there is no META-INF/services/javax.script.ScriptEngineFactory, required for detection by the ScriptEngineManager.

Is it an oversight or on purpose?

[devinrsmith]

I think @forman can speak to that. Looking at the javadoc I see this:

Note, jpy's JSR 223 support is not yet functional. This class is only used by unit-tests so far.

[forman]

Well, it's been long time ago... but I think @devinrsmith is right. The JSR 223 support hasn't been fully implemented yet. But I believe, this wouldn't be too much work.

[fabrice-ducos]

Hi folks,

not sure if you have got any progress on this.

During a night of insomnia (caused by the current heat wave), I managed to launch a JSR223 compliant JPy interpreter (tested with the standard jrunscript JDK tool) with hopefully minimal changes in the code base. Basically:

• I added the missing META-INF/services/javax.script.ScriptEngineFactory, required by the JSR223 specification
• I updated jsr223/ScriptEngineImpl.java, in order to load the proper native library paths in the JVM at class loading time (in the static initialization block)
• I updated jsr223/ScriptEngineFactoryImpl.java, adding "jpy" to the list of engine names, to avoid collision of engine names with other JSR223 Python implementations, e.g. jython (in the rare cases of applications launching several implementations of the same language), and to ensure that the ScriptManager will load JPy and not another Python/JVM implementation (if several are available in the classpath).

All in all, there must be less than 10 lines of difference with the official code base. You can review them at https://github.com/fabrice-ducos/jpy/tree/jsr223

In a separate branch https://github.com/fabrice-ducos/jpy/tree/jsr223-jpyinterp, I implemented a quick-and-dirty standalone JPy interpreter (jpyinterp.sh) based on jrunscript (a tool available in all JDK6+ installations, under $JAVA_HOME/bin). Since I do not master the way JPy configures its installation path, I had to look for Python and JPy native libraries in rather hackish ways. This is just for experimentation, so I preferred to keep it separate from the jsr223 branch. But it works on Linux and MacOSX (provided it is run from the jpy directory itself).

$ ./jpyinterp.sh 
cpython> print("Hello Jpy")
Hello Jpy
None
cpython>

Tested successfully on:

• MacOS 12.4 Monterey with Oracle JDK8 1.8.0.181 and Python 3.10.1
• Ubuntu 21.10 with OpenJDK17 and Python 3.9.7

[devinrsmith]

Thanks for the work. I do have some concerns about the best way to initialize python. I see that the constructor of ScriptEngineImpl calls PyLib.startPython - I wonder if it makes more sense to move PyLibInitializer.initPyLib and PyLib.startPython right before ScriptEngineImpl is constructed in org.jpy.jsr223.ScriptEngineFactoryImpl#getScriptEngine?

[fabrice-ducos]

Hi Devin,

thank you for your answer! I didn't touch ScriptEngineImpl and left the call to PyLib.startPython where it was. This was more an experiment of feasibility than a definitive proposal. PyLibInitializer.initPyLib didn't have to be called more than once (in my understanding), so an initialization block made sense to me. Now I am completely new to this project, and I do not have a global vision to give you a worthy opinion.

Feel free to rearrange the code as you see fit, for the good of the project.

[fabrice-ducos]

After playing a bit more with the experimental jpyinterp.sh interpreter, it looks like there is a bit more work than expected.

While pure Python statements seem to work, the jpy module (for interaction between the JVM and Python) seems to have issues.

Here is a session borrowed from jpy 0.9.0 at https://jpy.readthedocs.io/en/latest/reference.html, and tested with the current version jpy 0.12.0

$ ./jpyinterp.sh 
cpython> import jpy
None
cpython> String = jpy.get_type('java.lang.String')
None
cpython> s = String('Hello jpy!')
java.lang.RuntimeException: Error in Python interpreter:
Type: <class 'TypeError'>
Value: 'java.lang.Class' object is not callable
Line: 1
Namespace: <module>
File: <string>
Traceback (most recent call last):
  File "<string>", line 1, in <module>

	at org.jpy.PyLib.executeCode(Native Method)
	at org.jpy.PyObject.executeCode(PyObject.java:138)
	at org.jpy.jsr223.ScriptEngineImpl.eval(ScriptEngineImpl.java:118)
	at javax.script.AbstractScriptEngine.eval(AbstractScriptEngine.java:264)
	at com.sun.tools.script.shell.Main.evaluateString(Main.java:298)
	at com.sun.tools.script.shell.Main.processSource(Main.java:267)
	at com.sun.tools.script.shell.Main.access$100(Main.java:37)
	at com.sun.tools.script.shell.Main$1.run(Main.java:183)
	at com.sun.tools.script.shell.Main.main(Main.java:48)

[devinrsmith]

I've added a few commits on top of your branch, let me know what you think - see https://github.com/devinrsmith/jpy/tree/jsr223-jpyinterp. I think we could get the script engine changes merged soon if all looks good.

[fabrice-ducos]

Looks good for me! Just not sure about the rationale behind the volatile modifier you added at ScriptEngineFactoryImpl.java:46

While I am sure that you added it for a very good reason, maybe a short comment would be welcome for explaining its necessity?

[devinrsmith]

Mis-implemented double-checked locking patterns are pretty common - here's an article about it: https://www.cs.umd.edu/~pugh/java/memoryModel/DoubleCheckedLocking.html. Arguably, could be fixed as part of it's own issue / PR, but I've just gone ahead and fixed it here.

[fabrice-ducos]

Thanks. Personally, I like to put a reference (under the form of a short comment, e.g. your reply in the former post, with the link) in the code itself (instead of a commit message) for this kind of clever implementation (not everyone is knowledgeable of these patterns). Of course, the link may become stale, but it is clear enough to be easy to find again (and to be updated) with a search engine.

[devinrsmith]

I've added an additional commit with inline comments; as well as improved the getScriptEngine call.

[fabrice-ducos]

Great! I think it's clear enough now.

While looking at the PyLib_CallAndReturnObject function from src/main/c/jni/org_jpy_PyLib.c,

I unearthed a commented piece of code:

  // Check why: for some reason, we don't need the following code to invoke object methods.
   /*
   if (isMethodCall) {
       PyObject* pyMethod;

       pyMethod = PyMethod_New(pyCallable, pyObject);
       if (pyMethod == NULL) {
           JPy_DIAG_PRINT(JPy_DIAG_F_EXEC, "PyLib_CallAndReturnObject: error: callable '%s': no memory\n", nameChars);
           PyLib_HandlePythonException(jenv);
           goto error;
       }
       JPy_DECREF(pyCallable);
       pyCallable = pyMethod;
   }
   */

Reenabling this snippet (as is, without change) does no good.
Unexpectedly, isMethodCall is true even on a statement like print("Hello")

Any chance that it be related with the issue on the jpy module that was illustrated in my post of the 15 june?

cpython> import jpy
None
cpython> String = jpy.get_type('java.lang.String')
None
cpython> s = String('Hello jpy!')
java.lang.RuntimeException: Error in Python interpreter:
Type: <class 'TypeError'>
Value: 'java.lang.Class' object is not callable

[devinrsmith]

Can you link to the issue?

I think the implementation of ScriptEngineImpl may need to be changed to call into python "correctly". Also, I'm not sure how it's decided which method to call into wrt javax.script.ScriptEngine and javax.script.Invocable.

[fabrice-ducos]

By linking to the issue, do you mean this ?

[devinrsmith]

Oh sorry, I thought you meant a different issue. The object is not callable I think can be solved, but I think will take some debugging down into ScriptEngineImpl and maybe more knowledge on my part about JSR223.

When running interactively via python, it looks like this:

>>> import jpy
>>> String = jpy.get_type('java.lang.String')
>>> s = String('Hello jpy!')
>>> s
java.lang.String(objectRef=0x5568d3c6a7e0)

[fabrice-ducos]

Maybe Using Java from Scripts can help.