Libraries for JSR223 Scripting do not work with Jython 2.7.1

[spacemanspiff2007]

Libraries do not work using JSR223 Scripting and Jython 2.7.1.
Every script is loaded using a fresh jython interpreter,
thus it is not possible to use libraries and and share data between scripts.
Correctly every script should be loaded with the same jython interpreter.

Steps to reproduce:

• Download Openhab2.3 from https://www.openhab.org/
• Copy Jython Standalone to Openhab2\runtime\lib\boot
  Use 2.7.1 to reproduce problem, use 2.7.0 to see it work
• unzip attached file automation.zip into Folder Openhab2\conf\
• Edit Openhab2\runtime\bin\setenv.bat Line 118:
  Insert:
  -Dpython.path=%OPENHAB_CONF%/automation/lib ^
  to add library path to JAVA_OPTS
• Start openhab (start.bat)
• See https://www.openhab.org/docs/tutorial/1sttimesetup.html and select Expert
• Activate JSR223 scripting (Next-Gen Rule Engine)
  https://www.openhab.org/docs/configuration/rules-ng.html#installation
• Stop Openhab and restart and watch output in console window
  Scripts are located in automation/jsr223
  Library is located in automation/lib

The Library sets a property and prints the status:

   print( 'TESTPROP: {}'.format(hasattr(testModule, 'TESTPROP')))
   testModule.TESTPROP = 'TEST'
   print( 'TESTPROP: {}'.format(hasattr(testModule, 'TESTPROP')))

Correct Output (reproducible with jython 2.7.0 which was behaving not correct with imports and multiple interpreters):

   TESTPROP: 0
   TESTPROP: 1

Incorrect Output (jython 2.7.1 which behaves now correct):

   TESTPROP: 0
   TESTPROP: 1
   
   TESTPROP: 0
   TESTPROP: 1

Analysis from Jython-Team:
http://bugs.jython.org/issue2700

[kaikreuzer]

I remember someone referring to that issue a while ago already, but I couldn't find a link to that anymore :-/
It sounds like a serious problem for Jython scripting support and I don't know how it could be solved.
I hope that someone who uses Jython scripting will be able to analyse this and come up with a PR that addresses the issue.

[spacemanspiff2007]

Do you mean this one?

[kaikreuzer]

Ah, right, thanks for the link!

[kaikreuzer]

@smerschjohann & @lewie Could anyone of you provide some wisdom here?

[diijkstra]

If Jython's team analysis is correct (especially this message), I think that Openhab/ESH is creating a new instance of script engine for each script file. Take a look at the createScriptEngine() in ScriptEngineManagerImpl. It will always create a new script engine instance from engineProvider (at least I think - I'm no Java man).

I've tried to hack it, to always return same engine instance for python scripts and @spacemanspiff2007 example was running fine. Maybe you need dedicated factory for python backend which will run code always in the same interpreter?

[5iver]

I've tried to hack it, to always return same engine instance for python scripts and @spacemanspiff2007 example was running fine.

@diijkstra , could you please post a gist of your hack? Maybe we can turn it in to something more than a hack. 😄

[diijkstra]

@openhab-5iver To be honest it was sooo dirty that my conscience forced me to nuke it ;-) If I remember correctly, ScriptEngineManagerImpl::createScriptEngine() was always reusing stuff for 'py' extension. I will try to post something once I'm back from my little vacation.

But to be honest, I would love to hear from ESH guys, why they think that scripts should work on separate contexts. It maybe makes sense for pure rule centric scripts, but kills library-like scripts (custom handlers and stuff).

[kaikreuzer]

I would love to hear from ESH guys, why they think that scripts should work on separate contexts.

Not sure who you exactly refer to. The jsr233 support was originally implemented by @smerschjohann, but I haven't heard anything from him for a while :-( .
An argument for separate contexts is probably the better isolation of different rules (and with it the reduction of unexpected side effects). At least that's how it is done in the Xbase-rules: All rules within one file share a context and can thus interwork with each other, while separate files are kept in separate contexts, which improves isolation.

If this concept cannot be applied to Jython (or any jsr233) rules, so be it and I'd personally be fine for a different solution for them (like using a shared context for them all).

[diijkstra]

@kaikreuzer Thank you, this is what I wanted to know, if you are fine with this hack. I know, that isolation should be proper way to go, but that would require (I think) ability to somehow share 'services' from libraries, or initialize everything in each script (with subtle difference among scripts).

