[jsscripting] Fix multi-thread access (#13582)

* [jsscripting] Share the lock mechanism that was used only for rules
This change moves the lock object that was originally created for ThreadsafeSimpleRuleDelegate to OpenhabGraalJSScriptEngine to make share it across the whole engine.

* [jsscripting] Inject the lock object into the JS runtime
* [jsscripting] Update `setTimeout` & `setInterval` polyfills to enable threadsafety
* [jsscripting] Upgrade GraalJS from 21.3.0 to 22.3.0
* [jsscripting] Reduce compiler warnings
* [jsscripting] Update node version of frontend-maven-plugin

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
This commit is contained in:
Florian Hotze 2022-10-24 13:35:33 +02:00 committed by GitHub
parent 671d2de9b2
commit cf47dc0f39
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 222 additions and 65 deletions

View File

@ -22,7 +22,7 @@
!jdk.internal.reflect.*,
!jdk.vm.ci.services
</bnd.importpackage>
<graal.version>21.3.0</graal.version>
<graal.version>22.3.0</graal.version>
<asm.version>6.2.1</asm.version>
<oh.version>${project.version}</oh.version>
<ohjs.version>openhab@2.0.4</ohjs.version>
@ -50,7 +50,7 @@
<artifactId>frontend-maven-plugin</artifactId>
<version>1.12.0</version>
<configuration>
<nodeVersion>v12.16.1</nodeVersion>
<nodeVersion>v16.17.1</nodeVersion>
<workingDirectory>target/js</workingDirectory>
</configuration>
<executions>

View File

@ -65,7 +65,7 @@ public final class GraalJSScriptEngineFactory implements ScriptEngineFactory {
@Override
public void scopeValues(ScriptEngine scriptEngine, Map<String, Object> scopeValues) {
// noop; the are retrieved via modules, not injected
// noop; they are retrieved via modules, not injected
}
@Override

View File

@ -36,7 +36,6 @@ import java.util.function.Function;
import javax.script.ScriptContext;
import javax.script.ScriptException;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.graalvm.polyglot.Context;
import org.graalvm.polyglot.Engine;
@ -47,6 +46,7 @@ import org.openhab.automation.jsscripting.internal.fs.PrefixedSeekableByteChanne
import org.openhab.automation.jsscripting.internal.fs.ReadOnlySeekableByteArrayChannel;
import org.openhab.automation.jsscripting.internal.fs.watch.JSDependencyTracker;
import org.openhab.automation.jsscripting.internal.scriptengine.InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable;
import org.openhab.automation.jsscripting.internal.threading.ThreadsafeTimers;
import org.openhab.core.automation.module.script.ScriptExtensionAccessor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -58,6 +58,7 @@ import com.oracle.truffle.js.scriptengine.GraalJSScriptEngine;
*
* @author Jonathan Gilbert - Initial contribution
* @author Dan Cunningham - Script injections
* @author Florian Hotze - Create lock object for multi-thread synchronization
*/
public class OpenhabGraalJSScriptEngine
extends InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable<GraalJSScriptEngine> {
@ -68,9 +69,12 @@ public class OpenhabGraalJSScriptEngine
// final CommonJS search path for our library
private static final Path NODE_DIR = Paths.get("node_modules");
// shared lock object for synchronization of multi-thread access
private final Object lock = new Object();
// these fields start as null because they are populated on first use
private @NonNullByDefault({}) String engineIdentifier;
private @NonNullByDefault({}) Consumer<String> scriptDependencyListener;
private String engineIdentifier;
private Consumer<String> scriptDependencyListener;
private boolean initialized = false;
private String globalScript;
@ -83,6 +87,8 @@ public class OpenhabGraalJSScriptEngine
super(null); // delegate depends on fields not yet initialised, so we cannot set it immediately
this.globalScript = GLOBAL_REQUIRE + (injectionCode != null ? injectionCode : "");
LOGGER.debug("Initializing GraalJS script engine...");
// Custom translate JS Objects - > Java Objects
HostAccess hostAccess = HostAccess.newBuilder(HostAccess.ALL)
// Translate JS-Joda ZonedDateTime to java.time.ZonedDateTime
@ -194,14 +200,16 @@ public class OpenhabGraalJSScriptEngine
}
ScriptExtensionModuleProvider scriptExtensionModuleProvider = new ScriptExtensionModuleProvider(
scriptExtensionAccessor);
scriptExtensionAccessor, lock);
Function<Function<Object[], Object>, Function<String, Object>> wrapRequireFn = originalRequireFn -> moduleName -> scriptExtensionModuleProvider
.locatorFor(delegate.getPolyglotContext(), engineIdentifier).locateModule(moduleName)
.map(m -> (Object) m).orElseGet(() -> originalRequireFn.apply(new Object[] { moduleName }));
delegate.getBindings(ScriptContext.ENGINE_SCOPE).put(REQUIRE_WRAPPER_NAME, wrapRequireFn);
// Injections into the JS runtime
delegate.put("require", wrapRequireFn.apply((Function<Object[], Object>) delegate.get("require")));
delegate.put("ThreadsafeTimers", new ThreadsafeTimers(lock));
initialized = true;
@ -215,8 +223,8 @@ public class OpenhabGraalJSScriptEngine
/**
* Tests if this is a root node directory, `/node_modules`, `C:\node_modules`, etc...
*
* @param path
* @return
* @param path a root path
* @return whether the given path is a node root directory
*/
private boolean isRootNodePath(Path path) {
return path.startsWith(path.getRoot().resolve(NODE_DIR));
@ -226,8 +234,8 @@ public class OpenhabGraalJSScriptEngine
* Converts a root node path to a class resource path for loading local modules
* Ex: C:\node_modules\foo.js -> /node_modules/foo.js
*
* @param path
* @return
* @param path a root path, e.g. C:\node_modules\foo.js
* @return the class resource path for loading local modules
*/
private String nodeFileToResource(Path path) {
return "/" + path.subpath(0, path.getNameCount()).toString().replace('\\', '/');

View File

@ -29,6 +29,7 @@ import org.openhab.core.automation.module.script.rulesupport.shared.ScriptedAuto
* Class providing script extensions via CommonJS modules.
*
* @author Jonathan Gilbert - Initial contribution
* @author Florian Hotze - Pass in lock object for multi-thread synchronization
*/
@NonNullByDefault
@ -36,11 +37,13 @@ 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 ScriptExtensionAccessor scriptExtensionAccessor;
public ScriptExtensionModuleProvider(ScriptExtensionAccessor scriptExtensionAccessor) {
public ScriptExtensionModuleProvider(ScriptExtensionAccessor scriptExtensionAccessor, Object lock) {
this.scriptExtensionAccessor = scriptExtensionAccessor;
this.lock = lock;
}
public ModuleLocator locatorFor(Context ctx, String engineIdentifier) {
@ -94,7 +97,7 @@ public class ScriptExtensionModuleProvider {
for (Map.Entry<String, Object> entry : rv.entrySet()) {
if (entry.getValue() instanceof ScriptedAutomationManager) {
entry.setValue(new ThreadsafeWrappingScriptedAutomationManagerDelegate(
(ScriptedAutomationManager) entry.getValue()));
(ScriptedAutomationManager) entry.getValue(), lock));
}
}

View File

@ -15,6 +15,7 @@ package org.openhab.automation.jsscripting.internal.fs.watch;
import java.io.File;
import java.nio.file.Path;
import java.nio.file.WatchEvent;
import java.util.Objects;
import java.util.Optional;
import org.eclipse.jdt.annotation.Nullable;
@ -31,7 +32,6 @@ import org.openhab.core.service.ReadyService;
*/
public class JSScriptFileWatcher extends ScriptFileWatcher {
private static final String FILE_DIRECTORY = "automation" + File.separator + "js";
private static final String IGNORE_DIR_NAME = "node_modules";
private final String ignorePath;
@ -45,8 +45,10 @@ public class JSScriptFileWatcher extends ScriptFileWatcher {
@Override
protected void processWatchEvent(@Nullable WatchEvent<?> event, WatchEvent.@Nullable Kind<?> kind,
@Nullable Path path) {
if (!path.startsWith(ignorePath)) {
super.processWatchEvent(event, kind, path);
if (Objects.nonNull(path)) {
if (!path.startsWith(ignorePath)) {
super.processWatchEvent(event, kind, path);
}
}
}

View File

@ -13,13 +13,11 @@
package org.openhab.automation.jsscripting.internal.scope;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.automation.module.script.ScriptExtensionProvider;
import org.osgi.framework.BundleContext;
import org.osgi.service.component.annotations.Activate;
@ -63,10 +61,11 @@ public abstract class AbstractScriptExtensionProvider implements ScriptExtension
}
@Override
public Object get(String scriptIdentifier, String type) throws IllegalArgumentException {
public @Nullable Object get(String scriptIdentifier, String type) throws IllegalArgumentException {
Map<String, Object> forScript = idToTypes.computeIfAbsent(scriptIdentifier, k -> new HashMap<>());
return forScript.computeIfAbsent(type, k -> types.get(k).apply(scriptIdentifier));
return forScript.computeIfAbsent(type,
k -> Objects.nonNull(types.get(k)) ? types.get(k).apply(scriptIdentifier) : null);
}
@Override

View File

@ -13,13 +13,11 @@
package org.openhab.automation.jsscripting.internal.scope;
import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.*;
import java.util.concurrent.ConcurrentHashMap;
import java.util.function.Function;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.automation.module.script.ScriptExtensionProvider;
import org.osgi.framework.BundleContext;
import org.osgi.service.component.annotations.Activate;
@ -64,10 +62,11 @@ public abstract class ScriptDisposalAwareScriptExtensionProvider
}
@Override
public Object get(String scriptIdentifier, String type) throws IllegalArgumentException {
public @Nullable Object get(String scriptIdentifier, String type) throws IllegalArgumentException {
Map<String, Object> forScript = idToTypes.computeIfAbsent(scriptIdentifier, k -> new HashMap<>());
return forScript.computeIfAbsent(type, k -> types.get(k).apply(scriptIdentifier));
return forScript.computeIfAbsent(type,
k -> Objects.nonNull(types.get(k)) ? types.get(k).apply(scriptIdentifier) : null);
}
@Override

View File

@ -14,6 +14,7 @@
package org.openhab.automation.jsscripting.internal.scriptengine;
import java.io.Reader;
import java.util.Objects;
import javax.script.Bindings;
import javax.script.Invocable;
@ -22,6 +23,8 @@ import javax.script.ScriptEngine;
import javax.script.ScriptEngineFactory;
import javax.script.ScriptException;
import org.eclipse.jdt.annotation.Nullable;
/**
* {@link ScriptEngine} implementation that delegates to a supplied ScriptEngine instance. Allows overriding specific
* methods.
@ -37,83 +40,87 @@ public abstract class DelegatingScriptEngineWithInvocableAndAutocloseable<T exte
}
@Override
public Object eval(String s, ScriptContext scriptContext) throws ScriptException {
return delegate.eval(s, scriptContext);
public @Nullable Object eval(String s, ScriptContext scriptContext) throws ScriptException {
return Objects.nonNull(delegate) ? delegate.eval(s, scriptContext) : null;
}
@Override
public Object eval(Reader reader, ScriptContext scriptContext) throws ScriptException {
return delegate.eval(reader, scriptContext);
public @Nullable Object eval(Reader reader, ScriptContext scriptContext) throws ScriptException {
return Objects.nonNull(delegate) ? delegate.eval(reader, scriptContext) : null;
}
@Override
public Object eval(String s) throws ScriptException {
return delegate.eval(s);
public @Nullable Object eval(String s) throws ScriptException {
return Objects.nonNull(delegate) ? delegate.eval(s) : null;
}
@Override
public Object eval(Reader reader) throws ScriptException {
return delegate.eval(reader);
public @Nullable Object eval(Reader reader) throws ScriptException {
return Objects.nonNull(delegate) ? delegate.eval(reader) : null;
}
@Override
public Object eval(String s, Bindings bindings) throws ScriptException {
return delegate.eval(s, bindings);
public @Nullable Object eval(String s, Bindings bindings) throws ScriptException {
return Objects.nonNull(delegate) ? delegate.eval(s, bindings) : null;
}
@Override
public Object eval(Reader reader, Bindings bindings) throws ScriptException {
return delegate.eval(reader, bindings);
public @Nullable Object eval(Reader reader, Bindings bindings) throws ScriptException {
return Objects.nonNull(delegate) ? delegate.eval(reader, bindings) : null;
}
@Override
public void put(String s, Object o) {
delegate.put(s, o);
if (Objects.nonNull(delegate))
delegate.put(s, o);
}
@Override
public Object get(String s) {
return delegate.get(s);
public @Nullable Object get(String s) {
return Objects.nonNull(delegate) ? delegate.get(s) : null;
}
@Override
public Bindings getBindings(int i) {
return delegate.getBindings(i);
public @Nullable Bindings getBindings(int i) {
return Objects.nonNull(delegate) ? delegate.getBindings(i) : null;
}
@Override
public void setBindings(Bindings bindings, int i) {
delegate.setBindings(bindings, i);
if (Objects.nonNull(delegate))
delegate.setBindings(bindings, i);
}
@Override
public Bindings createBindings() {
return delegate.createBindings();
public @Nullable Bindings createBindings() {
return Objects.nonNull(delegate) ? delegate.createBindings() : null;
}
@Override
public ScriptContext getContext() {
return delegate.getContext();
public @Nullable ScriptContext getContext() {
return Objects.nonNull(delegate) ? delegate.getContext() : null;
}
@Override
public void setContext(ScriptContext scriptContext) {
delegate.setContext(scriptContext);
if (Objects.nonNull(delegate))
delegate.setContext(scriptContext);
}
@Override
public ScriptEngineFactory getFactory() {
return delegate.getFactory();
public @Nullable ScriptEngineFactory getFactory() {
return Objects.nonNull(delegate) ? delegate.getFactory() : null;
}
@Override
public Object invokeMethod(Object o, String s, Object... objects) throws ScriptException, NoSuchMethodException {
return delegate.invokeMethod(o, s, objects);
public @Nullable Object invokeMethod(Object o, String s, Object... objects)
throws ScriptException, NoSuchMethodException {
return Objects.nonNull(delegate) ? delegate.invokeMethod(o, s, objects) : null;
}
@Override
public Object invokeFunction(String s, Object... objects) throws ScriptException, NoSuchMethodException {
return delegate.invokeFunction(s, objects);
public @Nullable Object invokeFunction(String s, Object... objects) throws ScriptException, NoSuchMethodException {
return Objects.nonNull(delegate) ? delegate.invokeFunction(s, objects) : null;
}
@Override
@ -128,6 +135,7 @@ public abstract class DelegatingScriptEngineWithInvocableAndAutocloseable<T exte
@Override
public void close() throws Exception {
delegate.close();
if (Objects.nonNull(delegate))
delegate.close();
}
}

View File

@ -30,7 +30,7 @@ import org.openhab.core.config.core.ConfigDescriptionParameter;
import org.openhab.core.config.core.Configuration;
/**
* An version of {@link SimpleRule} which controls multithreaded execution access to this specific rule. This is useful
* A version of {@link SimpleRule} which controls multithreaded execution access to this specific rule. This is useful
* for rules which wrap GraalJS Contexts, which are not multithreaded.
*
* @author Jonathan Gilbert - Initial contribution

View File

@ -0,0 +1,137 @@
/**
* Copyright (c) 2010-2022 Contributors to the openHAB project
*
* See the NOTICE file(s) distributed with this work for additional
* information.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0
*
* SPDX-License-Identifier: EPL-2.0
*/
package org.openhab.automation.jsscripting.internal.threading;
import java.time.ZonedDateTime;
import java.util.concurrent.TimeUnit;
import org.eclipse.jdt.annotation.NonNullByDefault;
import org.eclipse.jdt.annotation.Nullable;
import org.openhab.core.model.script.ScriptServiceUtil;
import org.openhab.core.model.script.actions.Timer;
import org.openhab.core.scheduler.ScheduledCompletableFuture;
import org.openhab.core.scheduler.Scheduler;
import org.openhab.core.scheduler.SchedulerRunnable;
/**
* A replacement for the timer functionality of {@link org.openhab.core.model.script.actions.ScriptExecution
* ScriptExecution} which controls multithreaded execution access to the single-threaded GraalJS contexts.
*
* @author Florian Hotze - Initial contribution
*/
public class ThreadsafeTimers {
private final Object lock;
public ThreadsafeTimers(Object lock) {
this.lock = lock;
}
public Timer createTimer(ZonedDateTime instant, Runnable callable) {
return createTimer(null, instant, callable);
}
public Timer createTimer(@Nullable String identifier, ZonedDateTime instant, Runnable callable) {
Scheduler scheduler = ScriptServiceUtil.getScheduler();
return new TimerImpl(scheduler, instant, () -> {
synchronized (lock) {
callable.run();
}
}, identifier);
}
public Timer createTimerWithArgument(ZonedDateTime instant, Object arg1, Runnable callable) {
return createTimerWithArgument(null, instant, arg1, callable);
}
public Timer createTimerWithArgument(@Nullable String identifier, ZonedDateTime instant, Object arg1,
Runnable callable) {
Scheduler scheduler = ScriptServiceUtil.getScheduler();
return new TimerImpl(scheduler, instant, () -> {
synchronized (lock) {
callable.run();
}
}, identifier);
}
/**
* This is an implementation of the {@link Timer} interface.
* Copy of {@link org.openhab.core.model.script.internal.actions.TimerImpl} as this is not accessible from outside
* the
* package.
*
* @author Kai Kreuzer - Initial contribution
*/
@NonNullByDefault
public static class TimerImpl implements Timer {
private final Scheduler scheduler;
private final ZonedDateTime startTime;
private final SchedulerRunnable runnable;
private final @Nullable String identifier;
private ScheduledCompletableFuture<?> future;
public TimerImpl(Scheduler scheduler, ZonedDateTime startTime, SchedulerRunnable runnable) {
this(scheduler, startTime, runnable, null);
}
public TimerImpl(Scheduler scheduler, ZonedDateTime startTime, SchedulerRunnable runnable,
@Nullable String identifier) {
this.scheduler = scheduler;
this.startTime = startTime;
this.runnable = runnable;
this.identifier = identifier;
future = scheduler.schedule(runnable, identifier, startTime.toInstant());
}
@Override
public boolean cancel() {
return future.cancel(true);
}
@Override
public synchronized boolean reschedule(ZonedDateTime newTime) {
future.cancel(false);
future = scheduler.schedule(runnable, identifier, newTime.toInstant());
return true;
}
@Override
public @Nullable ZonedDateTime getExecutionTime() {
return future.isCancelled() ? null : ZonedDateTime.now().plusNanos(future.getDelay(TimeUnit.NANOSECONDS));
}
@Override
public boolean isActive() {
return !future.isDone();
}
@Override
public boolean isCancelled() {
return future.isCancelled();
}
@Override
public boolean isRunning() {
return isActive() && ZonedDateTime.now().isAfter(startTime);
}
@Override
public boolean hasTerminated() {
return future.isDone();
}
}
}

View File

@ -31,15 +31,17 @@ import org.openhab.core.automation.type.TriggerType;
* 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
*/
@NonNullByDefault
public class ThreadsafeWrappingScriptedAutomationManagerDelegate {
private ScriptedAutomationManager delegate;
private Object lock = new Object();
private final Object lock;
public ThreadsafeWrappingScriptedAutomationManagerDelegate(ScriptedAutomationManager delegate) {
public ThreadsafeWrappingScriptedAutomationManagerDelegate(ScriptedAutomationManager delegate, Object lock) {
this.delegate = delegate;
this.lock = lock;
}
public void removeModuleType(String UID) {

View File

@ -2,10 +2,9 @@
(function (global) {
'use strict';
//Append the script file name OR rule UID depending on which is available
// Append the script file name OR rule UID depending on which is available
const defaultLoggerName = "org.openhab.automation.script" + (globalThis["javax.script.filename"] ? ".file." + globalThis["javax.script.filename"].replace(/^.*[\\\/]/, '') : globalThis["ruleUID"] ? ".ui." + globalThis["ruleUID"] : "");
const System = Java.type('java.lang.System');
const ScriptExecution = Java.type('org.openhab.core.model.script.actions.ScriptExecution');
const ZonedDateTime = Java.type('java.time.ZonedDateTime');
const formatRegExp = /%[sdj%]/g;
@ -173,7 +172,7 @@
function setTimeout(cb, delay) {
const args = Array.prototype.slice.call(arguments, 2);
return ScriptExecution.createTimerWithArgument(
return ThreadsafeTimers.createTimerWithArgument(
ZonedDateTime.now().plusNanos(delay * 1000000),
args,
function (args) {
@ -191,7 +190,7 @@
function setInterval(cb, delay) {
const args = Array.prototype.slice.call(arguments, 2);
const delayNanos = delay * 1000000
let timer = ScriptExecution.createTimerWithArgument(
let timer = ThreadsafeTimers.createTimerWithArgument(
ZonedDateTime.now().plusNanos(delayNanos),
args,
function (args) {