OpenHAB
/
openhab-core
mirror of https://github.com/openhab/openhab-core.git
1
0
Fork 0
Code Issues Projects Releases Wiki Activity

Wrap ScriptEngine executions in a SafeCaller (#3180) 

Fixes #3179

Signed-off-by: Cody Cutrer <cody@cutrer.us>
pull/3147/head
Cody Cutrer 2022-11-30 14:36:49 -07:00 committed by GitHub
parent f64874e226
commit ff75505e70
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 50 additions and 30 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

77
bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java
View File

@ -36,6 +36,7 @@ import org.openhab.core.automation.module.script.ScriptEngineContainer;
import org.openhab.core.automation.module.script.ScriptEngineFactory;
import org.openhab.core.automation.module.script.ScriptEngineManager;
import org.openhab.core.automation.module.script.ScriptExtensionManagerWrapper;
import org.openhab.core.common.SafeCaller;
import org.openhab.core.common.ThreadPoolManager;
import org.osgi.service.component.annotations.Activate;
import org.osgi.service.component.annotations.Component;
@ -54,9 +55,14 @@ import org.slf4j.LoggerFactory;
@NonNullByDefault
@Component(service = ScriptEngineManager.class)
public class ScriptEngineManagerImpl implements ScriptEngineManager {
/**
* Timeout for scripts in milliseconds.
*/
static private final long TIMEOUT = TimeUnit.SECONDS.toMillis(30);
private final ScheduledExecutorService scheduler = ThreadPoolManager
.getScheduledPool(ThreadPoolManager.THREAD_POOL_NAME_COMMON);
private final SafeCaller safeCaller;
private final Logger logger = LoggerFactory.getLogger(ScriptEngineManagerImpl.class);
private final Map<String, ScriptEngineContainer> loadedScriptEngineInstances = new HashMap<>();
@ -66,8 +72,10 @@ public class ScriptEngineManagerImpl implements ScriptEngineManager {
private final Set<FactoryChangeListener> listeners = new HashSet<>();
@Activate
public ScriptEngineManagerImpl(final @Reference ScriptExtensionManager scriptExtensionManager) {
public ScriptEngineManagerImpl(final @Reference ScriptExtensionManager scriptExtensionManager,
final @Reference SafeCaller safeCaller) {
this.scriptExtensionManager = scriptExtensionManager;
this.safeCaller = safeCaller;
}
@Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
@ -181,22 +189,15 @@ public class ScriptEngineManagerImpl implements ScriptEngineManager {
} else {
ScriptEngine engine = container.getScriptEngine();
try {
engine.eval(scriptData);
if (engine instanceof Invocable) {
Invocable inv = (Invocable) engine;
try {
inv.invokeFunction("scriptLoaded", engineIdentifier);
} catch (NoSuchMethodException e) {
logger.trace("scriptLoaded() is not defined in the script: {}", engineIdentifier);
}
} else {
logger.trace("ScriptEngine does not support Invocable interface");
safeCall(engineIdentifier, () -> {
try {
engine.eval(scriptData);
} catch (Exception ex) {
logger.error("Error during evaluation of script '{}': {}", engineIdentifier, ex.getMessage());
logger.debug("", ex);
}
} catch (Exception ex) {
logger.error("Error during evaluation of script '{}': {}", engineIdentifier, ex.getMessage());
logger.debug("", ex);
}
}, true);
callHook(engineIdentifier, engine, "scriptLoaded", true, engineIdentifier);
}
}
@ -209,21 +210,10 @@ public class ScriptEngineManagerImpl implements ScriptEngineManager {
tracker.removeTracking(engineIdentifier);
}
ScriptEngine scriptEngine = container.getScriptEngine();
if (scriptEngine instanceof Invocable) {
Invocable inv = (Invocable) scriptEngine;
try {
inv.invokeFunction("scriptUnloaded");
} catch (NoSuchMethodException e) {
logger.trace("scriptUnloaded() is not defined in the script");
} catch (ScriptException ex) {
logger.error("Error while executing script", ex);
}
} else {
logger.trace("ScriptEngine does not support Invocable interface");
}
callHook(engineIdentifier, scriptEngine, "scriptUnloaded", false);
if (scriptEngine instanceof AutoCloseable) {
// we cannot not use ScheduledExecutorService.execute here as it might execute the task in the calling
// we cannot use ScheduledExecutorService.execute here as it might execute the task in the calling
// thread (calling ScriptEngine.close in the same thread may result in a deadlock if the ScriptEngine
// tries to Thread.join)
scheduler.schedule(() -> {
@ -294,4 +284,33 @@ public class ScriptEngineManagerImpl implements ScriptEngineManager {
public void removeFactoryChangeListener(FactoryChangeListener listener) {
listeners.remove(listener);
}
private void safeCall(String engineIdentifier, Runnable r, boolean unloadOnTimeout) {
safeCaller.create(r, Runnable.class).withTimeout(TIMEOUT).onTimeout(() -> {
logger.warn("Script evaluation of '{}' takes more than {}ms", engineIdentifier, TIMEOUT);
if (unloadOnTimeout) {
removeEngine(engineIdentifier);
}
}).build().run();
}
private void callHook(String engineIdentifier, ScriptEngine engine, String hook, boolean unloadOnTimeout,
Object... args) {
if (!(engine instanceof Invocable)) {
logger.trace("ScriptEngine does not support Invocable interface");
return;
}
safeCall(engineIdentifier, () -> {
Invocable inv = (Invocable) engine;
try {
inv.invokeFunction(hook, args);
} catch (NoSuchMethodException e) {
logger.trace("{}() is not defined in the script '{}'", hook, engineIdentifier);
} catch (ScriptException ex) {
logger.error("Error while executing script '{}': {}", engineIdentifier, ex.getMessage());
logger.debug("", ex);
}
}, unloadOnTimeout);
}
}

3
bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java
View File

@ -40,6 +40,7 @@ import org.mockito.junit.jupiter.MockitoSettings;
import org.mockito.quality.Strictness;
import org.openhab.core.automation.module.script.ScriptDependencyTracker;
import org.openhab.core.automation.module.script.ScriptEngineFactory;
import org.openhab.core.internal.common.SafeCallerImpl;
/**
* The {@link ScriptEngineManagerImplTest} is a test class for the {@link ScriptEngineManagerImpl}
@ -73,7 +74,7 @@ public class ScriptEngineManagerImplTest {
when(scriptEngineFactoryMock.getDependencyTracker()).thenReturn(scriptDependencyTrackerMock);
when(scriptDependencyTrackerMock.getTracker(any())).thenReturn(dependencyListenerMock);
scriptEngineManager = new ScriptEngineManagerImpl(scriptExtensionManagerMock);
scriptEngineManager = new ScriptEngineManagerImpl(scriptExtensionManagerMock, new SafeCallerImpl(null));
scriptEngineManager.addScriptEngineFactory(scriptEngineFactoryMock);
}