[jsscripting] Extend synchronization to common ScriptEngine methods

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>

bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/DebuggingGraalScriptEngine.java

@@ -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;
 
@@ -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");

bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java

@@ -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;
@@ -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\");";
@@ -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
@@ -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();

bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/DelegatingScriptEngineWithInvocableAndAutocloseable.java → bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/DelegatingScriptEngineWithInvocableAndAutocloseableAndSynchronization.java

@@ -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
@@ -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

bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.java → bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/scriptengine/InvocationInterceptingScriptEngineWithInvocableAndAutoCloseableAndSynchronization.java

@@ -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);
  }