[jsscripting] Prevent deadlock in ThreadsafeSimpleRuleDelegate (#20853)

* [jsscripting] Prevent deadlock in ThreadsafeSimpleRuleDelegate

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
This commit is contained in:
Nadahar
2026-06-19 16:42:55 +02:00
committed by GitHub
parent 21e9a13c56
commit 7261d9d8f1
12 changed files with 130 additions and 115 deletions
@@ -16,19 +16,18 @@ import static org.openhab.core.automation.module.script.ScriptEngineFactory.CONT
import static org.openhab.core.automation.module.script.ScriptTransformationService.OPENHAB_TRANSFORMATION_SCRIPT; import static org.openhab.core.automation.module.script.ScriptTransformationService.OPENHAB_TRANSFORMATION_SCRIPT;
import java.util.Arrays; import java.util.Arrays;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Condition;
import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.Lock;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import javax.script.Compilable; import javax.script.Compilable;
import javax.script.Invocable; import javax.script.Invocable;
import javax.script.ScriptContext; import javax.script.ScriptContext;
import javax.script.ScriptEngine;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable; import org.eclipse.jdt.annotation.Nullable;
import org.graalvm.polyglot.PolyglotException; import org.graalvm.polyglot.PolyglotException;
import org.openhab.automation.jsscripting.internal.scriptengine.InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable; import org.openhab.automation.jsscripting.internal.scriptengine.InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable;
import org.openhab.core.automation.module.script.LockableScriptEngine;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
@@ -38,8 +37,9 @@ import org.slf4j.LoggerFactory;
* @author Jonathan Gilbert - Initial contribution * @author Jonathan Gilbert - Initial contribution
* @author Florian Hotze - Improve logger name, Fix memory leak caused by exception logging * @author Florian Hotze - Improve logger name, Fix memory leak caused by exception logging
*/ */
public class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & AutoCloseable & Compilable & Lock> public class DebuggingGraalScriptEngine<T extends LockableScriptEngine & Invocable & AutoCloseable & Compilable>
extends InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable<T> implements Lock { extends InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable<T>
implements LockableScriptEngine {
private static final int STACK_TRACE_LENGTH = 5; private static final int STACK_TRACE_LENGTH = 5;
@@ -57,13 +57,14 @@ public class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & Aut
// a DebuggingGraalScriptEngine instance. // a DebuggingGraalScriptEngine instance.
// We therefore need to synchronize logger setup here and cannot rely on the synchronization in // We therefore need to synchronize logger setup here and cannot rely on the synchronization in
// OpenhabGraalJSScriptEngine. // OpenhabGraalJSScriptEngine.
delegate.lock(); Lock lock = delegate.getLock();
lock.lock();
try { try {
if (logger == null) { if (logger == null) {
initializeLogger(); initializeLogger();
} }
} finally { // Make sure that Lock is unlocked regardless of an exception being thrown or not to avoid deadlocks } finally { // Make sure that Lock is unlocked regardless of an exception being thrown or not to avoid deadlocks
delegate.unlock(); lock.unlock();
} }
} }
@@ -123,32 +124,12 @@ public class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & Aut
} }
@Override @Override
public void lock() { public @NonNull Lock getLock() {
delegate.lock(); return delegate.getLock();
} }
@Override @Override
public void lockInterruptibly() throws InterruptedException { public long getLockAcquisitionTimeoutMs() {
delegate.lockInterruptibly(); return delegate.getLockAcquisitionTimeoutMs();
}
@Override
public boolean tryLock() {
return delegate.tryLock();
}
@Override
public boolean tryLock(long l, TimeUnit timeUnit) throws InterruptedException {
return delegate.tryLock(l, timeUnit);
}
@Override
public void unlock() {
delegate.unlock();
}
@Override
public Condition newCondition() {
return delegate.newCondition();
} }
} }
@@ -13,6 +13,7 @@
package org.openhab.automation.jsscripting.internal; package org.openhab.automation.jsscripting.internal;
import java.util.Map; import java.util.Map;
import java.util.concurrent.TimeUnit;
import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.NonNullByDefault;
import org.openhab.core.config.core.ConfigParser; import org.openhab.core.config.core.ConfigParser;
@@ -35,6 +36,7 @@ public class GraalJSScriptEngineConfiguration {
private static final String CFG_DEPENDENCY_TRACKING_ENABLED = "dependencyTrackingEnabled"; private static final String CFG_DEPENDENCY_TRACKING_ENABLED = "dependencyTrackingEnabled";
private static final String CFG_DEBUGGER_ENABLED = "debuggerEnabled"; private static final String CFG_DEBUGGER_ENABLED = "debuggerEnabled";
private static final String CFG_DEBUGGER_PORT = "debuggerPort"; private static final String CFG_DEBUGGER_PORT = "debuggerPort";
private static final String CFG_LOCK_ACQUISITION_TIMEOUT = "lockAcquisitionTimeout";
private static final int INJECTION_ENABLED_FOR_SCRIPT_MODULES_ONLY = 1; private static final int INJECTION_ENABLED_FOR_SCRIPT_MODULES_ONLY = 1;
private static final int INJECTION_ENABLED_FOR_SCRIPT_MODULES_AND_TRANSFORMATIONS = 2; private static final int INJECTION_ENABLED_FOR_SCRIPT_MODULES_AND_TRANSFORMATIONS = 2;
@@ -42,6 +44,9 @@ public class GraalJSScriptEngineConfiguration {
private static final int DEBUGGER_PORT_DEFAULT = 9229; private static final int DEBUGGER_PORT_DEFAULT = 9229;
/** The default lock acquisition timeout in seconds */
private static final long LOCK_ACQUISITION_TIMEOUT_DEFAULT = 5L;
private int injectionEnabled = INJECTION_ENABLED_FOR_ALL_SCRIPTS; private int injectionEnabled = INJECTION_ENABLED_FOR_ALL_SCRIPTS;
private boolean injectionCachingEnabled = true; private boolean injectionCachingEnabled = true;
private boolean scriptConditionWrapperEnabled = false; private boolean scriptConditionWrapperEnabled = false;
@@ -49,10 +54,11 @@ public class GraalJSScriptEngineConfiguration {
private boolean dependencyTrackingEnabled = true; private boolean dependencyTrackingEnabled = true;
private boolean debuggerEnabled = false; private boolean debuggerEnabled = false;
private int debuggerPort = DEBUGGER_PORT_DEFAULT; private int debuggerPort = DEBUGGER_PORT_DEFAULT;
private long lockAcquisitionTimeout = TimeUnit.SECONDS.toMillis(LOCK_ACQUISITION_TIMEOUT_DEFAULT);
/** /**
* Create a new configuration instance from the given parameters. * Create a new configuration instance from the given parameters.
* *
* @param config configuration parameters to apply to JavaScript * @param config configuration parameters to apply to JavaScript
*/ */
public GraalJSScriptEngineConfiguration(Map<String, ?> config) { public GraalJSScriptEngineConfiguration(Map<String, ?> config) {
@@ -71,6 +77,7 @@ public class GraalJSScriptEngineConfiguration {
boolean oldEventConversionEnabled = eventConversionEnabled; boolean oldEventConversionEnabled = eventConversionEnabled;
boolean oldDebuggerEnabled = debuggerEnabled; boolean oldDebuggerEnabled = debuggerEnabled;
int oldDebuggerPort = debuggerPort; int oldDebuggerPort = debuggerPort;
long oldLockAcquisitionTimeout = lockAcquisitionTimeout;
this.update(config); this.update(config);
@@ -105,6 +112,11 @@ public class GraalJSScriptEngineConfiguration {
} else if (oldDebuggerPort != debuggerPort) { } else if (oldDebuggerPort != debuggerPort) {
logger.warn("Reconfigured debugger for JavaScript Scripting. Restart openHAB to apply this change."); logger.warn("Reconfigured debugger for JavaScript Scripting. Restart openHAB to apply this change.");
} }
if (oldLockAcquisitionTimeout != lockAcquisitionTimeout) {
logger.warn(
"JavaScript Scripting lock acquisition timeout changed from {} to {} milliseconds. Rules created with JavaScript scripts might need to be reloaded for the changes to apply.",
oldLockAcquisitionTimeout, lockAcquisitionTimeout);
}
} }
/** /**
@@ -127,12 +139,14 @@ public class GraalJSScriptEngineConfiguration {
Boolean.class, true); Boolean.class, true);
debuggerEnabled = ConfigParser.valueAsOrElse(config.get(CFG_DEBUGGER_ENABLED), Boolean.class, false); debuggerEnabled = ConfigParser.valueAsOrElse(config.get(CFG_DEBUGGER_ENABLED), Boolean.class, false);
debuggerPort = ConfigParser.valueAsOrElse(config.get(CFG_DEBUGGER_PORT), Integer.class, DEBUGGER_PORT_DEFAULT); debuggerPort = ConfigParser.valueAsOrElse(config.get(CFG_DEBUGGER_PORT), Integer.class, DEBUGGER_PORT_DEFAULT);
lockAcquisitionTimeout = TimeUnit.SECONDS.toMillis(ConfigParser
.valueAsOrElse(config.get(CFG_LOCK_ACQUISITION_TIMEOUT), Long.class, LOCK_ACQUISITION_TIMEOUT_DEFAULT));
} }
/** /**
* Whether injection is enabled for script modules, i.e. scripts executed by an implementation of * Whether injection is enabled for script modules, i.e. scripts executed by an implementation of
* {@link org.openhab.core.automation.module.script.internal.handler.AbstractScriptModuleHandler}. * {@link org.openhab.core.automation.module.script.internal.handler.AbstractScriptModuleHandler}.
* *
* @return whether injection is enabled for script modules * @return whether injection is enabled for script modules
*/ */
public boolean isInjectionEnabledForScriptModules() { public boolean isInjectionEnabledForScriptModules() {
@@ -142,7 +156,7 @@ public class GraalJSScriptEngineConfiguration {
/** /**
* Whether injection is enabled for transformations, i.e. scripts executed by the * Whether injection is enabled for transformations, i.e. scripts executed by the
* {@link org.openhab.core.automation.module.script.ScriptTransformationService}. * {@link org.openhab.core.automation.module.script.ScriptTransformationService}.
* *
* @return whether injection is enabled for transformations * @return whether injection is enabled for transformations
*/ */
public boolean isInjectionEnabledForTransformations() { public boolean isInjectionEnabledForTransformations() {
@@ -151,7 +165,7 @@ public class GraalJSScriptEngineConfiguration {
/** /**
* Whether injection is enabled for all scripts, i.e. script modules, transformations and script files. * Whether injection is enabled for all scripts, i.e. script modules, transformations and script files.
* *
* @return whether injection is enabled for all scripts * @return whether injection is enabled for all scripts
*/ */
public boolean isInjectionEnabledForAllScripts() { public boolean isInjectionEnabledForAllScripts() {
@@ -165,7 +179,7 @@ public class GraalJSScriptEngineConfiguration {
/** /**
* Whether the wrapper is enabled for script conditions (see * Whether the wrapper is enabled for script conditions (see
* {@link org.openhab.core.automation.module.script.internal.handler.ScriptConditionHandler}). * {@link org.openhab.core.automation.module.script.internal.handler.ScriptConditionHandler}).
* *
* @return whether the wrapper is enabled for script conditions * @return whether the wrapper is enabled for script conditions
*/ */
public boolean isScriptConditionWrapperEnabled() { public boolean isScriptConditionWrapperEnabled() {
@@ -187,4 +201,11 @@ public class GraalJSScriptEngineConfiguration {
public int getDebuggerPort() { public int getDebuggerPort() {
return debuggerPort; return debuggerPort;
} }
/**
* @return The log acquisition timeout in milliseconds.
*/
public long getLockAcquisitionTimeout() {
return lockAcquisitionTimeout;
}
} }
@@ -34,8 +34,6 @@ import java.time.ZonedDateTime;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; 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.Lock;
import java.util.concurrent.locks.ReentrantLock; import java.util.concurrent.locks.ReentrantLock;
import java.util.function.Consumer; import java.util.function.Consumer;
@@ -45,6 +43,7 @@ import java.util.regex.Pattern;
import javax.script.ScriptContext; import javax.script.ScriptContext;
import javax.script.ScriptException; import javax.script.ScriptException;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable; import org.eclipse.jdt.annotation.Nullable;
import org.graalvm.polyglot.Context; import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.Engine; import org.graalvm.polyglot.Engine;
@@ -61,6 +60,7 @@ import org.openhab.automation.jsscripting.internal.scriptengine.InvocationInterc
import org.openhab.automation.jsscripting.internal.scriptengine.helper.LifecycleTracker; import org.openhab.automation.jsscripting.internal.scriptengine.helper.LifecycleTracker;
import org.openhab.automation.jsscripting.internal.util.Slf4jOutputStream; import org.openhab.automation.jsscripting.internal.util.Slf4jOutputStream;
import org.openhab.core.OpenHAB; import org.openhab.core.OpenHAB;
import org.openhab.core.automation.module.script.LockableScriptEngine;
import org.openhab.core.automation.module.script.ScriptExtensionAccessor; import org.openhab.core.automation.module.script.ScriptExtensionAccessor;
import org.openhab.core.automation.module.script.internal.handler.AbstractScriptModuleHandler; import org.openhab.core.automation.module.script.internal.handler.AbstractScriptModuleHandler;
import org.openhab.core.automation.module.script.internal.handler.ScriptActionHandler; import org.openhab.core.automation.module.script.internal.handler.ScriptActionHandler;
@@ -84,7 +84,7 @@ import com.oracle.truffle.js.scriptengine.GraalJSScriptEngine;
*/ */
public class OpenhabGraalJSScriptEngine public class OpenhabGraalJSScriptEngine
extends InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable<GraalJSScriptEngine> extends InvocationInterceptingScriptEngineWithInvocableAndCompilableAndAutoCloseable<GraalJSScriptEngine>
implements Lock { implements LockableScriptEngine {
// see private constant GraalJSScriptEngine.ID // see private constant GraalJSScriptEngine.ID
static final String LANGUAGE_ID = "js"; static final String LANGUAGE_ID = "js";
@@ -312,7 +312,7 @@ public class OpenhabGraalJSScriptEngine
scriptDependencyListener = localScriptDependencyListener; scriptDependencyListener = localScriptDependencyListener;
ScriptExtensionModuleProvider scriptExtensionModuleProvider = new ScriptExtensionModuleProvider( ScriptExtensionModuleProvider scriptExtensionModuleProvider = new ScriptExtensionModuleProvider(
scriptExtensionAccessor, lock, lifecycleTracker); scriptExtensionAccessor, lock, getLockAcquisitionTimeoutMs(), lifecycleTracker);
// Wrap the "require" function to also allow loading modules from the ScriptExtensionModuleProvider // Wrap the "require" function to also allow loading modules from the ScriptExtensionModuleProvider
Function<Function<Object[], Object>, Function<String, Object>> wrapRequireFn = originalRequireFn -> moduleName -> scriptExtensionModuleProvider Function<Function<Object[], Object>, Function<String, Object>> wrapRequireFn = originalRequireFn -> moduleName -> scriptExtensionModuleProvider
@@ -466,7 +466,7 @@ public class OpenhabGraalJSScriptEngine
/** /**
* Tests if the script is a script file, i.e. it is loaded from a JavaScript file. * Tests if the script is a script file, i.e. it is loaded from a JavaScript file.
* *
* @return true if the script is loaded from a JavaScript file, false otherwise * @return true if the script is loaded from a JavaScript file, false otherwise
*/ */
private boolean isScriptFile() { private boolean isScriptFile() {
@@ -480,7 +480,7 @@ public class OpenhabGraalJSScriptEngine
/** /**
* Get the module type id (if any) of the module executing the script. * Get the module type id (if any) of the module executing the script.
* *
* @return the module type id (if any) of the module executing the script, or null if the script is not a module * @return the module type id (if any) of the module executing the script, or null if the script is not a module
*/ */
private @Nullable String getModuleTypeId() { private @Nullable String getModuleTypeId() {
@@ -500,7 +500,7 @@ public class OpenhabGraalJSScriptEngine
/** /**
* Tests if the script is a script module, i.e. executed by an implementation of * Tests if the script is a script module, i.e. executed by an implementation of
* {@link AbstractScriptModuleHandler}. * {@link AbstractScriptModuleHandler}.
* *
* @return true if the script is a script module, false otherwise * @return true if the script is a script module, false otherwise
*/ */
private boolean isScriptModule() { private boolean isScriptModule() {
@@ -510,7 +510,7 @@ public class OpenhabGraalJSScriptEngine
/** /**
* Tests if a script is a script action, i.e. executed by the ScriptActionHandler. * Tests if a script is a script action, i.e. executed by the ScriptActionHandler.
* *
* @return true if the script is a script action, false otherwise * @return true if the script is a script action, false otherwise
*/ */
private boolean isScriptAction() { private boolean isScriptAction() {
@@ -519,7 +519,7 @@ public class OpenhabGraalJSScriptEngine
/** /**
* Tests if the script is a script condition, i.e. executed by the ScriptConditionHandler. * Tests if the script is a script condition, i.e. executed by the ScriptConditionHandler.
* *
* @return true if the script is a script condition, false otherwise * @return true if the script is a script condition, false otherwise
*/ */
private boolean isScriptCondition() { private boolean isScriptCondition() {
@@ -528,7 +528,7 @@ public class OpenhabGraalJSScriptEngine
/** /**
* Tests if the script is a transformation script, i.e. created from the script transformation service. * Tests if the script is a transformation script, i.e. created from the script transformation service.
* *
* @return true if it is a transformation script, false otherwise * @return true if it is a transformation script, false otherwise
*/ */
private boolean isTransformation() { private boolean isTransformation() {
@@ -576,34 +576,12 @@ public class OpenhabGraalJSScriptEngine
} }
@Override @Override
public void lock() { public @NonNull Lock getLock() {
lock.lock(); return lock;
logger.debug("Lock acquired for engine '{}'.", engineIdentifier);
} }
@Override @Override
public void lockInterruptibly() throws InterruptedException { public long getLockAcquisitionTimeoutMs() {
lock.lockInterruptibly(); return configuration.getLockAcquisitionTimeout();
}
@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();
logger.debug("Lock released for engine '{}'.", engineIdentifier);
}
@Override
public Condition newCondition() {
return lock.newCondition();
} }
} }
@@ -41,14 +41,16 @@ public class ScriptExtensionModuleProvider {
private static final String RUNTIME_MODULE_PREFIX = "@runtime"; private static final String RUNTIME_MODULE_PREFIX = "@runtime";
private static final String DEFAULT_MODULE_NAME = "Defaults"; private static final String DEFAULT_MODULE_NAME = "Defaults";
private final Lock lock; private final Lock lock;
private final long lockAcquisitionTimeoutMS;
private final LifecycleTracker lifecycleTracker; private final LifecycleTracker lifecycleTracker;
private final ScriptExtensionAccessor scriptExtensionAccessor; private final ScriptExtensionAccessor scriptExtensionAccessor;
public ScriptExtensionModuleProvider(ScriptExtensionAccessor scriptExtensionAccessor, Lock lock, public ScriptExtensionModuleProvider(ScriptExtensionAccessor scriptExtensionAccessor, Lock lock,
LifecycleTracker lifecycleTracker) { long lockAcquisitionTimeoutMS, LifecycleTracker lifecycleTracker) {
this.scriptExtensionAccessor = scriptExtensionAccessor; this.scriptExtensionAccessor = scriptExtensionAccessor;
this.lock = lock; this.lock = lock;
this.lockAcquisitionTimeoutMS = lockAcquisitionTimeoutMS;
this.lifecycleTracker = lifecycleTracker; this.lifecycleTracker = lifecycleTracker;
} }
@@ -108,8 +110,8 @@ public class ScriptExtensionModuleProvider {
for (Map.Entry<String, Object> entry : rv.entrySet()) { for (Map.Entry<String, Object> entry : rv.entrySet()) {
if (entry.getValue() instanceof ScriptedAutomationManager scriptedAutomationManager) { if (entry.getValue() instanceof ScriptedAutomationManager scriptedAutomationManager) {
entry.setValue( entry.setValue(new ThreadsafeWrappingScriptedAutomationManagerDelegate(scriptedAutomationManager, lock,
new ThreadsafeWrappingScriptedAutomationManagerDelegate(scriptedAutomationManager, lock)); lockAcquisitionTimeoutMS));
} }
} }
@@ -15,6 +15,7 @@ package org.openhab.automation.jsscripting.internal.threading;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Set; import java.util.Set;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.Lock;
import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -40,27 +41,44 @@ import org.openhab.core.config.core.Configuration;
class ThreadsafeSimpleRuleDelegate implements Rule, SimpleRuleActionHandler { class ThreadsafeSimpleRuleDelegate implements Rule, SimpleRuleActionHandler {
private final Lock lock; private final Lock lock;
private final long lockAcquisitionTimeoutMS;
private final SimpleRule delegate; private final SimpleRule delegate;
/** /**
* Constructor requires a lock object and delegate to forward invocations to. * Constructor requires a lock object and delegate to forward invocations to.
* *
* @param lock rule executions will synchronize on this object * @param lock rule executions will synchronize on this object
* @param lockAcquisitionTimeoutMS the lock acquisition timeout in milliseconds.
* @param delegate the delegate to forward invocations to * @param delegate the delegate to forward invocations to
*/ */
ThreadsafeSimpleRuleDelegate(Lock lock, SimpleRule delegate) { ThreadsafeSimpleRuleDelegate(Lock lock, long lockAcquisitionTimeoutMS, SimpleRule delegate) {
this.lock = lock; this.lock = lock;
this.lockAcquisitionTimeoutMS = lockAcquisitionTimeoutMS;
this.delegate = delegate; this.delegate = delegate;
} }
@Override @Override
@NonNullByDefault({}) @NonNullByDefault({})
public Object execute(Action module, Map<String, ?> inputs) { public Object execute(Action module, Map<String, ?> inputs) {
lock.lock(); boolean locked;
try { try {
return delegate.execute(module, inputs); locked = lock.tryLock(lockAcquisitionTimeoutMS, TimeUnit.MILLISECONDS);
} finally { // Make sure that Lock is unlocked regardless of an exception is thrown or not to avoid deadlocks } catch (InterruptedException e) {
lock.unlock(); Thread.currentThread().interrupt();
throw new RuntimeException("Interrupted while waiting to acquire the lock for action '" + module.getId()
+ "' of rule '" + delegate.getUID() + '\'', e);
}
if (locked) {
try {
return delegate.execute(module, inputs);
} finally {
// Make sure that Lock is unlocked regardless of an exception is thrown or not to avoid deadlocks
lock.unlock();
}
} else {
throw new RuntimeException(
"Failed to acquire the lock for action '" + module.getId() + "' of rule '" + delegate.getUID()
+ "' within " + TimeUnit.MILLISECONDS.toSeconds(lockAcquisitionTimeoutMS) + " seconds.");
} }
} }
@@ -40,10 +40,13 @@ public class ThreadsafeWrappingScriptedAutomationManagerDelegate {
private ScriptedAutomationManager delegate; private ScriptedAutomationManager delegate;
private final Lock lock; private final Lock lock;
private final long lockAcquisitionTimeoutMS;
public ThreadsafeWrappingScriptedAutomationManagerDelegate(ScriptedAutomationManager delegate, Lock lock) { public ThreadsafeWrappingScriptedAutomationManagerDelegate(ScriptedAutomationManager delegate, Lock lock,
long lockAcquisitionTimeoutMS) {
this.delegate = delegate; this.delegate = delegate;
this.lock = lock; this.lock = lock;
this.lockAcquisitionTimeoutMS = lockAcquisitionTimeoutMS;
} }
public void removeModuleType(String UID) { public void removeModuleType(String UID) {
@@ -65,7 +68,7 @@ public class ThreadsafeWrappingScriptedAutomationManagerDelegate {
public Rule addRule(Rule element) { public Rule addRule(Rule element) {
// wrap in a threadsafe version, safe per context // wrap in a threadsafe version, safe per context
if (element instanceof SimpleRule rule) { if (element instanceof SimpleRule rule) {
element = new ThreadsafeSimpleRuleDelegate(lock, rule); element = new ThreadsafeSimpleRuleDelegate(lock, lockAcquisitionTimeoutMS, rule);
} }
return delegate.addRule(element); return delegate.addRule(element);
@@ -71,6 +71,14 @@
<default>true</default> <default>true</default>
<advanced>true</advanced> <advanced>true</advanced>
</parameter> </parameter>
<parameter name="lockAcquisitionTimeout" type="integer" min="1" max="120" step="1" groupName="system">
<label>Engine Lock Acquisition Timeout</label>
<description>The script engines must be reserved only for a specific thread before executing a script. This setting
decides how many seconds the system will try to acquire the engine lock before giving up. If you have long running
scripts that might be triggered faster than they can finish, you might want to use a high value.</description>
<default>5</default>
<advanced>true</advanced>
</parameter>
<!-- Debugger --> <!-- Debugger -->
<parameter name="debuggerEnabled" type="boolean" required="true" groupName="debugger"> <parameter name="debuggerEnabled" type="boolean" required="true" groupName="debugger">
@@ -24,5 +24,7 @@ automation.config.jsscripting.injectionEnabledV2.option.3 = Auto injection every
automation.config.jsscripting.injectionEnabledV2.option.2 = Auto injection for Script Actions, Script Conditions and transformations automation.config.jsscripting.injectionEnabledV2.option.2 = Auto injection for Script Actions, Script Conditions and transformations
automation.config.jsscripting.injectionEnabledV2.option.1 = Auto injection only for Script Actions & Script Conditions (recommended) automation.config.jsscripting.injectionEnabledV2.option.1 = Auto injection only for Script Actions & Script Conditions (recommended)
automation.config.jsscripting.injectionEnabledV2.option.0 = Disable auto-injection and import manually instead automation.config.jsscripting.injectionEnabledV2.option.0 = Disable auto-injection and import manually instead
automation.config.jsscripting.lockAcquisitionTimeout.label = Engine Lock Acquisition Timeout
automation.config.jsscripting.lockAcquisitionTimeout.description = The script engines must be reserved only for a specific thread before executing a script. This setting decides how many seconds the system will try to acquire the engine lock before giving up. If you have long running scripts that might be triggered faster than they can finish, you might want to use a high value.
automation.config.jsscripting.scriptConditionWrapperEnabled.label = Wrap Script Conditions in Self-Executing Function automation.config.jsscripting.scriptConditionWrapperEnabled.label = Wrap Script Conditions in Self-Executing Function
automation.config.jsscripting.scriptConditionWrapperEnabled.description = Wrapping script conditions in a self-executing function allows the use of the <code>let</code> and <code>const</code> variable declarations, as well as the use of <code>function</code> and <code>class</code> declarations.<br> With this option enabled, you need to use <code>return</code> statements in your script condition to return true or false. automation.config.jsscripting.scriptConditionWrapperEnabled.description = Wrapping script conditions in a self-executing function allows the use of the <code>let</code> and <code>const</code> variable declarations, as well as the use of <code>function</code> and <code>class</code> declarations.<br> With this option enabled, you need to use <code>return</code> statements in your script condition to return true or false.
@@ -30,8 +30,6 @@ import java.util.Arrays;
import java.util.HashSet; import java.util.HashSet;
import java.util.List; import java.util.List;
import java.util.Set; 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.Lock;
import java.util.concurrent.locks.ReentrantLock; import java.util.concurrent.locks.ReentrantLock;
import java.util.function.BiFunction; import java.util.function.BiFunction;
@@ -41,6 +39,7 @@ import java.util.stream.Collectors;
import javax.script.ScriptContext; import javax.script.ScriptContext;
import javax.script.ScriptException; import javax.script.ScriptException;
import org.eclipse.jdt.annotation.NonNull;
import org.eclipse.jdt.annotation.Nullable; import org.eclipse.jdt.annotation.Nullable;
import org.graalvm.polyglot.Context; import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.Engine; import org.graalvm.polyglot.Engine;
@@ -57,6 +56,7 @@ import org.openhab.automation.pythonscripting.internal.provider.LifecycleTracker
import org.openhab.automation.pythonscripting.internal.provider.ScriptExtensionModuleProvider; import org.openhab.automation.pythonscripting.internal.provider.ScriptExtensionModuleProvider;
import org.openhab.automation.pythonscripting.internal.scriptengine.InvocationInterceptingPythonScriptEngine; import org.openhab.automation.pythonscripting.internal.scriptengine.InvocationInterceptingPythonScriptEngine;
import org.openhab.automation.pythonscripting.internal.scriptengine.graal.GraalPythonScriptEngine; import org.openhab.automation.pythonscripting.internal.scriptengine.graal.GraalPythonScriptEngine;
import org.openhab.core.automation.module.script.LockableScriptEngine;
import org.openhab.core.automation.module.script.ScriptExtensionAccessor; import org.openhab.core.automation.module.script.ScriptExtensionAccessor;
import org.openhab.core.automation.module.script.internal.handler.AbstractScriptModuleHandler; import org.openhab.core.automation.module.script.internal.handler.AbstractScriptModuleHandler;
import org.openhab.core.library.types.DateTimeType; import org.openhab.core.library.types.DateTimeType;
@@ -75,7 +75,7 @@ import org.slf4j.event.Level;
* @author Holger Hees - Initial contribution * @author Holger Hees - Initial contribution
* @author Jeff James - Initial contribution * @author Jeff James - Initial contribution
*/ */
public class PythonScriptEngine extends InvocationInterceptingPythonScriptEngine implements Lock { public class PythonScriptEngine extends InvocationInterceptingPythonScriptEngine implements LockableScriptEngine {
private final Logger logger = LoggerFactory.getLogger(PythonScriptEngine.class); private final Logger logger = LoggerFactory.getLogger(PythonScriptEngine.class);
public static final String CONTEXT_KEY_ENGINE_LOGGER_OUTPUT = "ctx.engine-logger-output"; public static final String CONTEXT_KEY_ENGINE_LOGGER_OUTPUT = "ctx.engine-logger-output";
@@ -411,34 +411,13 @@ public class PythonScriptEngine extends InvocationInterceptingPythonScriptEngine
} }
@Override @Override
public void lock() { public @NonNull Lock getLock() {
lock.lock(); return lock;
logger.debug("Lock acquired for engine '{}'.", this.engineIdentifier);
} }
@Override @Override
public void lockInterruptibly() throws InterruptedException { public long getLockAcquisitionTimeoutMs() {
lock.lockInterruptibly(); return pythonScriptEngineConfiguration.getLockAcquisitionTimeout();
}
@Override
public boolean tryLock() {
boolean acquired = lock.tryLock();
logger.debug("{} for engine '{}'", acquired ? "Lock acquired." : "Lock not acquired.", this.engineIdentifier);
return acquired;
}
@Override
public boolean tryLock(long l, @Nullable TimeUnit timeUnit) throws InterruptedException {
boolean acquired = lock.tryLock(l, timeUnit);
logger.debug("{} for engine '{}'", acquired ? "Lock acquired." : "Lock not acquired.", this.engineIdentifier);
return acquired;
}
@Override
public void unlock() {
lock.unlock();
logger.debug("Lock released for engine '{}'.", this.engineIdentifier);
} }
@Override @Override
@@ -466,11 +445,6 @@ public class PythonScriptEngine extends InvocationInterceptingPythonScriptEngine
lock.unlock(); lock.unlock();
} }
@Override
public Condition newCondition() {
return lock.newCondition();
}
/** /**
* Initializes the logger. * Initializes the logger.
* This cannot be done on script engine creation because the context variables are not yet initialized. * This cannot be done on script engine creation because the context variables are not yet initialized.
@@ -22,6 +22,7 @@ import java.nio.file.Paths;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import java.util.Properties; import java.util.Properties;
import java.util.concurrent.TimeUnit;
import java.util.stream.Collectors; import java.util.stream.Collectors;
import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -69,6 +70,9 @@ public class PythonScriptEngineConfiguration {
private static final int DEBUGGER_PORT_DEFAULT = 9230; private static final int DEBUGGER_PORT_DEFAULT = 9230;
/** The default lock acquisition timeout in seconds */
private static final long LOCK_ACQUISITION_TIMEOUT_DEFAULT = 5L;
// The variable names must match the configuration keys in config.xml // The variable names must match the configuration keys in config.xml
public static class PythonScriptingConfiguration { public static class PythonScriptingConfiguration {
public int injectionEnabled = INJECTION_ENABLED_FOR_SCRIPT_MODULES_ONLY; public int injectionEnabled = INJECTION_ENABLED_FOR_SCRIPT_MODULES_ONLY;
@@ -78,6 +82,7 @@ public class PythonScriptEngineConfiguration {
public boolean debuggerEnabled = false; public boolean debuggerEnabled = false;
public int debuggerPort = DEBUGGER_PORT_DEFAULT; public int debuggerPort = DEBUGGER_PORT_DEFAULT;
public String pipModules = ""; public String pipModules = "";
public long lockAcquisitionTimeout = LOCK_ACQUISITION_TIMEOUT_DEFAULT;
} }
private PythonScriptingConfiguration configuration = new PythonScriptingConfiguration(); private PythonScriptingConfiguration configuration = new PythonScriptingConfiguration();
@@ -166,6 +171,7 @@ public class PythonScriptEngineConfiguration {
String oldPipModules = configuration.pipModules; String oldPipModules = configuration.pipModules;
boolean oldDebuggerEnabled = configuration.debuggerEnabled; boolean oldDebuggerEnabled = configuration.debuggerEnabled;
int oldDebuggerPort = configuration.debuggerPort; int oldDebuggerPort = configuration.debuggerPort;
long oldLockAcquisitionTimeout = configuration.lockAcquisitionTimeout;
configuration = new Configuration(config).as(PythonScriptingConfiguration.class); configuration = new Configuration(config).as(PythonScriptingConfiguration.class);
@@ -188,6 +194,11 @@ public class PythonScriptEngineConfiguration {
} else if (oldDebuggerPort != configuration.debuggerPort) { } else if (oldDebuggerPort != configuration.debuggerPort) {
logger.warn("Reconfigured debugger for Python Scripting. Restart openHAB to apply this change."); logger.warn("Reconfigured debugger for Python Scripting. Restart openHAB to apply this change.");
} }
if (oldLockAcquisitionTimeout != configuration.lockAcquisitionTimeout) {
logger.warn(
"Python Scripting lock acquisition timeout changed from {} to {} seconds. Rules created with JavaScript scripts might need to be reloaded for the changes to apply.",
oldLockAcquisitionTimeout, configuration.lockAcquisitionTimeout);
}
} }
public void setHelperLibVersion(Version version) { public void setHelperLibVersion(Version version) {
@@ -270,6 +281,13 @@ public class PythonScriptEngineConfiguration {
return installedHelperLibVersion; return installedHelperLibVersion;
} }
/**
* @return The log acquisition timeout in milliseconds.
*/
public long getLockAcquisitionTimeout() {
return TimeUnit.SECONDS.toMillis(configuration.lockAcquisitionTimeout);
}
/** /**
* Returns the current configuration as a map. * Returns the current configuration as a map.
* This is used to display the configuration in the console. * This is used to display the configuration in the console.
@@ -75,6 +75,14 @@
<default>false</default> <default>false</default>
<advanced>true</advanced> <advanced>true</advanced>
</parameter> </parameter>
<parameter name="lockAcquisitionTimeout" type="integer" min="1" max="120" step="1" groupName="system">
<label>Engine Lock Acquisition Timeout</label>
<description>The script engines must be reserved only for a specific thread before executing a script. This setting
decides how many seconds the system will try to acquire the engine lock before giving up. If you have long running
scripts that might be triggered faster than they can finish, you might want to use a high value.</description>
<default>5</default>
<advanced>true</advanced>
</parameter>
<!-- Debugger --> <!-- Debugger -->
<parameter name="debuggerEnabled" type="boolean" required="true" groupName="debugger"> <parameter name="debuggerEnabled" type="boolean" required="true" groupName="debugger">
@@ -27,5 +27,7 @@ automation.config.pythonscripting.injectionEnabled.option.1 = Disable auto-injec
automation.config.pythonscripting.injectionEnabled.option.0 = Disable completely automation.config.pythonscripting.injectionEnabled.option.0 = Disable completely
automation.config.pythonscripting.jythonEmulation.label = Enable Jython emulation automation.config.pythonscripting.jythonEmulation.label = Enable Jython emulation
automation.config.pythonscripting.jythonEmulation.description = This enables Jython emulation in GraalPy. It is strongly recommended to update code to GraalPy and Python 3 as the emulation can have performance degradation. For tips and instructions, please refer to <a href="https://www.graalvm.org/latest/reference-manual/python/Modern-Python-on-JVM">Jython Migration Guide</a>. automation.config.pythonscripting.jythonEmulation.description = This enables Jython emulation in GraalPy. It is strongly recommended to update code to GraalPy and Python 3 as the emulation can have performance degradation. For tips and instructions, please refer to <a href="https://www.graalvm.org/latest/reference-manual/python/Modern-Python-on-JVM">Jython Migration Guide</a>.
automation.config.pythonscripting.lockAcquisitionTimeout.label = Engine Lock Acquisition Timeout
automation.config.pythonscripting.lockAcquisitionTimeout.description = The script engines must be reserved only for a specific thread before executing a script. This setting decides how many seconds the system will try to acquire the engine lock before giving up. If you have long running scripts that might be triggered faster than they can finish, you might want to use a high value.
automation.config.pythonscripting.pipModules.label = Python pip modules (requires a manually configured venv) automation.config.pythonscripting.pipModules.label = Python pip modules (requires a manually configured venv)
automation.config.pythonscripting.pipModules.description = A comma separated list of Python modules to install. Versions may be constrained by separating with an <code>==</code> followed by standard python pip version constraint, such as "<code>tzdata==2025.2</code>". automation.config.pythonscripting.pipModules.description = A comma separated list of Python modules to install. Versions may be constrained by separating with an <code>==</code> followed by standard python pip version constraint, such as "<code>tzdata==2025.2</code>".