[jsscripting] Reimplement timer polyfills to conform standard JS (#13623)

* [jsscripting] Reimplement timers to conform standard JS
* [jsscripting] Name scheduled jobs by loggerName + id
* [jsscripting] Update timer identifiers
* [jsscripting] Update identifiers for scheduled jobs
* [jsscripting] Synchronize method that is called when the script is reloaded
* [jsscripting] Cancel all scheduled jobs when the engine is closed
* [jsscripting] Ensure that a timerId is never reused by a subsequent call & Use long primitive type instead of Integer
* [jsscripting] Use an abstraction class to inject features into the JS runtime
* [jsscripting] Make ThreadsafeTimers threadsafe for concurrent access to the class itself
* [jsscripting] Move the locking for `invokeFunction` to `OpenhabGraalJSScriptEngine`

Signed-off-by: Florian Hotze <florianh_dev@icloud.com>

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

@@ -0,0 +1,56 @@
+/**
+ * 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;
+
+import java.util.HashMap;
+import java.util.Map;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.openhab.automation.jsscripting.internal.threading.ThreadsafeTimers;
+
+/**
+ * Abstraction layer to collect all features injected into the JS runtime during the context creation.
+ *
+ * @author Florian Hotze - Initial contribution
+ */
+@NonNullByDefault
+public class JSRuntimeFeatures {
+    /**
+     * All elements of this Map are injected into the JS runtime using their key as the name.
+     */
+    private final Map<String, Object> features = new HashMap<>();
+    public final ThreadsafeTimers threadsafeTimers;
+
+    JSRuntimeFeatures(Object lock) {
+        this.threadsafeTimers = new ThreadsafeTimers(lock);
+
+        features.put("ThreadsafeTimers", threadsafeTimers);
+    }
+
+    /**
+     * Get the features that are to be injected into the JS runtime during context creation.
+     * 
+     * @return the runtime features
+     */
+    public Map<String, Object> getFeatures() {
+        return features;
+    }
+
+    /**
+     * Un-initialization hook, called when the engine is closed.
+     * Use this method to clean up resources or cancel operations that were created by the JS runtime.
+     */
+    public void close() {
+        threadsafeTimers.clearAll();
+    }
+}

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

@@ -46,7 +46,6 @@ 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,7 +57,8 @@ 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
+ * @author Florian Hotze - Create lock object for multi-thread synchronization; Inject the {@link JSRuntimeFeatures}
+ *         into the JS context
  */
 public class OpenhabGraalJSScriptEngine
         extends InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable<GraalJSScriptEngine> {
@@ -71,6 +71,7 @@ public class OpenhabGraalJSScriptEngine
 
     // shared lock object for synchronization of multi-thread access
     private final Object lock = new Object();
+    private final JSRuntimeFeatures jsRuntimeFeatures = new JSRuntimeFeatures(lock);
 
     // these fields start as null because they are populated on first use
     private String engineIdentifier;
@@ -209,7 +210,7 @@ public class OpenhabGraalJSScriptEngine
         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));
+        jsRuntimeFeatures.getFeatures().forEach((key, obj) -> delegate.put(key, obj));
 
         initialized = true;
 
@@ -220,6 +221,19 @@ public class OpenhabGraalJSScriptEngine
         }
     }
 
+    @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();
+    }
+
     /**
      * Tests if this is a root node directory, `/node_modules`, `C:\node_modules`, etc...
      *

bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/threading/ThreadsafeTimers.java

@@ -12,126 +12,209 @@
  */
 package org.openhab.automation.jsscripting.internal.threading;
 
+import java.time.Duration;
 import java.time.ZonedDateTime;
-import java.util.concurrent.TimeUnit;
+import java.time.temporal.Temporal;
+import java.util.Map;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicLong;
 