@openhab-5iver If you can, you can try this patch. It is simple specialization (with copy-paste style) of script engine factory for jython. It will only create one instance and reuse it. For @spacemanspiff2007 example it will print out:

2019-01-14 16:43:56.951 [INFO ] [.a.m.s.r.i.l.ScriptFileWatcher:170  ] - Loading script 'loadA.py'
Loading testModule
TestModule =  645524324
TESTPROP: 0
TESTPROP: 1
Loaded testModule
A Loaded
2019-01-14 16:43:57.168 [DEBUG] [.a.m.s.r.i.l.ScriptFileWatcher:178  ] - Script loaded: loadA.py
2019-01-14 16:43:57.169 [INFO ] [.a.m.s.r.i.l.ScriptFileWatcher:170  ] - Loading script 'loadB.py'
B Loaded

So a module is loaded only once on 2.7.1 :-)

I have not tested it on production server, till than please consider it as a 'proof of concept' rather than proper PR. Use at own risk ;-)

[5iver]

I made the jar, stopped OH, swapped in 2.7.1 standalone, cleared the OH cache, and it appears to be working. I noticed two oddities though.

1. It seems like the order that modules and scripts changed between 2.7.0 and 2.7.1. With 2.7.0, scripts loaded alphabetically, without regard to the name of the parent directory. With 2.7.1, the scripts also loaded alphabetically, but first by directory name and then by file name. This caused an error, since my one script was using a module that used a component script which had not loaded yet. Manually saving the script (after the component had loaded), loaded the script successfully. This will require a change to the directory structure for the openhab2-jython modules, and an update to the docs.

2. Soon after restarting OH, I saw a few errors, where my lighting rule could not execute the action. Once, it threw this traceback...

  File "/opt/openhab2/conf/automation/lib/jython/core/log.py", line 43, in wrapper
    return fn(*args, **kwargs)
  File "<script>", line 60, in areaTrigger
SystemError: __getattribute__ not found on type GroupItemStateChangedEvent. See http://bugs.jython.org/issue2487 for details.

The linked bug report (!), claims this is due to slow hardware performance, which is accurate, since OH had just restarted and causes almost 100% CPU utilization. The issue is also claimed to be fixed in 2.7.1.patch2609, which I have not tested yet.

[smerschjohann]

Actually you can share instances between different jython instances. If you want to use libraries you can do it like this: https://github.com/smerschjohann/docker-oh2-jsr223/blob/master/run/conf/automation/jsr223/0oh1compat.py#L210

This file registeres different triggers and objects which are made available to all scripting contexts.

If you share the same ScriptEngine on multiple files you may run into errors if you are loading scripts with errors. At some point the whole script engine might fail. That was the main reason behind this isolation.

[5iver]

What I had reported in (1) about script loading order appears to be the same behavior as in 2.7.0. Consensus has been to change the script loading order, so I'll open a new issue for this.

Using jython-standalone-2.7.1.patch2618.jar, the errors I reported earlier (2) have stopped.

Using a single script engine instance, as is done in @diijkstra 's solution, comes with some rather large side effects. All imports, local variables, and functions (and imports, IIRC) in one script are now accessible in all of the others. So, this solution corrects the issue caused by the changes in 2.7.1, by preserving the state of modules across all scripts, but people will need to be educated on the shared context. For example...

# Script_1.py
from core.log import logging, LOG_PREFIX
log = logging.getLogger(LOG_PREFIX + ".Script1")
log.debug("Test1")

# Script_2.py
from core.log import logging, LOG_PREFIX
log = logging.getLogger(LOG_PREFIX + ".Script2")
log.debug("Test2")

Using a single script engine, and based on how scripts are currently loaded, Script2 will load after Script_1, so the import is redundant. If you use the log variable in Script1 after Script2 is loaded, it will have the definition from Script_2. Something like this can be easily remedied by naming the variable something different, like log1 and log2.

This presents a large issue though, which I haven't thought of a solution for. When there are multiple scripts that use scriptLoaded or scriptUnloaded, these functions are being redefined. The component scripts use these, and whenever I load/unload a script, even ones without these functions, the scriptLoaded/scriptUnloaded from the last script where it was defined will run, usually with errors, because they'll be referencing something that does not exist in the context of the current script. So, this solution introduces a bug in critical functionality.

