</execution>
</executions>
</plugin>
+ <plugin>
+ <groupId>org.openhab.tools.sat</groupId>
+ <artifactId>sat-plugin</artifactId>
+ <configuration>
+ <pmdFilter>${project.basedir}/suppressions.properties</pmdFilter>
+ </configuration>
+ </plugin>
</plugins>
</build>
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;
}
@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;
}
}
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;
@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'));";
}
@Override
- public ScriptEngine createScriptEngine(String scriptType) {
+ public @Nullable ScriptEngine createScriptEngine(String scriptType) {
return new DebuggingGraalScriptEngine<>(
new OpenhabGraalJSScriptEngine(injectionEnabled ? INJECTION_CODE : null, jsScriptServiceUtil));
}
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;
private final Map<String, Object> 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());
*/
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;
return scriptExecution;
}
- public JSRuntimeFeatures getJSRuntimeFeatures(Object lock) {
+ public JSRuntimeFeatures getJSRuntimeFeatures(Lock lock) {
return new JSRuntimeFeatures(lock, this);
}
}
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;
* @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<GraalJSScriptEngine> {
}, 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<String> scriptDependencyListener;
+ private @Nullable Consumer<String> scriptDependencyListener;
private boolean initialized = false;
private final String globalScript;
@Override
public SeekableByteChannel newByteChannel(Path path, Set<? extends OpenOption> options,
FileAttribute<?>... attrs) throws IOException {
- if (scriptDependencyListener != null) {
- scriptDependencyListener.accept(path.toString());
+ Consumer<String> localScriptDependencyListener = scriptDependencyListener;
+ if (localScriptDependencyListener != null) {
+ localScriptDependencyListener.accept(path.toString());
}
if (path.toString().endsWith(".js")) {
@Override
protected void beforeInvocation() {
+ lock.lock();
+
if (initialized) {
return;
}
}
@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
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;
* 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
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;
}
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;
* @author Jonathan Gilbert - Initial contribution
*/
public abstract class AbstractScriptExtensionProvider implements ScriptExtensionProvider {
- private Map<String, Function<String, Object>> types;
+ private Map<String, Function<String, Object>> types = new HashMap<>();
private Map<String, Map<String, Object>> idToTypes = new ConcurrentHashMap<>();
protected abstract String getPresetName();
@Activate
public void activate(final BundleContext context) {
- types = new HashMap<>();
+ types.clear();
initializeTypes(context);
}
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;
package org.openhab.automation.jsscripting.internal.scriptengine;
import java.io.Reader;
-import java.util.Objects;
import javax.script.Bindings;
import javax.script.Invocable;
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
*/
public abstract class DelegatingScriptEngineWithInvocableAndAutocloseable<T extends ScriptEngine & Invocable & AutoCloseable>
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
@Override
public void close() throws Exception {
- if (Objects.nonNull(delegate))
- delegate.close();
+ delegate.close();
}
}
package org.openhab.automation.jsscripting.internal.scriptengine;
import java.io.Reader;
+import java.lang.reflect.UndeclaredThrowableException;
import javax.script.Bindings;
import javax.script.Invocable;
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);
}
}
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);
}
}
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);
}
}
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);
}
}
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);
}
}
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
}
}
}
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;
@NonNullByDefault
class ThreadsafeSimpleRuleDelegate implements Rule, SimpleRuleActionHandler {
- private final Object lock;
+ private final Lock lock;
private final SimpleRule delegate;
/**
* @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;
}
@Override
@NonNullByDefault({})
public Object execute(Action module, Map<String, ?> 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();
}
}
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;
* 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
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;
*/
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();
}
});
}
* @return Positive integer value which identifies the timer created; this value can be passed to
* <code>clearTimeout()</code> 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<Object> 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);
* @return Numeric, non-zero value which identifies the timer created; this value can be passed to
* <code>clearInterval()</code> 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<Object> 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);
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;
* 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;
}
--- /dev/null
+# 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
projects / openhab-addons.git / commitdiff