-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;
+import org.openhab.core.scheduler.SchedulerTemporalAdjuster;
 
 /**
- * A replacement for the timer functionality of {@link org.openhab.core.model.script.actions.ScriptExecution
+ * A polyfill implementation of NodeJS timer functionality (<code>setTimeout()</code>, <code>setInterval()</code> and
- * ScriptExecution} which controls multithreaded execution access to the single-threaded GraalJS contexts.
+ * the cancel methods) which controls multithreaded execution access to the single-threaded GraalJS contexts.
  *
  * @author Florian Hotze - Initial contribution
+ * @author Florian Hotze - Reimplementation to conform standard JS setTimeout and setInterval
  */
 public class ThreadsafeTimers {
     private final Object lock;
+    private final Scheduler scheduler;
+    // Mapping of positive, non-zero integer values (used as timeoutID or intervalID) and the Scheduler
+    private final Map<Long, ScheduledCompletableFuture<Object>> idSchedulerMapping = new ConcurrentHashMap<>();
+    private AtomicLong lastId = new AtomicLong();
+    private String identifier = "noIdentifier";
 
     public ThreadsafeTimers(Object lock) {
         this.lock = lock;
-    }
+        this.scheduler = ScriptServiceUtil.getScheduler();
-
-    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.
+     * Set the identifier base string used for naming scheduled jobs.
-     * 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
+     * @param identifier identifier to use
      */
-    @NonNullByDefault
+    public void setIdentifier(String identifier) {
-    public static class TimerImpl implements Timer {
+        this.identifier = identifier;
+    }
 
-        private final Scheduler scheduler;
+    /**
-        private final ZonedDateTime startTime;
+     * Schedules a callback to run at a given time.
-        private final SchedulerRunnable runnable;
+     *
-        private final @Nullable String identifier;
+     * @param id timerId to append to the identifier base for naming the scheduled job
-        private ScheduledCompletableFuture<?> future;
+     * @param zdt time to schedule the job
+     * @param callback function to run at the given time
+     * @return a {@link ScheduledCompletableFuture}
+     */
+    private ScheduledCompletableFuture<Object> createFuture(long id, ZonedDateTime zdt, Runnable callback) {
+        return scheduler.schedule(() -> {
+            synchronized (lock) {
+                callback.run();
+            }
+        }, identifier + ".timeout." + id, zdt.toInstant());
+    }
 
-        public TimerImpl(Scheduler scheduler, ZonedDateTime startTime, SchedulerRunnable runnable) {
+    /**
-            this(scheduler, startTime, runnable, null);
+     * <a href="https://developer.mozilla.org/en-US/docs/Web/API/setTimeout"><code>setTimeout()</code></a> polyfill.
+     * Sets a timer which executes a given function once the timer expires.
+     *
+     * @param callback function to run after the given delay
+     * @param delay time in milliseconds that the timer should wait before the callback is executed
+     * @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) {
+        return setTimeout(callback, delay, new Object());
+    }
+
+    /**
+     * <a href="https://developer.mozilla.org/en-US/docs/Web/API/setTimeout"><code>setTimeout()</code></a> polyfill.
+     * Sets a timer which executes a given function once the timer expires.
+     *
+     * @param callback function to run after the given delay
+     * @param delay time in milliseconds that the timer should wait before the callback is executed
+     * @param args
+     * @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) {
+        long id = lastId.incrementAndGet();
+        ScheduledCompletableFuture<Object> future = createFuture(id, ZonedDateTime.now().plusNanos(delay * 1000000),
+                callback);
+        idSchedulerMapping.put(id, future);
+        return id;
+    }
+
+    /**
+     * <a href="https://developer.mozilla.org/en-US/docs/Web/API/clearTimeout"><code>clearTimeout()</code></a> polyfill.
+     * Cancels a timeout previously created by <code>setTimeout()</code>.
+     *
+     * @param timeoutId The identifier of the timeout you want to cancel. This ID was returned by the corresponding call
+     *            to setTimeout().
+     */
+    public void clearTimeout(long timeoutId) {
+        ScheduledCompletableFuture<Object> scheduled = idSchedulerMapping.remove(timeoutId);
+        if (scheduled != null) {
+            scheduled.cancel(true);
         }
+    }
 
-        public TimerImpl(Scheduler scheduler, ZonedDateTime startTime, SchedulerRunnable runnable,
+    /**
-                @Nullable String identifier) {
+     * Schedules a callback to run in a loop with a given delay between the executions.
-            this.scheduler = scheduler;
+     *
-            this.startTime = startTime;
+     * @param id timerId to append to the identifier base for naming the scheduled job
-            this.runnable = runnable;
+     * @param delay time in milliseconds that the timer should delay in between executions of the callback
-            this.identifier = identifier;
+     * @param callback function to run
+     */
+    private void createLoopingFuture(long id, Long delay, Runnable callback) {
+        ScheduledCompletableFuture<Object> future = scheduler.schedule(() -> {
+            synchronized (lock) {
+                callback.run();
+            }
+        }, identifier + ".interval." + id, new LoopingAdjuster(Duration.ofMillis(delay)));
+        idSchedulerMapping.put(id, future);
+    }
 
-            future = scheduler.schedule(runnable, identifier, startTime.toInstant());
+    /**
+     * <a href="https://developer.mozilla.org/en-US/docs/Web/API/setInterval"><code>setInterval()</code></a> polyfill.
+     * Repeatedly calls a function with a fixed time delay between each call.
+     *
+     * @param callback function to run
+     * @param delay time in milliseconds that the timer should delay in between executions of the callback
+     * @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) {
+        return setInterval(callback, delay, new Object());
+    }
+
+    /**
+     * <a href="https://developer.mozilla.org/en-US/docs/Web/API/setInterval"><code>setInterval()</code></a> polyfill.
+     * Repeatedly calls a function with a fixed time delay between each call.
+     *
+     * @param callback function to run
+     * @param delay time in milliseconds that the timer should delay in between executions of the callback
+     * @param args
+     * @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) {
+        long id = lastId.incrementAndGet();
+        createLoopingFuture(id, delay, callback);
+        return id;
+    }
+
+    /**
+     * <a href="https://developer.mozilla.org/en-US/docs/Web/API/clearInterval"><code>clearInterval()</code></a>
+     * polyfill.
+     * Cancels a timed, repeating action which was previously established by a call to <code>setInterval()</code>.
+     *
+     * @param intervalID The identifier of the repeated action you want to cancel. This ID was returned by the
+     *            corresponding call to <code>setInterval()</code>.
+     */
+    public void clearInterval(long intervalID) {
+        clearTimeout(intervalID);
+    }
+
+    /**
+     * Cancels all timed actions (i.e. timeouts and intervals) that were created with this instance of
+     * {@link ThreadsafeTimers}.
+     * Should be called in a de-initialization/unload hook of the script engine to avoid having scheduled jobs that are
+     * running endless.
+     */
+    public void clearAll() {
+        idSchedulerMapping.forEach((id, future) -> future.cancel(true));
+        idSchedulerMapping.clear();
+    }
+
+    /**
+     * This is a temporal adjuster that takes a single delay.
+     * This adjuster makes the scheduler run as a fixed rate scheduler from the first time adjustInto was called.
+     *
+     * @author Florian Hotze - Initial contribution
+     */
+    private static class LoopingAdjuster implements SchedulerTemporalAdjuster {
+
+        private Duration delay;
+        private @Nullable Temporal timeDone;
+
+        LoopingAdjuster(Duration delay) {
+            this.delay = delay;
         }
 
         @Override
-        public boolean cancel() {
+        public boolean isDone(Temporal temporal) {
-            return future.cancel(true);
+            // Always return false so that a new job will be scheduled
+            return false;
         }
 
         @Override
-        public synchronized boolean reschedule(ZonedDateTime newTime) {
+        public Temporal adjustInto(Temporal temporal) {
-            future.cancel(false);
+            Temporal localTimeDone = timeDone;
-            future = scheduler.schedule(runnable, identifier, newTime.toInstant());
+            Temporal nextTime;
-            return true;
+            if (localTimeDone != null) {
-        }
+                nextTime = localTimeDone.plus(delay);
-
+            } else {
-        @Override
+                nextTime = temporal.plus(delay);
-        public @Nullable ZonedDateTime getExecutionTime() {
+            }
-            return future.isCancelled() ? null : ZonedDateTime.now().plusNanos(future.getDelay(TimeUnit.NANOSECONDS));
+            timeDone = nextTime;
-        }
+            return nextTime;
-
-        @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();
         }
     }
 }

bundles/org.openhab.automation.jsscripting/src/main/resources/node_modules/@jsscripting-globals.js

@@ -1,3 +1,4 @@
+// ThreadsafeTimers is injected into the JS runtime
 
 (function (global) {
     'use strict';
@@ -5,8 +6,9 @@
     // Append the script file name OR rule UID depending on which is available
     const defaultIdentifier = "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 ZonedDateTime = Java.type('java.time.ZonedDateTime');
     const formatRegExp = /%[sdj%]/g;
+    // Pass the defaultIdentifier to ThreadsafeTimers to enable naming of scheduled jobs
+    ThreadsafeTimers.setIdentifier(defaultIdentifier);
 
     function createLogger(name = defaultIdentifier) {
         return Java.type("org.slf4j.LoggerFactory").getLogger(name);
@@ -162,61 +164,24 @@
         },
 
         // Allow user customizable logging names
+        // Be aware that a log4j2 required a logger defined for the logger name, otherwise messages won't be logged!
         set loggerName(name) {
             log = createLogger(name);
             this._loggerName = name;
+            ThreadsafeTimers.setIdentifier(name);
         },
 
         get loggerName() {
-            return this._loggerName || defaultLoggerName;
+            return this._loggerName || defaultIdentifier;
         }
     };
 
-    function setTimeout(cb, delay) {
-        const args = Array.prototype.slice.call(arguments, 2);
-        return ThreadsafeTimers.createTimerWithArgument(
-            defaultIdentifier + '.setTimeout',
-            ZonedDateTime.now().plusNanos(delay * 1000000),
-            args,
-            function (args) {
-                cb.apply(global, args);
-            }
-        );
-    }
-
-    function clearTimeout(timer) {
-        if (timer !== undefined && timer.isActive()) {
-            timer.cancel();
-        }
-    }
-
-    function setInterval(cb, delay) {
-        const args = Array.prototype.slice.call(arguments, 2);
-        const delayNanos = delay * 1000000
-        let timer = ThreadsafeTimers.createTimerWithArgument(
-            defaultIdentifier + '.setInterval',
-            ZonedDateTime.now().plusNanos(delayNanos),
-            args,
-            function (args) {
-                cb.apply(global, args);
-                if (!timer.isCancelled()) {
-                    timer.reschedule(ZonedDateTime.now().plusNanos(delayNanos));
-                }
-            }
-        );
-        return timer;
-    }
-
-    function clearInterval(timer) {
-        clearTimeout(timer);
-    }
-
     // Polyfill common NodeJS functions onto the global object
     globalThis.console = console;
-    globalThis.setTimeout = setTimeout;
+    globalThis.setTimeout = ThreadsafeTimers.setTimeout;
-    globalThis.clearTimeout = clearTimeout;
+    globalThis.clearTimeout = ThreadsafeTimers.clearTimeout;
-    globalThis.setInterval = setInterval;
+    globalThis.setInterval = ThreadsafeTimers.setInterval;
-    globalThis.clearInterval = clearInterval;
+    globalThis.clearInterval = ThreadsafeTimers.clearInterval;
 
     // Support legacy NodeJS libraries 
     globalThis.global = globalThis;