diff --git a/bundles/org.openhab.automation.jsscripting/pom.xml b/bundles/org.openhab.automation.jsscripting/pom.xml index 6247b4058df9d..4ea4af2c93527 100644 --- a/bundles/org.openhab.automation.jsscripting/pom.xml +++ b/bundles/org.openhab.automation.jsscripting/pom.xml @@ -101,6 +101,13 @@ + + org.openhab.tools.sat + sat-plugin + + ${project.basedir}/suppressions.properties + + diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/DebuggingGraalScriptEngine.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/DebuggingGraalScriptEngine.java index 4855f02c2f7fd..3cedaead07ec5 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/DebuggingGraalScriptEngine.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/DebuggingGraalScriptEngine.java @@ -15,7 +15,6 @@ import javax.script.Invocable; import javax.script.ScriptEngine; -import javax.script.ScriptException; import org.graalvm.polyglot.PolyglotException; import org.openhab.automation.jsscripting.internal.scriptengine.InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable; @@ -38,11 +37,11 @@ public DebuggingGraalScriptEngine(T delegate) { } @Override - public ScriptException afterThrowsInvocation(ScriptException se) { - Throwable cause = se.getCause(); + public Exception afterThrowsInvocation(Exception e) { + Throwable cause = e.getCause(); if (cause instanceof PolyglotException) { STACK_LOGGER.error("Failed to execute script:", cause); } - return se; + return e; } } diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineFactory.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineFactory.java index 16d0625f09323..1bdd8a23779ad 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineFactory.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/GraalJSScriptEngineFactory.java @@ -17,6 +17,7 @@ import javax.script.ScriptEngine; +import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.openhab.automation.jsscripting.internal.fs.watch.JSDependencyTracker; import org.openhab.core.automation.module.script.ScriptDependencyTracker; @@ -37,6 +38,7 @@ @Component(service = ScriptEngineFactory.class, configurationPid = "org.openhab.jsscripting", property = Constants.SERVICE_PID + "=org.openhab.jsscripting") @ConfigurableService(category = "automation", label = "JS Scripting", description_uri = "automation:jsscripting") +@NonNullByDefault public final class GraalJSScriptEngineFactory implements ScriptEngineFactory { private static final String CFG_INJECTION_ENABLED = "injectionEnabled"; private static final String INJECTION_CODE = "Object.assign(this, require('openhab'));"; @@ -80,7 +82,7 @@ public void scopeValues(ScriptEngine scriptEngine, Map scopeValu } @Override - public ScriptEngine createScriptEngine(String scriptType) { + public @Nullable ScriptEngine createScriptEngine(String scriptType) { return new DebuggingGraalScriptEngine<>( new OpenhabGraalJSScriptEngine(injectionEnabled ? INJECTION_CODE : null, jsScriptServiceUtil)); } diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/JSRuntimeFeatures.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/JSRuntimeFeatures.java index ca5d30115e78e..62f6339c34a4c 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/JSRuntimeFeatures.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/JSRuntimeFeatures.java @@ -14,6 +14,7 @@ import java.util.HashMap; import java.util.Map; +import java.util.concurrent.locks.Lock; import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.automation.jsscripting.internal.threading.ThreadsafeTimers; @@ -31,7 +32,7 @@ public class JSRuntimeFeatures { private final Map features = new HashMap<>(); public final ThreadsafeTimers threadsafeTimers; - JSRuntimeFeatures(Object lock, JSScriptServiceUtil jsScriptServiceUtil) { + JSRuntimeFeatures(Lock lock, JSScriptServiceUtil jsScriptServiceUtil) { this.threadsafeTimers = new ThreadsafeTimers(lock, jsScriptServiceUtil.getScriptExecution(), jsScriptServiceUtil.getScheduler()); diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/JSScriptServiceUtil.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/JSScriptServiceUtil.java index 8531cacfd80e8..9ecbf21f5e3cb 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/JSScriptServiceUtil.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/JSScriptServiceUtil.java @@ -12,6 +12,8 @@ */ package org.openhab.automation.jsscripting.internal; +import java.util.concurrent.locks.Lock; + import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.core.automation.module.script.action.ScriptExecution; import org.openhab.core.scheduler.Scheduler; @@ -44,7 +46,7 @@ public ScriptExecution getScriptExecution() { return scriptExecution; } - public JSRuntimeFeatures getJSRuntimeFeatures(Object lock) { + public JSRuntimeFeatures getJSRuntimeFeatures(Lock lock) { return new JSRuntimeFeatures(lock, this); } } diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java index 92dfd5b7374b6..bd401d0c2eccc 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java @@ -30,6 +30,8 @@ import java.util.Collections; import java.util.Map; import java.util.Set; +import java.util.concurrent.locks.Lock; +import java.util.concurrent.locks.ReentrantLock; import java.util.function.Consumer; import java.util.function.Function; @@ -58,7 +60,8 @@ * @author Jonathan Gilbert - Initial contribution * @author Dan Cunningham - Script injections * @author Florian Hotze - Create lock object for multi-thread synchronization; Inject the {@link JSRuntimeFeatures} - * into the JS context; Fix memory leak caused by HostObject by making HostAccess reference static + * into the JS context; Fix memory leak caused by HostObject by making HostAccess reference static; Switch to + * {@link Lock} for multi-thread synchronization */ public class OpenhabGraalJSScriptEngine extends InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable { @@ -83,13 +86,13 @@ public class OpenhabGraalJSScriptEngine }, HostAccess.TargetMappingPrecedence.LOW) .build(); - /** Shared lock object for synchronization of multi-thread access */ - private final Object lock = new Object(); + /** {@link Lock} synchronization of multi-thread access */ + private final Lock lock = new ReentrantLock(); private final JSRuntimeFeatures jsRuntimeFeatures; // these fields start as null because they are populated on first use private String engineIdentifier; - private Consumer scriptDependencyListener; + private @Nullable Consumer scriptDependencyListener; private boolean initialized = false; private final String globalScript; @@ -119,8 +122,9 @@ public OpenhabGraalJSScriptEngine(@Nullable String injectionCode, JSScriptServic @Override public SeekableByteChannel newByteChannel(Path path, Set options, FileAttribute... attrs) throws IOException { - if (scriptDependencyListener != null) { - scriptDependencyListener.accept(path.toString()); + Consumer localScriptDependencyListener = scriptDependencyListener; + if (localScriptDependencyListener != null) { + localScriptDependencyListener.accept(path.toString()); } if (path.toString().endsWith(".js")) { @@ -174,6 +178,8 @@ public Path toRealPath(Path path, LinkOption... linkOptions) throws IOException @Override protected void beforeInvocation() { + lock.lock(); + if (initialized) { return; } @@ -227,11 +233,15 @@ protected void beforeInvocation() { } @Override - public Object invokeFunction(String s, Object... objects) throws ScriptException, NoSuchMethodException { - // Synchronize multi-thread access to avoid exceptions when reloading a script file while the script is running - synchronized (lock) { - return super.invokeFunction(s, objects); - } + protected Object afterInvocation(Object obj) { + lock.unlock(); + return obj; + } + + @Override + protected Exception afterThrowsInvocation(Exception e) { + lock.unlock(); + return e; } @Override diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/ScriptExtensionModuleProvider.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/ScriptExtensionModuleProvider.java index 6793fbc6270a0..da131c27fe323 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/ScriptExtensionModuleProvider.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/ScriptExtensionModuleProvider.java @@ -16,6 +16,7 @@ import java.util.HashMap; import java.util.Map; import java.util.Optional; +import java.util.concurrent.locks.Lock; import org.eclipse.jdt.annotation.NonNullByDefault; import org.graalvm.polyglot.Context; @@ -29,7 +30,8 @@ * Class providing script extensions via CommonJS modules. * * @author Jonathan Gilbert - Initial contribution - * @author Florian Hotze - Pass in lock object for multi-thread synchronization + * @author Florian Hotze - Pass in lock object for multi-thread synchronization; Switch to {@link Lock} for multi-thread + * synchronization */ @NonNullByDefault @@ -37,11 +39,11 @@ public class ScriptExtensionModuleProvider { private static final String RUNTIME_MODULE_PREFIX = "@runtime"; private static final String DEFAULT_MODULE_NAME = "Defaults"; - private final Object lock; + private final Lock lock; private final ScriptExtensionAccessor scriptExtensionAccessor; - public ScriptExtensionModuleProvider(ScriptExtensionAccessor scriptExtensionAccessor, Object lock) { + public ScriptExtensionModuleProvider(ScriptExtensionAccessor scriptExtensionAccessor, Lock lock) { this.scriptExtensionAccessor = scriptExtensionAccessor; this.lock = lock; } diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scope/AbstractScriptExtensionProvider.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scope/AbstractScriptExtensionProvider.java index 97520e98cc8c9..537ec6324f995 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scope/AbstractScriptExtensionProvider.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scope/AbstractScriptExtensionProvider.java @@ -13,7 +13,11 @@ package org.openhab.automation.jsscripting.internal.scope; -import java.util.*; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; @@ -28,7 +32,7 @@ * @author Jonathan Gilbert - Initial contribution */ public abstract class AbstractScriptExtensionProvider implements ScriptExtensionProvider { - private Map> types; + private Map> types = new HashMap<>(); private Map> idToTypes = new ConcurrentHashMap<>(); protected abstract String getPresetName(); @@ -41,7 +45,7 @@ protected void addType(String name, Function value) { @Activate public void activate(final BundleContext context) { - types = new HashMap<>(); + types.clear(); initializeTypes(context); } diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scope/ScriptDisposalAwareScriptExtensionProvider.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scope/ScriptDisposalAwareScriptExtensionProvider.java index 380f0214d7d89..d8ae613f46700 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scope/ScriptDisposalAwareScriptExtensionProvider.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scope/ScriptDisposalAwareScriptExtensionProvider.java @@ -13,7 +13,11 @@ package org.openhab.automation.jsscripting.internal.scope; -import java.util.*; +import java.util.Collection; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Objects; import java.util.concurrent.ConcurrentHashMap; import java.util.function.Function; diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/DelegatingScriptEngineWithInvocableAndAutocloseable.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/DelegatingScriptEngineWithInvocableAndAutocloseable.java index f96ce621db4d1..8b55017acb9cf 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/DelegatingScriptEngineWithInvocableAndAutocloseable.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/DelegatingScriptEngineWithInvocableAndAutocloseable.java @@ -14,7 +14,6 @@ package org.openhab.automation.jsscripting.internal.scriptengine; import java.io.Reader; -import java.util.Objects; import javax.script.Bindings; import javax.script.Invocable; @@ -23,7 +22,7 @@ import javax.script.ScriptEngineFactory; import javax.script.ScriptException; -import org.eclipse.jdt.annotation.Nullable; +import org.eclipse.jdt.annotation.NonNull; /** * {@link ScriptEngine} implementation that delegates to a supplied ScriptEngine instance. Allows overriding specific @@ -33,94 +32,91 @@ */ public abstract class DelegatingScriptEngineWithInvocableAndAutocloseable implements ScriptEngine, Invocable, AutoCloseable { - protected T delegate; + protected @NonNull T delegate; - public DelegatingScriptEngineWithInvocableAndAutocloseable(T delegate) { + public DelegatingScriptEngineWithInvocableAndAutocloseable(@NonNull T delegate) { this.delegate = delegate; } @Override - public @Nullable Object eval(String s, ScriptContext scriptContext) throws ScriptException { - return Objects.nonNull(delegate) ? delegate.eval(s, scriptContext) : null; + public Object eval(String s, ScriptContext scriptContext) throws ScriptException { + return delegate.eval(s, scriptContext); } @Override - public @Nullable Object eval(Reader reader, ScriptContext scriptContext) throws ScriptException { - return Objects.nonNull(delegate) ? delegate.eval(reader, scriptContext) : null; + public Object eval(Reader reader, ScriptContext scriptContext) throws ScriptException { + return delegate.eval(reader, scriptContext); } @Override - public @Nullable Object eval(String s) throws ScriptException { - return Objects.nonNull(delegate) ? delegate.eval(s) : null; + public Object eval(String s) throws ScriptException { + return delegate.eval(s); } @Override - public @Nullable Object eval(Reader reader) throws ScriptException { - return Objects.nonNull(delegate) ? delegate.eval(reader) : null; + public Object eval(Reader reader) throws ScriptException { + return delegate.eval(reader); } @Override - public @Nullable Object eval(String s, Bindings bindings) throws ScriptException { - return Objects.nonNull(delegate) ? delegate.eval(s, bindings) : null; + public Object eval(String s, Bindings bindings) throws ScriptException { + return delegate.eval(s, bindings); } @Override - public @Nullable Object eval(Reader reader, Bindings bindings) throws ScriptException { - return Objects.nonNull(delegate) ? delegate.eval(reader, bindings) : null; + public Object eval(Reader reader, Bindings bindings) throws ScriptException { + return delegate.eval(reader, bindings); } @Override public void put(String s, Object o) { - if (Objects.nonNull(delegate)) - delegate.put(s, o); + delegate.put(s, o); } @Override - public @Nullable Object get(String s) { - return Objects.nonNull(delegate) ? delegate.get(s) : null; + public Object get(String s) { + return delegate.get(s); } @Override - public @Nullable Bindings getBindings(int i) { - return Objects.nonNull(delegate) ? delegate.getBindings(i) : null; + public Bindings getBindings(int i) { + return delegate.getBindings(i); } @Override public void setBindings(Bindings bindings, int i) { - if (Objects.nonNull(delegate)) - delegate.setBindings(bindings, i); + delegate.setBindings(bindings, i); } @Override - public @Nullable Bindings createBindings() { - return Objects.nonNull(delegate) ? delegate.createBindings() : null; + public Bindings createBindings() { + return delegate.createBindings(); } @Override - public @Nullable ScriptContext getContext() { - return Objects.nonNull(delegate) ? delegate.getContext() : null; + public ScriptContext getContext() { + return delegate.getContext(); } @Override public void setContext(ScriptContext scriptContext) { - if (Objects.nonNull(delegate)) - delegate.setContext(scriptContext); + delegate.setContext(scriptContext); } @Override - public @Nullable ScriptEngineFactory getFactory() { - return Objects.nonNull(delegate) ? delegate.getFactory() : null; + public ScriptEngineFactory getFactory() { + return delegate.getFactory(); } @Override - public @Nullable Object invokeMethod(Object o, String s, Object... objects) - throws ScriptException, NoSuchMethodException { - return Objects.nonNull(delegate) ? delegate.invokeMethod(o, s, objects) : null; + public Object invokeMethod(Object o, String s, Object... objects) + throws ScriptException, NoSuchMethodException, IllegalArgumentException { + return delegate.invokeMethod(o, s, objects); } @Override - public @Nullable Object invokeFunction(String s, Object... objects) throws ScriptException, NoSuchMethodException { - return Objects.nonNull(delegate) ? delegate.invokeFunction(s, objects) : null; + public Object invokeFunction(String s, Object... objects) throws ScriptException, NoSuchMethodException { + return delegate.invokeFunction(s, objects); } @Override @@ -135,7 +131,6 @@ public T getInterface(Object o, Class aClass) { @Override public void close() throws Exception { - if (Objects.nonNull(delegate)) - delegate.close(); + delegate.close(); } } diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.java index a538bea09a36a..b283192cd39a0 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.java @@ -14,6 +14,7 @@ package org.openhab.automation.jsscripting.internal.scriptengine; import java.io.Reader; +import java.lang.reflect.UndeclaredThrowableException; import javax.script.Bindings; import javax.script.Invocable; @@ -38,17 +39,21 @@ public InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable(T delegat protected void beforeInvocation() { } - protected ScriptException afterThrowsInvocation(ScriptException se) { - return se; + protected Object afterInvocation(Object obj) { + return obj; + } + + protected Exception afterThrowsInvocation(Exception e) { + return e; } @Override public Object eval(String s, ScriptContext scriptContext) throws ScriptException { try { beforeInvocation(); - return super.eval(s, scriptContext); + return afterInvocation(super.eval(s, scriptContext)); } catch (ScriptException se) { - throw afterThrowsInvocation(se); + throw (ScriptException) afterThrowsInvocation(se); } } @@ -56,9 +61,9 @@ public Object eval(String s, ScriptContext scriptContext) throws ScriptException public Object eval(Reader reader, ScriptContext scriptContext) throws ScriptException { try { beforeInvocation(); - return super.eval(reader, scriptContext); + return afterInvocation(super.eval(reader, scriptContext)); } catch (ScriptException se) { - throw afterThrowsInvocation(se); + throw (ScriptException) afterThrowsInvocation(se); } } @@ -66,9 +71,9 @@ public Object eval(Reader reader, ScriptContext scriptContext) throws ScriptExce public Object eval(String s) throws ScriptException { try { beforeInvocation(); - return super.eval(s); + return afterInvocation(super.eval(s)); } catch (ScriptException se) { - throw afterThrowsInvocation(se); + throw (ScriptException) afterThrowsInvocation(se); } } @@ -76,9 +81,9 @@ public Object eval(String s) throws ScriptException { public Object eval(Reader reader) throws ScriptException { try { beforeInvocation(); - return super.eval(reader); + return afterInvocation(super.eval(reader)); } catch (ScriptException se) { - throw afterThrowsInvocation(se); + throw (ScriptException) afterThrowsInvocation(se); } } @@ -86,9 +91,9 @@ public Object eval(Reader reader) throws ScriptException { public Object eval(String s, Bindings bindings) throws ScriptException { try { beforeInvocation(); - return super.eval(s, bindings); + return afterInvocation(super.eval(s, bindings)); } catch (ScriptException se) { - throw afterThrowsInvocation(se); + throw (ScriptException) afterThrowsInvocation(se); } } @@ -96,29 +101,47 @@ public Object eval(String s, Bindings bindings) throws ScriptException { public Object eval(Reader reader, Bindings bindings) throws ScriptException { try { beforeInvocation(); - return super.eval(reader, bindings); + return afterInvocation(super.eval(reader, bindings)); } catch (ScriptException se) { - throw afterThrowsInvocation(se); + throw (ScriptException) afterThrowsInvocation(se); } } @Override - public Object invokeMethod(Object o, String s, Object... objects) throws ScriptException, NoSuchMethodException { + public Object invokeMethod(Object o, String s, Object... objects) + throws ScriptException, NoSuchMethodException, NullPointerException, IllegalArgumentException { try { beforeInvocation(); - return super.invokeMethod(o, s, objects); + return afterInvocation(super.invokeMethod(o, s, objects)); } catch (ScriptException se) { - throw afterThrowsInvocation(se); + throw (ScriptException) afterThrowsInvocation(se); + } catch (NoSuchMethodException e) { // Make sure to unlock on exceptions from Invocable.invokeMethod to avoid + // deadlocks + throw (NoSuchMethodException) afterThrowsInvocation(e); + } catch (NullPointerException e) { + throw (NullPointerException) afterThrowsInvocation(e); + } catch (IllegalArgumentException e) { + throw (IllegalArgumentException) afterThrowsInvocation(e); + } catch (Exception e) { + throw new UndeclaredThrowableException(afterThrowsInvocation(e)); // Wrap and rethrow other exceptions } } @Override - public Object invokeFunction(String s, Object... objects) throws ScriptException, NoSuchMethodException { + public Object invokeFunction(String s, Object... objects) + throws ScriptException, NoSuchMethodException, NullPointerException { try { beforeInvocation(); - return super.invokeFunction(s, objects); + return afterInvocation(super.invokeFunction(s, objects)); } catch (ScriptException se) { - throw afterThrowsInvocation(se); + throw (ScriptException) afterThrowsInvocation(se); + } catch (NoSuchMethodException e) { // Make sure to unlock on exceptions from Invocable.invokeFunction to avoid + // deadlocks + throw (NoSuchMethodException) afterThrowsInvocation(e); + } catch (NullPointerException e) { + throw (NullPointerException) afterThrowsInvocation(e); + } catch (Exception e) { + throw new UndeclaredThrowableException(afterThrowsInvocation(e)); // Wrap and rethrow other exceptions } } } diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeSimpleRuleDelegate.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeSimpleRuleDelegate.java index 3fdd6c57e6247..3006d87e8f249 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeSimpleRuleDelegate.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeSimpleRuleDelegate.java @@ -15,6 +15,7 @@ import java.util.List; import java.util.Map; import java.util.Set; +import java.util.concurrent.locks.Lock; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -38,7 +39,7 @@ @NonNullByDefault class ThreadsafeSimpleRuleDelegate implements Rule, SimpleRuleActionHandler { - private final Object lock; + private final Lock lock; private final SimpleRule delegate; /** @@ -47,7 +48,7 @@ class ThreadsafeSimpleRuleDelegate implements Rule, SimpleRuleActionHandler { * @param lock rule executions will synchronize on this object * @param delegate the delegate to forward invocations to */ - ThreadsafeSimpleRuleDelegate(Object lock, SimpleRule delegate) { + ThreadsafeSimpleRuleDelegate(Lock lock, SimpleRule delegate) { this.lock = lock; this.delegate = delegate; } @@ -55,8 +56,11 @@ class ThreadsafeSimpleRuleDelegate implements Rule, SimpleRuleActionHandler { @Override @NonNullByDefault({}) public Object execute(Action module, Map inputs) { - synchronized (lock) { + lock.lock(); + try { return delegate.execute(module, inputs); + } finally { // Make sure that Lock is unlocked regardless of an exception is thrown or not to avoid deadlocks + lock.unlock(); } } diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeTimers.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeTimers.java index 513dceee40cab..c87bb407768da 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeTimers.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeTimers.java @@ -19,6 +19,7 @@ import java.util.Map; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicLong; +import java.util.concurrent.locks.Lock; import org.eclipse.jdt.annotation.Nullable; import org.openhab.core.automation.module.script.action.ScriptExecution; @@ -35,7 +36,7 @@ * Threadsafe reimplementation of the timer creation methods of {@link ScriptExecution} */ public class ThreadsafeTimers { - private final Object lock; + private final Lock lock; private final Scheduler scheduler; private final ScriptExecution scriptExecution; // Mapping of positive, non-zero integer values (used as timeoutID or intervalID) and the Scheduler @@ -43,7 +44,7 @@ public class ThreadsafeTimers { private AtomicLong lastId = new AtomicLong(); private String identifier = "noIdentifier"; - public ThreadsafeTimers(Object lock, ScriptExecution scriptExecution, Scheduler scheduler) { + public ThreadsafeTimers(Lock lock, ScriptExecution scriptExecution, Scheduler scheduler) { this.lock = lock; this.scheduler = scheduler; this.scriptExecution = scriptExecution; @@ -79,8 +80,12 @@ public Timer createTimer(ZonedDateTime instant, Runnable closure) { */ public Timer createTimer(@Nullable String identifier, ZonedDateTime instant, Runnable closure) { return scriptExecution.createTimer(identifier, instant, () -> { - synchronized (lock) { + lock.lock(); + try { closure.run(); + } finally { // Make sure that Lock is unlocked regardless of an exception is thrown or not to avoid + // deadlocks + lock.unlock(); } }); } @@ -108,12 +113,16 @@ public long setTimeout(Runnable callback, Long delay) { * @return Positive integer value which identifies the timer created; this value can be passed to * clearTimeout() to cancel the timeout. */ - public long setTimeout(Runnable callback, Long delay, Object... args) { + public long setTimeout(Runnable callback, Long delay, @Nullable Object... args) { long id = lastId.incrementAndGet(); ScheduledCompletableFuture future = scheduler.schedule(() -> { - synchronized (lock) { + lock.lock(); + try { callback.run(); idSchedulerMapping.remove(id); + } finally { // Make sure that Lock is unlocked regardless of an exception is thrown or not to avoid + // deadlocks + lock.unlock(); } }, identifier + ".timeout." + id, Instant.now().plusMillis(delay)); idSchedulerMapping.put(id, future); @@ -157,11 +166,15 @@ public long setInterval(Runnable callback, Long delay) { * @return Numeric, non-zero value which identifies the timer created; this value can be passed to * clearInterval() to cancel the interval. */ - public long setInterval(Runnable callback, Long delay, Object... args) { + public long setInterval(Runnable callback, Long delay, @Nullable Object... args) { long id = lastId.incrementAndGet(); ScheduledCompletableFuture future = scheduler.schedule(() -> { - synchronized (lock) { + lock.lock(); + try { callback.run(); + } finally { // Make sure that Lock is unlocked regardless of an exception is thrown or not to avoid + // deadlocks + lock.unlock(); } }, identifier + ".interval." + id, new LoopingAdjuster(Duration.ofMillis(delay))); idSchedulerMapping.put(id, future); diff --git a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeWrappingScriptedAutomationManagerDelegate.java b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeWrappingScriptedAutomationManagerDelegate.java index 6684d065e89a3..fef38d3f8f237 100644 --- a/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeWrappingScriptedAutomationManagerDelegate.java +++ b/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeWrappingScriptedAutomationManagerDelegate.java @@ -13,6 +13,8 @@ package org.openhab.automation.jsscripting.internal.threading; +import java.util.concurrent.locks.Lock; + import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.core.automation.Rule; import org.openhab.core.automation.module.script.rulesupport.shared.ScriptedAutomationManager; @@ -31,15 +33,16 @@ * instance of this class that they are registered with. * * @author Jonathan Gilbert - Initial contribution - * @author Florian Hotze - Pass in lock object for multi-thread synchronization + * @author Florian Hotze - Pass in lock object for multi-thread synchronization; Switch to {@link Lock} for multi-thread + * synchronization */ @NonNullByDefault public class ThreadsafeWrappingScriptedAutomationManagerDelegate { private ScriptedAutomationManager delegate; - private final Object lock; + private final Lock lock; - public ThreadsafeWrappingScriptedAutomationManagerDelegate(ScriptedAutomationManager delegate, Object lock) { + public ThreadsafeWrappingScriptedAutomationManagerDelegate(ScriptedAutomationManager delegate, Lock lock) { this.delegate = delegate; this.lock = lock; } diff --git a/bundles/org.openhab.automation.jsscripting/suppressions.properties b/bundles/org.openhab.automation.jsscripting/suppressions.properties new file mode 100644 index 0000000000000..f57fe0e511d9b --- /dev/null +++ b/bundles/org.openhab.automation.jsscripting/suppressions.properties @@ -0,0 +1,2 @@ +# Please check here how to add suppressions https://maven.apache.org/plugins/maven-pmd-plugin/examples/violation-exclusions.html +org.openhab.automation.jsscripting.internal.scriptengine.InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable=AvoidThrowingNullPointerException,AvoidCatchingNPE