demus/openhab-core
1
0
Fork 0
mirror of https://github.com/danieldemus/openhab-core.git synced 2025-01-10 13:21:53 +01:00
Code Issues 1 Packages Projects Releases Wiki Activity

[automation] Synchronize script action/condition execution if engine implements Lock (#4426)

Browse Source 
This moves the locking mechanism added in #4402 to the inheritors of AbstractScriptModuleHandler
to synchronize execution context access as well.

This fixes the problem thathttps://github.com/openhab/openhab-addons/pull/17510 worked around by using Thread.sleep.

Signed-off-by: Florian Hotze <dev@florianhotze.com>
This commit is contained in:
Florian Hotze 2024-10-26 23:26:20 +02:00 committed by GitHub
parent 52b26baf17
commit 922a2068c0
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 57 additions and 31 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

20
bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/AbstractScriptModuleHandler.java
View File

@ -17,8 +17,6 @@ import java.util.Map;
import java.util.Map.Entry;
import java.util.Optional;
import java.util.UUID;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Lock;
import javax.script.Compilable;
import javax.script.CompiledScript;
@ -38,6 +36,8 @@ import org.slf4j.LoggerFactory;
/**
* This is an abstract class that can be used when implementing any module handler that handles scripts.
* <p>
* Remember to implement multi-thread synchronization in the concrete handler if the script engine is not thread-safe!
*
* @author Kai Kreuzer - Initial contribution
* @author Simon Merschjohann - Initial contribution
@ -212,28 +212,14 @@ public abstract class AbstractScriptModuleHandler<T extends Module> extends Base
protected @Nullable Object eval(ScriptEngine engine, String script) {
try {
if (compiledScript.isPresent()) {
if (engine instanceof Lock lock && !lock.tryLock(1, TimeUnit.MINUTES)) {
logger.error("Failed to acquire lock within one minute for script module of rule with UID '{}'",
ruleUID);
return null;
}
logger.debug("Executing pre-compiled script of rule with UID '{}'", ruleUID);
try {
return compiledScript.get().eval(engine.getContext());
} finally { // Make sure that Lock is unlocked regardless of an exception being thrown or not to avoid
// deadlocks
if (engine instanceof Lock lock) {
lock.unlock();
}
}
return compiledScript.get().eval(engine.getContext());
}
logger.debug("Executing script of rule with UID '{}'", ruleUID);
return engine.eval(script);
} catch (ScriptException e) {
logger.error("Script execution of rule with UID '{}' failed: {}", ruleUID, e.getMessage(),
logger.isDebugEnabled() ? e : null);
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
return null;
}

30
bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/ScriptActionHandler.java
View File

@ -14,6 +14,8 @@ package org.openhab.core.automation.module.script.internal.handler;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Lock;
import java.util.function.Consumer;
import javax.script.ScriptException;
@ -31,7 +33,8 @@ import org.slf4j.LoggerFactory;
*
* @author Kai Kreuzer - Initial contribution
* @author Simon Merschjohann - Initial contribution
* @author Florian Hotze - Add support for script pre-compilation
* @author Florian Hotze - Add support for script pre-compilation, Synchronize script context access if the ScriptEngine
* implements locking
*/
@NonNullByDefault
public class ScriptActionHandler extends AbstractScriptModuleHandler<Action> implements ActionHandler {
@ -76,10 +79,27 @@ public class ScriptActionHandler extends AbstractScriptModuleHandler<Action> imp
}
getScriptEngine().ifPresent(scriptEngine -> {
setExecutionContext(scriptEngine, context);
Object result = eval(scriptEngine, script);
resultMap.put("result", result);
resetExecutionContext(scriptEngine, context);
try {
if (scriptEngine instanceof Lock lock && !lock.tryLock(1, TimeUnit.MINUTES)) {
logger.error(
"Failed to acquire lock within one minute for script module '{}' of rule with UID '{}'",
module.getId(), ruleUID);
return;
}
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
try {
setExecutionContext(scriptEngine, context);
Object result = eval(scriptEngine, script);
resultMap.put("result", result);
resetExecutionContext(scriptEngine, context);
} finally { // Make sure that Lock is unlocked regardless of an exception being thrown or not to avoid
// deadlocks
if (scriptEngine instanceof Lock lock) {
lock.unlock();
}
}
});
return resultMap;

38
bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/ScriptConditionHandler.java
View File

@ -14,6 +14,8 @@ package org.openhab.core.automation.module.script.internal.handler;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.locks.Lock;
import javax.script.ScriptEngine;
import javax.script.ScriptException;
@ -30,7 +32,8 @@ import org.slf4j.LoggerFactory;
*
* @author Kai Kreuzer - Initial contribution
* @author Simon Merschjohann - Initial contribution
* @author Florian Hotze - Add support for script pre-compilation
* @author Florian Hotze - Add support for script pre-compilation, Synchronize script context access if the ScriptEngine
* implements locking
*/
@NonNullByDefault
public class ScriptConditionHandler extends AbstractScriptModuleHandler<Condition> implements ConditionHandler {
@ -60,15 +63,32 @@ public class ScriptConditionHandler extends AbstractScriptModuleHandler<Conditio
if (engine.isPresent()) {
ScriptEngine scriptEngine = engine.get();
setExecutionContext(scriptEngine, context);
Object returnVal = eval(scriptEngine, script);
if (returnVal instanceof Boolean boolean1) {
result = boolean1;
} else {
logger.error("Script of rule with UID '{}' did not return a boolean value, but '{}'", ruleUID,
returnVal);
try {
if (scriptEngine instanceof Lock lock && !lock.tryLock(1, TimeUnit.MINUTES)) {
logger.error(
"Failed to acquire lock within one minute for script module '{}' of rule with UID '{}'",
module.getId(), ruleUID);
return result;
}
} catch (InterruptedException e) {
throw new RuntimeException(e);
}
try {
setExecutionContext(scriptEngine, context);
Object returnVal = eval(scriptEngine, script);
if (returnVal instanceof Boolean boolean1) {
result = boolean1;
} else {
logger.error("Script of rule with UID '{}' did not return a boolean value, but '{}'", ruleUID,
returnVal);
}
resetExecutionContext(scriptEngine, context);
} finally { // Make sure that Lock is unlocked regardless of an exception being thrown or not to avoid
// deadlocks
if (scriptEngine instanceof Lock lock) {
lock.unlock();
}
}
resetExecutionContext(scriptEngine, context);
}
return result;