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

Revert "Wrap ScriptEngine executions in a SafeCaller (#3180)" (#3227) 

This reverts commit ff75505e70.

Signed-off-by: Cody Cutrer <cody@cutrer.us>
pull/3236/head
Cody Cutrer 2022-12-13 11:16:27 -07:00 committed by GitHub
parent d4ea62b983
commit 1d8d04f75e
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 30 additions and 50 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,7 +36,6 @@ 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;
@ -55,14 +54,9 @@ 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<>();
@ -72,10 +66,8 @@ public class ScriptEngineManagerImpl implements ScriptEngineManager {
private final Set<FactoryChangeListener> listeners = new HashSet<>();
@Activate
public ScriptEngineManagerImpl(final @Reference ScriptExtensionManager scriptExtensionManager,
final @Reference SafeCaller safeCaller) {
public ScriptEngineManagerImpl(final @Reference ScriptExtensionManager scriptExtensionManager) {
this.scriptExtensionManager = scriptExtensionManager;
this.safeCaller = safeCaller;
}
@Reference(cardinality = ReferenceCardinality.MULTIPLE, policy = ReferencePolicy.DYNAMIC)
@ -189,15 +181,22 @@ public class ScriptEngineManagerImpl implements ScriptEngineManager {
} else {
ScriptEngine engine = container.getScriptEngine();
safeCall(engineIdentifier, () -> {
try {
engine.eval(scriptData);
} catch (Exception ex) {
logger.error("Error during evaluation of script '{}': {}", engineIdentifier, ex.getMessage());
logger.debug("", ex);
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");
}
}, true);
callHook(engineIdentifier, engine, "scriptLoaded", true, engineIdentifier);
} catch (Exception ex) {
logger.error("Error during evaluation of script '{}': {}", engineIdentifier, ex.getMessage());
logger.debug("", ex);
}
}
}
@ -210,10 +209,21 @@ public class ScriptEngineManagerImpl implements ScriptEngineManager {
tracker.removeTracking(engineIdentifier);
}
ScriptEngine scriptEngine = container.getScriptEngine();
callHook(engineIdentifier, scriptEngine, "scriptUnloaded", false);
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");
}
if (scriptEngine instanceof AutoCloseable) {
// we cannot use ScheduledExecutorService.execute here as it might execute the task in the calling
// we cannot not 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(() -> {
@ -284,33 +294,4 @@ 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,7 +40,6 @@ 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}
@ -74,7 +73,7 @@ public class ScriptEngineManagerImplTest {
when(scriptEngineFactoryMock.getDependencyTracker()).thenReturn(scriptDependencyTrackerMock);
when(scriptDependencyTrackerMock.getTracker(any())).thenReturn(dependencyListenerMock);
scriptEngineManager = new ScriptEngineManagerImpl(scriptExtensionManagerMock, new SafeCallerImpl(null));
scriptEngineManager = new ScriptEngineManagerImpl(scriptExtensionManagerMock);
scriptEngineManager.addScriptEngineFactory(scriptEngineFactoryMock);
}