diff --git a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java index 48d08a249e4..fc11724d4db 100644 --- a/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java +++ b/bundles/org.openhab.core.automation.module.script/src/main/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImpl.java @@ -36,7 +36,6 @@ 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 @@ @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 loadedScriptEngineInstances = new HashMap<>(); @@ -72,10 +66,8 @@ public class ScriptEngineManagerImpl implements ScriptEngineManager { private final Set 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 void loadScript(String engineIdentifier, InputStreamReader scriptData) { } 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 void removeEngine(String engineIdentifier) { 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 void addFactoryChangeListener(FactoryChangeListener listener) { 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); - } } diff --git a/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java b/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java index 00215d212ec..01d876d1a40 100644 --- a/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java +++ b/bundles/org.openhab.core.automation.module.script/src/test/java/org/openhab/core/automation/module/script/internal/ScriptEngineManagerImplTest.java @@ -40,7 +40,6 @@ 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 void setup() { 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); }