Skip to content

Commit

Permalink
[jsscripting] Extend synchronization to common ScriptEngine methods
Browse files Browse the repository at this point in the history
This extends the multi-thread synchronization to "eval" and "invokeMethod" and moves synchronization for "invokeFunction" to the DelegatingScriptEngineWithInvocableAndAutocloseableAndSynchronization class. Fixes the multi-thread access requested warnings described in the community (https://community.openhab.org/t/openhab-3-4-milestone-discussion/138093/130) and related to openhab/openhab-core#3180.

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>
  • Loading branch information
florian-h05 committed Dec 12, 2022
1 parent 712262f commit aadd21e
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
import javax.script.ScriptException;

import org.graalvm.polyglot.PolyglotException;
import org.openhab.automation.jsscripting.internal.scriptengine.InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable;
import org.openhab.automation.jsscripting.internal.scriptengine.InvocationInterceptingScriptEngineWithInvocableAndAutoCloseableAndSynchronization;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

Expand All @@ -28,7 +28,7 @@
* @author Jonathan Gilbert - Initial contribution
*/
class DebuggingGraalScriptEngine<T extends ScriptEngine & Invocable & AutoCloseable>
extends InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable<T> {
extends InvocationInterceptingScriptEngineWithInvocableAndAutoCloseableAndSynchronization<T> {

private static final Logger STACK_LOGGER = LoggerFactory
.getLogger("org.openhab.automation.script.javascript.stack");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@
import org.openhab.automation.jsscripting.internal.fs.PrefixedSeekableByteChannel;
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.scriptengine.InvocationInterceptingScriptEngineWithInvocableAndAutoCloseableAndSynchronization;
import org.openhab.core.automation.module.script.ScriptExtensionAccessor;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
Expand All @@ -61,7 +61,7 @@
* into the JS context; Fix memory leak caused by HostObject by making HostAccess reference static
*/
public class OpenhabGraalJSScriptEngine
extends InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable<GraalJSScriptEngine> {
extends InvocationInterceptingScriptEngineWithInvocableAndAutoCloseableAndSynchronization<GraalJSScriptEngine> {

private static final Logger LOGGER = LoggerFactory.getLogger(OpenhabGraalJSScriptEngine.class);
private static final String GLOBAL_REQUIRE = "require(\"@jsscripting-globals\");";
Expand All @@ -83,8 +83,6 @@ public class OpenhabGraalJSScriptEngine
}, HostAccess.TargetMappingPrecedence.LOW)
.build();

/** Shared lock object for synchronization of multi-thread access */
private final Object lock = new Object();
private final JSRuntimeFeatures jsRuntimeFeatures;

// these fields start as null because they are populated on first use
Expand Down Expand Up @@ -226,14 +224,6 @@ 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);
}
}

@Override
public void close() {
jsRuntimeFeatures.close();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,47 +26,85 @@
import org.eclipse.jdt.annotation.Nullable;

/**
* {@link ScriptEngine} implementation that delegates to a supplied ScriptEngine instance. Allows overriding specific
* methods.
* {@link ScriptEngine} implementation that delegates to a supplied ScriptEngine instance and provides multi-thread
* synchronization. Allows overriding specific methods.
*
* @author Jonathan Gilbert - Initial contribution
*/
public abstract class DelegatingScriptEngineWithInvocableAndAutocloseable<T extends ScriptEngine & Invocable & AutoCloseable>
public abstract class DelegatingScriptEngineWithInvocableAndAutocloseableAndSynchronization<T extends ScriptEngine & Invocable & AutoCloseable>
implements ScriptEngine, Invocable, AutoCloseable {
protected T delegate;
/** Shared lock object for synchronization of multi-thread access */
protected final Object lock = new Object();

public DelegatingScriptEngineWithInvocableAndAutocloseable(T delegate) {
public DelegatingScriptEngineWithInvocableAndAutocloseableAndSynchronization(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;
if (Objects.nonNull(delegate)) {
synchronized (lock) {
return delegate.eval(s, scriptContext);
}
} else {
return null;
}
}

@Override
public @Nullable Object eval(Reader reader, ScriptContext scriptContext) throws ScriptException {
return Objects.nonNull(delegate) ? delegate.eval(reader, scriptContext) : null;
if (Objects.nonNull(delegate)) {
synchronized (lock) {
return delegate.eval(reader, scriptContext);
}
} else {
return null;
}
}

@Override
public @Nullable Object eval(String s) throws ScriptException {
return Objects.nonNull(delegate) ? delegate.eval(s) : null;
if (Objects.nonNull(delegate)) {
synchronized (lock) {
return delegate.eval(s);
}
} else {
return null;
}
}

@Override
public @Nullable Object eval(Reader reader) throws ScriptException {
return Objects.nonNull(delegate) ? delegate.eval(reader) : null;
if (Objects.nonNull(delegate)) {
synchronized (lock) {
return delegate.eval(reader);
}
} else {
return null;
}
}

@Override
public @Nullable Object eval(String s, Bindings bindings) throws ScriptException {
return Objects.nonNull(delegate) ? delegate.eval(s, bindings) : null;
if (Objects.nonNull(delegate)) {
synchronized (lock) {
return delegate.eval(s, bindings);
}
} else {
return null;
}
}

@Override
public @Nullable Object eval(Reader reader, Bindings bindings) throws ScriptException {
return Objects.nonNull(delegate) ? delegate.eval(reader, bindings) : null;
if (Objects.nonNull(delegate)) {
synchronized (lock) {
return delegate.eval(reader, bindings);
}
} else {
return null;
}
}

@Override
Expand Down Expand Up @@ -115,12 +153,24 @@ public void setContext(ScriptContext scriptContext) {
@Override
public @Nullable Object invokeMethod(Object o, String s, Object... objects)
throws ScriptException, NoSuchMethodException {
return Objects.nonNull(delegate) ? delegate.invokeMethod(o, s, objects) : null;
if (Objects.nonNull(delegate)) {
synchronized (lock) {
return delegate.invokeMethod(o, s, objects);
}
} else {
return null;
}
}

@Override
public @Nullable Object invokeFunction(String s, Object... objects) throws ScriptException, NoSuchMethodException {
return Objects.nonNull(delegate) ? delegate.invokeFunction(s, objects) : null;
if (Objects.nonNull(delegate)) {
synchronized (lock) {
return delegate.invokeFunction(s, objects);
}
} else {
return null;
}
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@
* @param <T> The delegate class
* @author Jonathan Gilbert - Initial contribution
*/
public abstract class InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable<T extends ScriptEngine & Invocable & AutoCloseable>
extends DelegatingScriptEngineWithInvocableAndAutocloseable<T> {
public abstract class InvocationInterceptingScriptEngineWithInvocableAndAutoCloseableAndSynchronization<T extends ScriptEngine & Invocable & AutoCloseable>
extends DelegatingScriptEngineWithInvocableAndAutocloseableAndSynchronization<T> {

public InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable(T delegate) {
public InvocationInterceptingScriptEngineWithInvocableAndAutoCloseableAndSynchronization(T delegate) {
super(delegate);
}

Expand Down

0 comments on commit aadd21e

Please sign in to comment.