[jsscripting] Fix multi-thread access requested by logger initialization (#16497)

* [jsscripting] Fix multi-threading issue with logger initialization

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/DebuggingGraalScriptEngine.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/DebuggingGraalScriptEngine.java
index c83fba16ee49c05e883359d2d063019b53bd6837..6342c6848b4d3341233da6d73276bd0c25944af2 100644 (file)
--- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/DebuggingGraalScriptEngine.java
+++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/DebuggingGraalScriptEngine.java
@@ -13,6 +13,7 @@
 package org.openhab.automation.jsscripting.internal;
 
 import javax.script.Invocable;
+import javax.script.ScriptContext;
 import javax.script.ScriptEngine;
 
 import org.eclipse.jdt.annotation.Nullable;
@@ -38,11 +39,15 @@ class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & AutoClosea
     }
 
     @Override
-    public Exception afterThrowsInvocation(Exception e) {
+    protected void beforeInvocation() {
+        super.beforeInvocation();
         if (logger == null) {
             initializeLogger();
         }
+    }
 
+    @Override
+    public Exception afterThrowsInvocation(Exception e) {
         Throwable cause = e.getCause();
         if (cause instanceof IllegalArgumentException) {
             logger.error("Failed to execute script:", e);
@@ -59,9 +64,10 @@ class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & AutoClosea
      * Therefore, the logger needs to be initialized on the first use after script engine creation.
      */
     private void initializeLogger() {
-        Object fileName = delegate.getContext().getAttribute("javax.script.filename");
-        Object ruleUID = delegate.getContext().getAttribute("ruleUID");
-        Object ohEngineIdentifier = delegate.getContext().getAttribute("oh.engine-identifier");
+        ScriptContext ctx = delegate.getContext();
+        Object fileName = ctx.getAttribute("javax.script.filename");
+        Object ruleUID = ctx.getAttribute("ruleUID");
+        Object ohEngineIdentifier = ctx.getAttribute("oh.engine-identifier");
 
         String identifier = "stack";
         if (fileName != null) {

diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java
index 5bb8511f9e365839de9bd72282d361ecb0c9f7a7..ce9450a3a3c64aff663d0cdfc567c1d1f228f35d 100644 (file)
--- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java
+++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java
@@ -134,7 +134,6 @@ public class OpenhabGraalJSScriptEngine
     private final JSRuntimeFeatures jsRuntimeFeatures;
 
     // these fields start as null because they are populated on first use
-    private @Nullable String engineIdentifier;
     private @Nullable Consumer<String> scriptDependencyListener;
 
     private boolean initialized = false;
@@ -243,7 +242,6 @@ public class OpenhabGraalJSScriptEngine
         if (localEngineIdentifier == null) {
             throw new IllegalStateException("Failed to retrieve engine identifier from engine bindings");
         }
-        engineIdentifier = localEngineIdentifier;
 
         ScriptExtensionAccessor scriptExtensionAccessor = (ScriptExtensionAccessor) ctx
                 .getAttribute(CONTEXT_KEY_EXTENSION_ACCESSOR);
@@ -251,12 +249,13 @@ public class OpenhabGraalJSScriptEngine
             throw new IllegalStateException("Failed to retrieve script extension accessor from engine bindings");
         }
 
-        scriptDependencyListener = (Consumer<String>) ctx
+        Consumer<String> localScriptDependencyListener = (Consumer<String>) ctx
                 .getAttribute("oh.dependency-listener"/* CONTEXT_KEY_DEPENDENCY_LISTENER */);
-        if (scriptDependencyListener == null) {
+        if (localScriptDependencyListener == null) {
             LOGGER.warn(
                     "Failed to retrieve script script dependency listener from engine bindings. Script dependency tracking will be disabled.");
         }
+        scriptDependencyListener = localScriptDependencyListener;
 
         ScriptExtensionModuleProvider scriptExtensionModuleProvider = new ScriptExtensionModuleProvider(
                 scriptExtensionAccessor, lock);
@@ -317,7 +316,7 @@ public class OpenhabGraalJSScriptEngine
      * @param path a root path
      * @return whether the given path is a node root directory
      */
-    private boolean isRootNodePath(Path path) {
+    private static boolean isRootNodePath(Path path) {
         return path.startsWith(path.getRoot().resolve(NODE_DIR));
     }
 
@@ -328,7 +327,7 @@ public class OpenhabGraalJSScriptEngine
      * @param path a root path, e.g. C:\node_modules\foo.js
      * @return the class resource path for loading local modules
      */
-    private String nodeFileToResource(Path path) {
+    private static String nodeFileToResource(Path path) {
         return "/" + path.subpath(0, path.getNameCount()).toString().replace('\\', '/');
     }