BUT... there is a solution that does not require using a single script engine! I had been playing with a ScriptExtensionProvider, based on the suggestion from @smerschjohann, and noticed that attributes set on a module were globally available in other scripts, but attributes that had been set in other scripts were not. After some trial and error, I found that using any method of scriptExtension somehow put the script into a "shared" context. So, this appears to resolve the original issue reported by @spacemanspiff2007! Try this in loadA.py, and similar in loadB.py:

scriptExtension.importPreset(None)
import testModule
print( 'A Loaded')

You'll notice that only the first script will initialize testModule. I have stared at code for hours and can't explain this behavior. I'm not sure if this could be a bug somewhere in Jython or javax. If it is, this solution may not be permanent. As a side note, it is odd that importPreset can take None as an argument, but this seemed the least expensive way to cause this to happen. My WAG is that it is when the ScriptExtensionProviders do the getPresets in ScriptExtensionManager that the magic happens. As I mentioned, any method of scriptExtension in a script will work, including scriptExtension.getPresets().

For a better look at it, try something like this (uses libraries from openhab2-jython):

# script1
import core
core.test = 5

# script2
from core.log import logging, LOG_PREFIX
log = logging.getLogger(LOG_PREFIX + ".TEST")
import core
log.info("before preset [{}]".format(dir(core)))# will not have test attribute
scriptExtension.importPreset(None)
import core
log.info("after preset [{}]".format(dir(core)))# will have test attribute

If you share the same ScriptEngine on multiple files you may run into errors if you are loading scripts with errors. At some point the whole script engine might fail. That was the main reason behind this isolation.

Could you please provide some more insight on how scripts with errors could bring down the ScriptEngine? I'd like to understand more, in case we do end up using a single engine.

You are quite elusive 😄... so while you are here, have you tried updating your scripts and modules for use in OH 2.4? I've dug deep in the automation code, trying to get custom module handlers working, specifically the StartupTriggerHandler, but haven't been able to get them working yet. There has also been some discussion on adding some of the Jython/JS helper library functionality into OH, building out a scripting API, so that they can be used in any language. Your input/assistance would be very much appreciated! And a big THANK YOU for the work you've done with integrating JSR223 into OH!

[diijkstra]

Using a single script engine, and based on how scripts are currently loaded, Script2 will load after Script_1, so the import is redundant. If you use the log variable in Script1 after Script2 is loaded, it will have the definition from Script_2. Something like this can be easily remedied by naming the variable something different, like log1 and log2.

@openhab-5iver To be honest, I think if my 'hack' is causing such a behaviour it should not be used. As I said I haven't got time to test it on my main machine, if I had time I wouldn't post it ;-) I really hoped that each script will get separate context for its storage (as if you would run multiple files). No sane person will be able to keep all stuff across multiple scripts in order.

I would more be inclined to refactor OH-Jython-Scripters stuff to register as services (at least what can be done) and initialize other things on imports. We could cleanly mark stuff we know is broken on 2.7.1 and fix it one by one (from which we might post some bugs if something can not be done). But maybe this discussion should be moved to said repo, since despite my initial feeling, not much (if any) should be done on ESH side. What do you think?

And yeah, I would love if someone found why StartupTriggerHandler (or any custom handler) is not working! :-)

[5iver]

To be honest, I think if my 'hack' is causing such a behaviour it should not be used.

I agree, but what you put together will be very useful, since with some tweaks, I have enabled Jython and Groovy scripted actions for rules... especially useful for rules created in Paper UI! I think we should be fine using the importPreset magic, I'd just like to understand it better. I haven't gotten to look at what modules could be made into an extensions yet, and would prefer to put the time into migrating them to an API for others to use. I already have the core modules updated in my local repo, and I'm using them in my production system without issues. There's little to no chance that the changes to make 2.7.1 compatible would break 2.7.0 functionality, but I'll want to test it first before merging. Hopefully it solves everything for @spacemanspiff2007!

[diijkstra]

There's little to no chance that the changes to make 2.7.1 compatible would break 2.7.0 functionality, but I'll want to test it first before merging.

That is great news, good to know you are working on it :-)

[smerschjohann]

My WAG is that it is when the ScriptExtensionProviders do the getPresets in ScriptExtensionManager that the magic happens. As I mentioned, any method of scriptExtension in a script will work, including scriptExtension.getPresets().

