-
-
Notifications
You must be signed in to change notification settings - Fork 459
ScriptTransformationService: Implement missing script dependency tracking #5062
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ScriptTransformationService: Implement missing script dependency tracking #5062
Conversation
…king Closes openhab#5046. Signed-off-by: Florian Hotze <[email protected]>
|
In the use case described at #5046 with automation/js/node_modules/nmod1/index.js var a = "G"
console.log("uuu")
module.exports = { a }sitemap and the current change applied, when I change index.js sometimes it works, and sometimes it logs
and shows in the sitemap "Y -". I think I have to reload the sitemap twice. The first time the above waring is logged, the second time |
|
I see the the root cause of the issue, close is called two times inside the ScriptTransformationService if ScriptEngine implements Compilable, else once, and after the engine is already closed, on the next invocation of transform the ScriptEngineManager::createScriptEngine call causes the ScriptEngineManager to internally clean up the old engine, calling scriptUnloaded on the closed engine. |
Currently, there are two issues: When a ScriptRecord is cleared from the cache, the ScriptTransformationService currently closes the ScriptEngine itself, without involving the ScriptEngineManager. If a transformation script is invoked again after it has been cleared from the scriptCache, the ScriptEngineManager::createScriptEngine call makes the ScriptEngineManager first remove the old engine from its internal "store", which includes invoking the scriptUnloaded hook. This invocation can cause an exception because the engine is already closed (by ScriptTransformationService) and hence function/method invocations inside the engine are not possible anymore. Ff the engine implement Compilable, it is even closed two times by ScriptTransformationService, possibly causing an exception if the ScriptEngine doesn't handle multiple close() calls gracefully. These issues are fixed by properly notifying the ScriptEngineManager when an engine should be removed and leaving the removal handling to the ScriptEngineManager. The issues were discovered while working on openhab#5062. Signed-off-by: Florian Hotze <[email protected]>
|
See #5063 for that fix. |
|
With both patches applied, it works. After changing the dependency, the dependency is reloaded when:
After changing the dependency, the dependency does not reload when:
|
|
Turn on debug logging and you will see the script is always reloaded when the dependency changes. |
I mean above with reload, that if the dependency contains |
|
When the dependency changes, the bytecode of the script is thrown away. I use a JS transformation with an Item, the transformation depends on the items namespace from openhab-js:
Proper behaviour. |
…neManager (#5063) * ScriptTransformationService: Fix engine closed too early Currently, there are two issues: When a ScriptRecord is cleared from the cache, the ScriptTransformationService currently closes the ScriptEngine itself, without involving the ScriptEngineManager. If a transformation script is invoked again after it has been cleared from the scriptCache, the ScriptEngineManager::createScriptEngine call makes the ScriptEngineManager first remove the old engine from its internal "store", which includes invoking the scriptUnloaded hook. This invocation can cause an exception because the engine is already closed (by ScriptTransformationService) and hence function/method invocations inside the engine are not possible anymore. Ff the engine implement Compilable, it is even closed two times by ScriptTransformationService, possibly causing an exception if the ScriptEngine doesn't handle multiple close() calls gracefully. These issues are fixed by properly notifying the ScriptEngineManager when an engine should be removed and leaving the removal handling to the ScriptEngineManager. The issues were discovered while working on #5062. Signed-off-by: Florian Hotze <[email protected]>
Fixes #5046.