Skip to content

Commit 22f29c5

Browse files
NadaharNadahar
authored
[automation] Enable asynchronous execution of rules and lock engine lock when loading scripts (#5635)
Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
1 parent 29cfacf commit 22f29c5

11 files changed

Lines changed: 459 additions & 77 deletions

File tree

‎bundles/org.openhab.core.automation.module.script.rulesupport/src/main/java/org/openhab/core/automation/module/script/rulesupport/internal/delegates/SimpleTriggerHandlerCallbackDelegate.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
package org.openhab.core.automation.module.script.rulesupport.internal.delegates;
1414

1515
import java.util.Map;
16+
import java.util.concurrent.Future;
1617
import java.util.concurrent.ScheduledExecutorService;
1718

1819
import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -82,4 +83,15 @@ public void runNow(String uid) {
8283
public void runNow(String uid, boolean considerConditions, @Nullable Map<String, @Nullable Object> context) {
8384
callback.runNow(uid, considerConditions, context);
8485
}
86+
87+
@Override
88+
public Future<Map<String, @Nullable Object>> runAsync(String ruleUID) {
89+
return callback.runAsync(ruleUID);
90+
}
91+
92+
@Override
93+
public Future<Map<String, @Nullable Object>> runAsync(String ruleUID, boolean considerConditions,
94+
@Nullable Map<String, @Nullable Object> context) {
95+
return callback.runAsync(ruleUID, considerConditions, context);
96+
}
8597
}

‎bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/LockableScriptEngine.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/*
2+
* Copyright (c) 2010-2026 Contributors to the openHAB project
3+
*
4+
* See the NOTICE file(s) distributed with this work for additional
5+
* information.
6+
*
7+
* This program and the accompanying materials are made available under the
8+
* terms of the Eclipse Public License 2.0 which is available at
9+
* http://www.eclipse.org/legal/epl-2.0
10+
*
11+
* SPDX-License-Identifier: EPL-2.0
12+
*/
13+
package org.openhab.core.automation.module.script;
14+
15+
import java.util.concurrent.locks.Lock;
16+
17+
import javax.script.ScriptEngine;
18+
19+
import org.eclipse.jdt.annotation.NonNullByDefault;
20+
21+
/**
22+
* This interface is used to indicate that a {@link ScriptEngine} is lockable, i.e. that it doesn't support
23+
* concurrency and should run scripts inside a lock.
24+
*
25+
* @author Ravi Nadahar - Initial contribution
26+
*/
27+
@NonNullByDefault
28+
public interface LockableScriptEngine extends ScriptEngine {
29+
30+
/**
31+
* @return The {@link Lock} instance that should be used to guard script execution.
32+
*/
33+
Lock getLock();
34+
35+
/**
36+
* Get the lock acquisition timeout for use with {@link Lock#tryLock(long, java.util.concurrent.TimeUnit)}. The
37+
* lock should always be acquired with a timeout to avoid deadlocks.
38+
*
39+
* @return The timeout period to wait when trying to acquire the {@link Lock} in milliseconds before considering the
40+
* acquisition a failure.
41+
*
42+
* @implNote A default implementation with a 5 seconds timeout exists.
43+
*/
44+
default long getLockAcquisitionTimeoutMs() {
45+
return 5000L;
46+
}
47+
}

‎bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java‎

Lines changed: 59 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import java.util.Set;
2323
import java.util.concurrent.ScheduledExecutorService;
2424
import java.util.concurrent.TimeUnit;
25+
import java.util.concurrent.locks.Lock;
2526

2627
import javax.script.Invocable;
2728
import javax.script.ScriptContext;
@@ -31,6 +32,7 @@
3132

3233
import org.eclipse.jdt.annotation.NonNullByDefault;
3334
import org.eclipse.jdt.annotation.Nullable;
35+
import org.openhab.core.automation.module.script.LockableScriptEngine;
3436
import org.openhab.core.automation.module.script.ScriptDependencyTracker;
3537
import org.openhab.core.automation.module.script.ScriptEngineContainer;
3638
import org.openhab.core.automation.module.script.ScriptEngineFactory;
@@ -159,32 +161,69 @@ public boolean loadScript(String engineIdentifier, InputStreamReader scriptData)
159161
ScriptEngineContainer container = loadedScriptEngineInstances.get(engineIdentifier);
160162
if (container == null) {
161163
logger.error("Could not load script, as no ScriptEngine has been created");
162-
} else {
163-
ScriptEngine engine = container.getScriptEngine();
164+
return false;
165+
}
166+
ScriptEngine engine = container.getScriptEngine();
167+
if (engine instanceof LockableScriptEngine lockable) {
168+
Lock lock = lockable.getLock();
169+
long timeout = lockable.getLockAcquisitionTimeoutMs();
170+
boolean locked;
164171
try {
165-
engine.eval(scriptData);
166-
if (engine instanceof Invocable inv) {
167-
try {
168-
inv.invokeFunction("scriptLoaded", engineIdentifier);
169-
} catch (NoSuchMethodException e) {
170-
logger.trace("scriptLoaded() is not defined in the script: {}", engineIdentifier);
171-
}
172+
locked = lock.tryLock(timeout, TimeUnit.MILLISECONDS);
173+
} catch (InterruptedException e) {
174+
Thread.currentThread().interrupt();
175+
logger.error("Interrupted while waiting to acquire the lock while loading script for engine '{}'",
176+
engineIdentifier);
177+
logger.trace("", e);
178+
return false;
179+
}
180+
if (locked) {
181+
try {
182+
return runScript(engineIdentifier, engine, scriptData);
183+
} finally {
184+
lock.unlock();
185+
}
186+
} else {
187+
if (timeout < 2000L) {
188+
logger.error(
189+
"Failed to acquire the lock while loading script for engine '{}' within {} milliseconds. Aborting loading of script.",
190+
engineIdentifier, timeout);
172191
} else {
173-
logger.trace("ScriptEngine does not support Invocable interface");
192+
logger.error(
193+
"Failed to acquire the lock while loading script for engine '{}' within {} seconds. Aborting loading of script.",
194+
engineIdentifier, TimeUnit.MILLISECONDS.toSeconds(timeout));
174195
}
175-
return true;
176-
} catch (Exception ex) {
177-
logger.error("Error during evaluation of script '{}': {}", engineIdentifier, ex.getMessage());
178-
// Only call logger if debug level is actually enabled, because OPS4J Pax Logging holds (at least for
179-
// some time) a reference to the exception and its cause, which may hold a reference to the script
180-
// engine.
181-
// This prevents garbage collection (at least for some time) to remove the script engine from heap.
182-
if (logger.isDebugEnabled()) {
183-
logger.debug("", ex);
196+
return false;
197+
}
198+
} else {
199+
return runScript(engineIdentifier, engine, scriptData);
200+
}
201+
}
202+
203+
private boolean runScript(String engineIdentifier, ScriptEngine engine, InputStreamReader scriptData) {
204+
try {
205+
engine.eval(scriptData);
206+
if (engine instanceof Invocable inv) {
207+
try {
208+
inv.invokeFunction("scriptLoaded", engineIdentifier);
209+
} catch (NoSuchMethodException e) {
210+
logger.trace("scriptLoaded() is not defined in the script: {}", engineIdentifier);
184211
}
212+
} else {
213+
logger.trace("ScriptEngine does not support Invocable interface");
214+
}
215+
return true;
216+
} catch (Exception ex) {
217+
logger.error("Error during evaluation of script '{}': {}", engineIdentifier, ex.getMessage());
218+
// Only call logger if debug level is actually enabled, because OPS4J Pax Logging holds (at least for
219+
// some time) a reference to the exception and its cause, which may hold a reference to the script
220+
// engine.
221+
// This prevents garbage collection (at least for some time) to remove the script engine from heap.
222+
if (logger.isDebugEnabled()) {
223+
logger.debug("", ex);
185224
}
225+
return false;
186226
}
187-
return false;
188227
}
189228

190229
@Override

‎bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/ScriptActionHandler.java‎

Lines changed: 21 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import org.eclipse.jdt.annotation.Nullable;
2626
import org.openhab.core.automation.Action;
2727
import org.openhab.core.automation.handler.ActionHandler;
28+
import org.openhab.core.automation.module.script.LockableScriptEngine;
2829
import org.openhab.core.automation.module.script.ScriptEngineManager;
2930
import org.slf4j.Logger;
3031
import org.slf4j.LoggerFactory;
@@ -85,25 +86,38 @@ public void compile() throws ScriptException {
8586
}
8687

8788
ScriptEngine scriptEngine = getScriptEngine();
89+
Lock lock = null;
90+
long timeout = 0L;
91+
if (scriptEngine instanceof LockableScriptEngine lockable) {
92+
lock = lockable.getLock();
93+
timeout = lockable.getLockAcquisitionTimeoutMs();
94+
}
8895
if (scriptEngine != null) {
8996
try {
90-
if (scriptEngine instanceof Lock lock && !lock.tryLock(1, TimeUnit.MINUTES)) {
91-
logger.error(
92-
"Failed to acquire lock within one minute for script module '{}' of rule with UID '{}'",
93-
module.getId(), ruleUID);
97+
if (lock != null && !lock.tryLock(timeout, TimeUnit.MILLISECONDS)) {
98+
if (timeout < 2000L) {
99+
logger.error(
100+
"Failed to acquire lock within {} milliseconds for script module '{}' of rule with UID '{}'",
101+
timeout, module.getId(), ruleUID);
102+
103+
} else {
104+
logger.error(
105+
"Failed to acquire lock within {} seconds for script module '{}' of rule with UID '{}'",
106+
TimeUnit.MILLISECONDS.toSeconds(timeout), module.getId(), ruleUID);
107+
}
94108
return resultMap;
95109
}
96110
} catch (InterruptedException e) {
111+
Thread.currentThread().interrupt();
97112
throw new RuntimeException(e);
98113
}
99114
try {
100115
setExecutionContext(scriptEngine, context);
101116
Object result = eval(scriptEngine);
102117
resultMap.put("result", result);
103118
resetExecutionContext(scriptEngine, context);
104-
} finally { // Make sure that Lock is unlocked regardless of an exception being thrown or not to avoid
105-
// deadlocks
106-
if (scriptEngine instanceof Lock lock) {
119+
} finally {
120+
if (lock != null) {
107121
lock.unlock();
108122
}
109123
}

‎bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/handler/ScriptConditionHandler.java‎

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.eclipse.jdt.annotation.NonNullByDefault;
2323
import org.openhab.core.automation.Condition;
2424
import org.openhab.core.automation.handler.ConditionHandler;
25+
import org.openhab.core.automation.module.script.LockableScriptEngine;
2526
import org.openhab.core.automation.module.script.ScriptEngineManager;
2627
import org.slf4j.Logger;
2728
import org.slf4j.LoggerFactory;
@@ -64,16 +65,29 @@ public boolean isSatisfied(final Map<String, Object> context) {
6465
}
6566

6667
ScriptEngine scriptEngine = getScriptEngine();
68+
Lock lock = null;
69+
long timeout = 0L;
70+
if (scriptEngine instanceof LockableScriptEngine lockable) {
71+
lock = lockable.getLock();
72+
timeout = lockable.getLockAcquisitionTimeoutMs();
73+
}
6774

6875
if (scriptEngine != null) {
6976
try {
70-
if (scriptEngine instanceof Lock lock && !lock.tryLock(1, TimeUnit.MINUTES)) {
71-
logger.error(
72-
"Failed to acquire lock within one minute for script module '{}' of rule with UID '{}'",
73-
module.getId(), ruleUID);
77+
if (lock != null && !lock.tryLock(timeout, TimeUnit.MILLISECONDS)) {
78+
if (timeout < 2000L) {
79+
logger.error(
80+
"Failed to acquire lock within {} milliseconds for script module '{}' of rule with UID '{}'",
81+
timeout, module.getId(), ruleUID);
82+
} else {
83+
logger.error(
84+
"Failed to acquire lock within {} seconds for script module '{}' of rule with UID '{}'",
85+
TimeUnit.MILLISECONDS.toSeconds(timeout), module.getId(), ruleUID);
86+
}
7487
return result;
7588
}
7689
} catch (InterruptedException e) {
90+
Thread.currentThread().interrupt();
7791
throw new RuntimeException(e);
7892
}
7993
try {
@@ -86,9 +100,8 @@ public boolean isSatisfied(final Map<String, Object> context) {
86100
returnVal);
87101
}
88102
resetExecutionContext(scriptEngine, context);
89-
} finally { // Make sure that Lock is unlocked regardless of an exception being thrown or not to avoid
90-
// deadlocks
91-
if (scriptEngine instanceof Lock lock) {
103+
} finally {
104+
if (lock != null) {
92105
lock.unlock();
93106
}
94107
}

‎bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/ModuleHandlerCallback.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
package org.openhab.core.automation;
1414

1515
import java.util.Map;
16+
import java.util.concurrent.Future;
1617

1718
import org.eclipse.jdt.annotation.NonNullByDefault;
1819
import org.eclipse.jdt.annotation.Nullable;
@@ -92,4 +93,45 @@ public interface ModuleHandlerCallback {
9293
* @param context the context that is passed to the conditions and the actions of the rule.
9394
*/
9495
void runNow(String uid, boolean considerConditions, @Nullable Map<String, @Nullable Object> context);
96+
97+
/**
98+
* The method skips triggers and conditions and executes the actions of the rule asynchronously.
99+
* This should always be possible unless an action has a mandatory input that is linked to a trigger.
100+
* In that case the action is skipped and the rule engine continues execution of remaining actions.
101+
* <p>
102+
* <b>Note:</b> Unlike {@link #runNow(String)}, this method will return immediately. To wait for the execution to be
103+
* completed, call {@link Future#get()} on the returned {@link Future}.
104+
*
105+
* @param ruleUID uid of the rule whose actions should be executed.
106+
* @return A {@link Future} containing the copy of the rule context after completion, including possible return
107+
* values.
108+
* @throws UnsupportedOperationException If asynchronous execution isn't supported by the {@link RuleManager}
109+
* implementation.
110+
*
111+
* @implNote The default implementation simply calls {@link #runAsync(String, boolean, Map)}.
112+
*/
113+
default Future<Map<String, @Nullable Object>> runAsync(String ruleUID) {
114+
return runAsync(ruleUID, false, null);
115+
}
116+
117+
/**
118+
* Same as {@link #runAsync(String)} with the additional option to enable/disable evaluation of
119+
* conditions defined in the target rule. The context can be set here, too, but might also be {@code null}.
120+
* <p>
121+
* <b>Note:</b> Unlike {@link #runNow(String, boolean, Map)}, this method will return immediately. To wait for the
122+
* execution to be completed, call {@link Future#get()} on the returned {@link Future}.
123+
*
124+
* @param ruleUID uid of the rule whose actions should be executed.
125+
* @param considerConditions if {@code true} the conditions of the rule will be checked.
126+
* @param context the context that is passed to the conditions and the actions of the rule.
127+
* @return a copy of the rule context, including possible return values
128+
* @throws UnsupportedOperationException If asynchronous execution isn't supported by the {@link RuleManager}
129+
* implementation.
130+
*
131+
* @implNote The default implementation throws an {@link UnsupportedOperationException}.
132+
*/
133+
default Future<Map<String, @Nullable Object>> runAsync(String ruleUID, boolean considerConditions,
134+
@Nullable Map<String, @Nullable Object> context) {
135+
throw new UnsupportedOperationException("runAsync() isn't implemented by " + getClass().getName());
136+
}
95137
}

‎bundles/org.openhab.core.automation/src/main/java/org/openhab/core/automation/RuleManager.java‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
import java.time.ZonedDateTime;
1616
import java.util.Map;
17+
import java.util.concurrent.Future;
1718
import java.util.stream.Stream;
1819

1920
import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -100,6 +101,47 @@ public interface RuleManager {
100101
Map<String, @Nullable Object> runNow(String uid, boolean considerConditions,
101102
@Nullable Map<String, @Nullable Object> context);
102103

104+
/**
105+
* The method skips triggers and conditions and executes the actions of the rule asynchronously.
106+
* This should always be possible unless an action has a mandatory input that is linked to a trigger.
107+
* In that case the action is skipped and the rule engine continues execution of remaining actions.
108+
* <p>
109+
* <b>Note:</b> Unlike {@link #runNow(String)}, this method will return immediately. To wait for the execution to be
110+
* completed, call {@link Future#get()} on the returned {@link Future}.
111+
*
112+
* @param ruleUID uid of the rule whose actions should be executed.
113+
* @return A {@link Future} containing the copy of the rule context after completion, including possible return
114+
* values.
115+
* @throws UnsupportedOperationException If asynchronous execution isn't supported by the {@link RuleManager}
116+
* implementation.
117+
*
118+
* @implNote The default implementation simply calls {@link #runAsync(String, boolean, Map)}.
119+
*/
120+
default Future<Map<String, @Nullable Object>> runAsync(String ruleUID) {
121+
return runAsync(ruleUID, false, null);
122+
}
123+
124+
/**
125+
* Same as {@link #runAsync(String)} with the additional option to enable/disable evaluation of
126+
* conditions defined in the target rule. The context can be set here, too, but might also be {@code null}.
127+
* <p>
128+
* <b>Note:</b> Unlike {@link #runNow(String, boolean, Map)}, this method will return immediately. To wait for the
129+
* execution to be completed, call {@link Future#get()} on the returned {@link Future}.
130+
*
131+
* @param ruleUID uid of the rule whose actions should be executed.
132+
* @param considerConditions if {@code true} the conditions of the rule will be checked.
133+
* @param context the context that is passed to the conditions and the actions of the rule.
134+
* @return a copy of the rule context, including possible return values
135+
* @throws UnsupportedOperationException If asynchronous execution isn't supported by the {@link RuleManager}
136+
* implementation.
137+
*
138+
* @implNote The default implementation throws an {@link UnsupportedOperationException}.
139+
*/
140+
default Future<Map<String, @Nullable Object>> runAsync(String ruleUID, boolean considerConditions,
141+
@Nullable Map<String, @Nullable Object> context) {
142+
throw new UnsupportedOperationException("runAsync() isn't implemented by " + getClass().getName());
143+
}
144+
103145
/**
104146
* Simulates the execution of all rules with tag 'Schedule' for the given time interval.
105147
* The result is sorted ascending by execution time.

0 commit comments

Comments
 (0)