wow, so if you use any method of scriptExtension triggers this behavior? Does it even work if you use dir(se)? - Sounds like unintended behavior, I'm still using 2.7.1b3 in my setup. But I never used imports that much.

Could you please provide some more insight on how scripts with errors could bring down the ScriptEngine? I'd like to understand more, in case we do end up using a single engine.

If I remember correctly, I have successfully killed the script engine if I made syntax errors. In that case the script engine ends up dead, you cannot load any more script data into it and you have to "throw it away". I ran into that issue a lot with kts (Kotlin) as scripting language in another project of mine (Mangfold), so this problem is real.

Furthermore, you cannot unload a previously loaded file if you just load the script "sourcecode" in an already initialized scriptengine. It might work if we use different script contexts for each file, but wouldn't we end up in the same problem as you currently have? I thought it was quiet elegant: If a script is changed or deleted, I remove everything the script added to the rule engine, let the script cleanup after itself using the scriptUnloaded method and then purge the script engine. I don't think we can achieve the same if we are just using script contexts.

so while you are here, have you tried updating your scripts and modules for use in OH 2.4?

I haven't touched my running setup for quiet a while now. I'm still using OH 2.2.0

I've dug deep in the automation code, trying to get custom module handlers working, specifically the StartupTriggerHandler, but haven't been able to get them working yet.

Hm, I might have a look into it. sounds to me someone removed or changed the trigger discovery logic.

There has also been some discussion on adding some of the Jython/JS helper library functionality into OH, building out a scripting API, so that they can be used in any language. Your input/assistance would be very much appreciated! And a big THANK YOU for the work you've done with integrating JSR223 into OH!

I thought about something like that while developing the jsr223 part. For that reason I added the preset part and made this script to adapt the Openhab1 script engine.

So you can even think about designing a "simpler" API in a scripting language like python and use that in another script engine like groovy, you don't have to do all the work in java.

[5iver]

Does it even work if you use dir(se)?

No, just the se methods (and attributes)... 'addScriptExtensionProvider', 'defaultPresets', 'get', 'getClass', 'getDefaultPresets', 'getPresets', 'getTypes', 'importPreset', 'presets', 'removeScriptExtensionProvider', 'types'.

I ran into that issue a lot with kts (Kotlin) as scripting language in another project of mine

A bit off topic, but this reminds me... did you ever get Kotlin working in OH? I'm not sure is it is even possible yet, but I've made some attempts at getting Xtend working in JSR223 to ease the migration to the new rule engine.

I haven't touched my running setup for quiet a while now. I'm still using OH 2.2.0

There have been significant changes, but all of the developers who have worked on the new rule engine seem to have gone. Is there any possibility that we could reel you back in? Maybe this will help?

Hm, I might have a look into it. sounds to me someone removed or changed the trigger discovery logic.

I've debugged it, but didn't spot anything in the history of the files being used that looked like it would cause the problem. My suspicion is that they stopped working after this massive PR, or at least around this timeframe. The handlers never seem to get registered properly in ScriptedCustomModuleHandlerFactory, so that when getModuleHandlerFactory goes to find them, they are not there. ModuleTypes appear to get added properly (they all show up when doing a smarthome:automation listModuleTypes in the console).

So you can even think about designing a "simpler" API in a scripting language like python and use that in another script engine like groovy, you don't have to do all the work in java.

I can think about it, but I have serious doubts a PR would be accepted! The API will also be used in rules created through the REST API and JSON too, which may complicate things if done this way.

Thank you for the info on scripts killing the engine!

[lewie]

@diijkstra,

But to be honest, I would love to hear from ESH guys, why they think that scripts should work on separate contexts. It maybe makes sense for pure rule centric scripts, but kills library-like scripts (custom handlers and stuff).

Why not load multiple custom handlers and library-like scripts from a "startup script" as a starting point and main "connector" to openHAB? In this single context, you have all the freedoms you need for your described applications.

Conversely, a single context would be fatal for all scripts running in parallel in that context. Timers and time-critical scripts or rules would also endanger each other, restart and abort each other.

With the current multi context solution you have all freedoms, without restrictions.

[5iver]

Hm, I might have a look into it. sounds to me someone removed or changed the trigger discovery logic.

Sorry for going off-topic, but I wanted to update here so that @smerschjohann didn't waste time debugging this. I finally figured out custom handlers. ScriptedCustomHandlerFactory was not being registered. More details in #517.

[spacemanspiff2007]

This is more than stale so I'm closing the issue.