]> git.basschouten.com Git - openhab-addons.git/commitdiff
[jsscripting] Synchronize context access in logger initialization (#17496)
authorFlorian Hotze <florianh_dev@icloud.com>
Wed, 2 Oct 2024 20:53:49 +0000 (22:53 +0200)
committerGitHub <noreply@github.com>
Wed, 2 Oct 2024 20:53:49 +0000 (22:53 +0200)
* [jsscripting] Synchronize context access in logger initialisation to avoid illegal multi-thread access
Fixes #17494.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/DebuggingGraalScriptEngine.java
bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java

index 76f1afb2dd54483c33b893d4589cdf4fb2783744..8c71899a6880a35a9fa90abe84b47182fc046c2d 100644 (file)
@@ -15,6 +15,7 @@ package org.openhab.automation.jsscripting.internal;
 import static org.openhab.core.automation.module.script.ScriptTransformationService.OPENHAB_TRANSFORMATION_SCRIPT;
 
 import java.util.Arrays;
+import java.util.concurrent.locks.Lock;
 import java.util.stream.Collectors;
 
 import javax.script.Compilable;
@@ -34,7 +35,7 @@ import org.slf4j.LoggerFactory;
  * @author Jonathan Gilbert - Initial contribution
  * @author Florian Hotze - Improve logger name, Fix memory leak caused by exception logging
  */
-class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & AutoCloseable & Compilable>
+class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & AutoCloseable & Compilable & Lock>
         extends InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable<T> {
 
     private static final int STACK_TRACE_LENGTH = 5;
@@ -48,8 +49,18 @@ class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & AutoClosea
     @Override
     protected void beforeInvocation() {
         super.beforeInvocation();
-        if (logger == null) {
-            initializeLogger();
+        // OpenhabGraalJSScriptEngine::beforeInvocation will be executed after
+        // DebuggingGraalScriptEngine::beforeInvocation, because GraalJSScriptEngineFactory::createScriptEngine returns
+        // a DebuggingGraalScriptEngine instance.
+        // We therefore need to synchronize logger setup here and cannot rely on the synchronization in
+        // OpenhabGraalJSScriptEngine.
+        delegate.lock();
+        try {
+            if (logger == null) {
+                initializeLogger();
+            }
+        } finally { // Make sure that Lock is unlocked regardless of an exception being thrown or not to avoid deadlocks
+            delegate.unlock();
         }
     }
 
index cb4dcfd23b58b6a3f7ea1290fc201bd752f66b44..94b89666318b0231db47514307ae1aad98c3db40 100644 (file)
@@ -32,6 +32,8 @@ import java.time.Instant;
 import java.time.ZonedDateTime;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.locks.Condition;
 import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 import java.util.function.Consumer;
@@ -69,7 +71,8 @@ import com.oracle.truffle.js.scriptengine.GraalJSScriptEngine;
  *         {@link Lock} for multi-thread synchronization; globals and openhab-js injection code caching
  */
 public class OpenhabGraalJSScriptEngine
-        extends InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable<GraalJSScriptEngine> {
+        extends InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable<GraalJSScriptEngine>
+        implements Lock {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(OpenhabGraalJSScriptEngine.class);
     private static final Source GLOBAL_SOURCE;
@@ -346,4 +349,34 @@ public class OpenhabGraalJSScriptEngine
 
         return new InputStreamReader(ioStream);
     }
+
+    @Override
+    public void lock() {
+        lock.lock();
+    }
+
+    @Override
+    public void lockInterruptibly() throws InterruptedException {
+        lock.lockInterruptibly();
+    }
+
+    @Override
+    public boolean tryLock() {
+        return lock.tryLock();
+    }
+
+    @Override
+    public boolean tryLock(long l, TimeUnit timeUnit) throws InterruptedException {
+        return lock.tryLock(l, timeUnit);
+    }
+
+    @Override
+    public void unlock() {
+        lock.unlock();
+    }
+
+    @Override
+    public Condition newCondition() {
+        return lock.newCondition();
+    }
 }