Merge pull request #311 from Distributive-Network/Xmader/feat/timers-setInterval

Implement `setInterval` functions & Fix timers memory bug

Author: Xmader

include/PyEventLoop.hh

@@ -32,7 +32,6 @@ public:
    */
   struct AsyncHandle {
     using id_t = uint32_t;
-    using id_ptr_pair = std::pair<id_t, AsyncHandle *>;
   public:
     explicit AsyncHandle(PyObject *handle) : _handle(handle) {};
     AsyncHandle(const AsyncHandle &old) = delete; // forbid copy-initialization
@@ -42,16 +41,29 @@ public:
         Py_XDECREF(_handle);
       }
     }
-    static inline id_ptr_pair newEmpty() {
+
+    /**
+     * @brief Create a new `AsyncHandle` without an associated `asyncio.Handle` Python object
+     * @return the timeoutId
+     */
+    static inline id_t newEmpty() {
       auto handle = AsyncHandle(Py_None);
-      return AsyncHandle::getUniqueIdAndPtr(std::move(handle));
+      return AsyncHandle::getUniqueId(std::move(handle));
     }
 
     /**
      * @brief Cancel the scheduled event-loop job.
      * If the job has already been canceled or executed, this method has no effect.
      */
     void cancel();
+    /**
+     * @return true if the job has been cancelled.
+     */
+    bool cancelled();
+    /**
+     * @return true if the job function has already been executed or cancelled.
+     */
+    bool _finishedOrCancelled();
 
     /**
      * @brief Get the unique `timeoutID` for JS `setTimeout`/`clearTimeout` methods
@@ -69,11 +81,6 @@ public:
         return nullptr; // invalid timeoutID
       }
     }
-    static inline id_ptr_pair getUniqueIdAndPtr(AsyncHandle &&handle) {
-      auto timeoutID = getUniqueId(std::move(handle));
-      auto ptr = fromId(timeoutID);
-      return std::make_pair(timeoutID, ptr);
-    }
 
     /**
      * @brief Get the underlying `asyncio.Handle` Python object
@@ -104,7 +111,9 @@ public:
     inline void addRef() {
       if (!_refed) {
         _refed = true;
-        PyEventLoop::_locker->incCounter();
+        if (!_finishedOrCancelled()) { // noop if the timer is finished or canceled
+          PyEventLoop::_locker->incCounter();
+        }
       }
     }
 
@@ -132,9 +141,10 @@ public:
    * @brief Schedule a job to the Python event-loop, with the given delay
    * @param jobFn - The JS event-loop job converted to a Python function
    * @param delaySeconds - The job function will be called after the given number of seconds
-   * @return the timeoutId and a pointer to the AsyncHandle
+   * @param repeat - If true, the job will be executed repeatedly on a fixed interval
+   * @return the timeoutId
    */
-  [[nodiscard]] AsyncHandle::id_ptr_pair enqueueWithDelay(PyObject *jobFn, double delaySeconds);
+  [[nodiscard]] AsyncHandle::id_t enqueueWithDelay(PyObject *jobFn, double delaySeconds, bool repeat);
 
   /**
    * @brief C++ wrapper for Python `asyncio.Future` class

python/pythonmonkey/builtin_modules/internal-binding.d.ts

@@ -34,12 +34,14 @@ declare function internalBinding(namespace: "utils"): {
 
 declare function internalBinding(namespace: "timers"): {
   /**
-   * internal binding helper for the `setTimeout` global function
+   * internal binding helper for the `setTimeout`/`setInterval` global functions
    * 
    * **UNSAFE**, does not perform argument type checks
+   * 
+   * @param repeat The call is to `setInterval` if true
    * @return timeoutId
    */
-  enqueueWithDelay(handler: Function, delaySeconds: number): number;
+  enqueueWithDelay(handler: Function, delaySeconds: number, repeat: boolean): number;
 
   /**
    * internal binding helper for the `clearTimeout` global function

python/pythonmonkey/builtin_modules/timers.js

@@ -15,6 +15,8 @@ const {
   timerRemoveRef,
 } = internalBinding('timers');
 
+const { DOMException } = require('dom-exception');
+
 /**
  * Implement Node.js-style `timeoutId` class returned from setTimeout() and setInterval()
  * @see https://nodejs.org/api/timers.html#class-timeout
@@ -63,11 +65,16 @@ class Timeout
   }
 
   /**
-   * @returns a number that can be used to reference this timeout
+   * Sets the timer's start time to the current time, 
+   * and reschedules the timer to call its callback at the previously specified duration adjusted to the current time.
+   * 
+   * Using this on a timer that has already called its callback will reactivate the timer.
    */
-  [Symbol.toPrimitive]()
+  refresh()
   {
-    return this.#numericId;
+    throw new DOMException('Timeout.refresh() method is not supported by PythonMonkey.', 'NotSupportedError');
+    // TODO: Do we really need to closely resemble the Node.js API?
+    // This one is not easy to implement since we need to memorize the callback function and delay parameters in every `setTimeout`/`setInterval` call.
   }
 
   /**
@@ -78,18 +85,23 @@ class Timeout
   {
     return clearTimeout(this);
   }
+
+  /**
+   * @returns a number that can be used to reference this timeout
+   */
+  [Symbol.toPrimitive]()
+  {
+    return this.#numericId;
+  }
 }
 
 /**
- * Implement the `setTimeout` global function
- * @see https://developer.mozilla.org/en-US/docs/Web/API/setTimeout and
- * @see https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-settimeout
+ * Normalize the arguments to `setTimeout` or `setInterval`
  * @param {Function | string} handler
- * @param {number} delayMs timeout milliseconds, use value of 0 if this is omitted
- * @param {any[]} args additional arguments to be passed to the `handler`
- * @return {Timeout} timeoutId
+ * @param {number} delayMs timeout milliseconds
+ * @param {any[]} additionalArgs additional arguments to be passed to the `handler`
  */
-function setTimeout(handler, delayMs = 0, ...args) 
+function _normalizeTimerArgs(handler, delayMs, additionalArgs)
 {
   // Ensure the first parameter is a function
   // We support passing a `code` string to `setTimeout` as the callback function
@@ -102,7 +114,7 @@ function setTimeout(handler, delayMs = 0, ...args)
   // Wrap the job function into a bound function with the given additional arguments
   //    https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/bind
   /** @type {Function} */
-  const boundHandler = handler.bind(thisArg, ...args);
+  const boundHandler = handler.bind(thisArg, ...additionalArgs);
 
   // Get the delay time in seconds
   //  JS `setTimeout` takes milliseconds, but Python takes seconds
@@ -111,7 +123,22 @@ function setTimeout(handler, delayMs = 0, ...args)
     delayMs = 0; // as spec-ed
   const delaySeconds = delayMs / 1000; // convert ms to s
 
-  return new Timeout(enqueueWithDelay(boundHandler, delaySeconds));
+  return { boundHandler, delaySeconds };
+}
+
+/**
+ * Implement the `setTimeout` global function
+ * @see https://developer.mozilla.org/en-US/docs/Web/API/setTimeout and
+ * @see https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-settimeout
+ * @param {Function | string} handler
+ * @param {number} delayMs timeout milliseconds, use value of 0 if this is omitted
+ * @param {any[]} args additional arguments to be passed to the `handler`
+ * @return {Timeout} timeoutId
+ */
+function setTimeout(handler, delayMs = 0, ...args) 
+{
+  const { boundHandler, delaySeconds } = _normalizeTimerArgs(handler, delayMs, args);
+  return new Timeout(enqueueWithDelay(boundHandler, delaySeconds, false));
 }
 
 /**
@@ -130,13 +157,43 @@ function clearTimeout(timeoutId)
   return cancelByTimeoutId(Number(timeoutId));
 }
 
+/**
+ * Implement the `setInterval` global function
+ * @see https://developer.mozilla.org/en-US/docs/Web/API/setInterval and
+ * @see https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-setinterval
+ * @param {Function | string} handler
+ * @param {number} delayMs timeout milliseconds, use value of 0 if this is omitted
+ * @param {any[]} args additional arguments to be passed to the `handler`
+ * @return {Timeout} timeoutId
+ */
+function setInterval(handler, delayMs = 0, ...args) 
+{
+  const { boundHandler, delaySeconds } = _normalizeTimerArgs(handler, delayMs, args);
+  return new Timeout(enqueueWithDelay(boundHandler, delaySeconds, true));
+}
+
+/**
+ * Implement the `clearInterval` global function
+ * @alias to `clearTimeout`
+ * @see https://developer.mozilla.org/en-US/docs/Web/API/clearInterval
+ */
+const clearInterval = clearTimeout;
+
 // expose the `Timeout` class
 setTimeout.Timeout = Timeout;
+setInterval.Timeout = Timeout;
 
 if (!globalThis.setTimeout)
   globalThis.setTimeout = setTimeout;
 if (!globalThis.clearTimeout)
   globalThis.clearTimeout = clearTimeout;
 
+if (!globalThis.setInterval)
+  globalThis.setInterval = setInterval;
+if (!globalThis.clearInterval)
+  globalThis.clearInterval = clearInterval;
+
 exports.setTimeout = setTimeout;
 exports.clearTimeout = clearTimeout;
+exports.setInterval = setInterval;
+exports.clearInterval = clearInterval;

python/pythonmonkey/cli/pmjs.py

@@ -343,10 +343,17 @@ def main():
         elif o in ("-i", "--interactive"):
             forceRepl = True
         elif o in ("-e", "--eval"):
-            pm.eval(a, evalOpts)
+            async def runEval():
+                pm.eval(a, evalOpts)
+                await pm.wait()
+            asyncio.run(runEval())
             enterRepl = False
         elif o in ("-p", "--print"):
-            print(pm.eval(a, evalOpts))
+            async def runEvalPrint():
+                ret = pm.eval(a, evalOpts)
+                pm.eval("ret => console.log(ret)", evalOpts)(ret)
+                await pm.wait()
+            asyncio.run(runEvalPrint())
             enterRepl = False
         elif o in ("-r", "--require"):
             globalThis.require(a)

python/pythonmonkey/global.d.ts

@@ -56,6 +56,9 @@ declare var btoa: typeof import("base64").btoa;
 // Expose `setTimeout`/`clearTimeout` APIs
 declare var setTimeout: typeof import("timers").setTimeout;
 declare var clearTimeout: typeof import("timers").clearTimeout;
+// Expose `setInterval`/`clearInterval` APIs
+declare var setInterval: typeof import("timers").setInterval;
+declare var clearInterval: typeof import("timers").clearInterval;
 
 // Expose `URL`/`URLSearchParams` APIs
 declare var URL: typeof import("url").URL;

src/PyEventLoop.cc

@@ -28,21 +28,37 @@ static PyObject *eventLoopJobWrapper(PyObject *jobFn, PyObject *Py_UNUSED(_)) {
 }
 static PyMethodDef loopJobWrapperDef = {"eventLoopJobWrapper", eventLoopJobWrapper, METH_NOARGS, NULL};
 
+static PyObject *_enqueueWithDelay(PyObject *_loop, PyEventLoop::AsyncHandle::id_t handleId, PyObject *jobFn, double delaySeconds, bool repeat);
+
 /**
  * @brief Wrapper to remove the reference of the timer after the job finishes
  */
-static PyObject *timerJobWrapper(PyObject *jobFn, PyObject *handlerPtr) {
-  auto handle = (PyEventLoop::AsyncHandle *)PyLong_AsVoidPtr(handlerPtr);
+static PyObject *timerJobWrapper(PyObject *jobFn, PyObject *args) {
+  PyObject *_loop = PyTuple_GetItem(args, 0);
+  PyEventLoop::AsyncHandle::id_t handleId = PyLong_AsLong(PyTuple_GetItem(args, 1));
+  double delaySeconds = PyFloat_AsDouble(PyTuple_GetItem(args, 2));
+  bool repeat = (bool)PyLong_AsLong(PyTuple_GetItem(args, 3));
+  auto handle = PyEventLoop::AsyncHandle::fromId(handleId);
+
   PyObject *ret = PyObject_CallObject(jobFn, NULL); // jobFn()
   Py_XDECREF(ret); // don't care about its return value
-  handle->removeRef();
-  if (PyErr_Occurred()) {
+
+  PyObject *errType, *errValue, *traceback; // we can't call any Python code unless the error indicator is clear
+  PyErr_Fetch(&errType, &errValue, &traceback);
+  if (repeat && !handle->cancelled()) {
+    _enqueueWithDelay(_loop, handleId, jobFn, delaySeconds, repeat);
+  } else {
+    handle->removeRef();
+  }
+
+  if (errType != NULL) { // PyErr_Occurred()
+    PyErr_Restore(errType, errValue, traceback);
     return NULL;
   } else {
     Py_RETURN_NONE;
   }
 }
-static PyMethodDef timerJobWrapperDef = {"timerJobWrapper", timerJobWrapper, METH_O, NULL};
+static PyMethodDef timerJobWrapperDef = {"timerJobWrapper", timerJobWrapper, METH_VARARGS, NULL};
 
 PyEventLoop::AsyncHandle PyEventLoop::enqueue(PyObject *jobFn) {
   PyEventLoop::_locker->incCounter();
@@ -53,20 +69,28 @@ PyEventLoop::AsyncHandle PyEventLoop::enqueue(PyObject *jobFn) {
   return PyEventLoop::AsyncHandle(asyncHandle);
 }
 
-PyEventLoop::AsyncHandle::id_ptr_pair PyEventLoop::enqueueWithDelay(PyObject *jobFn, double delaySeconds) {
-  auto handler = PyEventLoop::AsyncHandle::newEmpty();
+static PyObject *_enqueueWithDelay(PyObject *_loop, PyEventLoop::AsyncHandle::id_t handleId, PyObject *jobFn, double delaySeconds, bool repeat) {
   PyObject *wrapper = PyCFunction_New(&timerJobWrapperDef, jobFn);
-  PyObject *handlerPtr = PyLong_FromVoidPtr(handler.second);
   // Schedule job to the Python event-loop
   //    https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.call_later
-  PyObject *asyncHandle = PyObject_CallMethod(_loop, "call_later", "dOO", delaySeconds, wrapper, handlerPtr); // https://docs.python.org/3/c-api/arg.html#c.Py_BuildValue
-  if (asyncHandle == nullptr) {
+  PyObject *asyncHandle = PyObject_CallMethod(_loop, "call_later", "dOOIdb", delaySeconds, wrapper, _loop, handleId, delaySeconds, repeat); // https://docs.python.org/3/c-api/arg.html#c.Py_BuildValue
+  if (!asyncHandle) {
+    return nullptr; // RuntimeError
+  }
+
+  auto handle = PyEventLoop::AsyncHandle::fromId(handleId);
+  Py_XDECREF(handle->swap(asyncHandle));
+  handle->addRef();
+
+  return asyncHandle;
+}
+
+PyEventLoop::AsyncHandle::id_t PyEventLoop::enqueueWithDelay(PyObject *jobFn, double delaySeconds, bool repeat) {
+  auto handleId = PyEventLoop::AsyncHandle::newEmpty();
+  if (!_enqueueWithDelay(_loop, handleId, jobFn, delaySeconds, repeat)) {
     PyErr_Print(); // RuntimeError: Non-thread-safe operation invoked on an event loop other than the current one
-    return handler;
   }
-  handler.second->swap(asyncHandle);
-  handler.second->addRef();
-  return handler;
+  return handleId;
 }
 
 PyEventLoop::Future PyEventLoop::createFuture() {
@@ -172,19 +196,31 @@ PyEventLoop PyEventLoop::getRunningLoop() {
 }
 
 void PyEventLoop::AsyncHandle::cancel() {
-  PyObject *scheduled = PyObject_GetAttrString(_handle, "_scheduled"); // this attribute only exists on asyncio.TimerHandle returned by loop.call_later
-                                                                       // NULL if no such attribute (on a strict asyncio.Handle returned by loop.call_soon)
-  bool finishedOrCanceled = scheduled && scheduled == Py_False; // the job function has already been executed or canceled
-  if (!finishedOrCanceled) {
+  if (!_finishedOrCancelled()) {
     removeRef(); // automatically unref at finish
   }
-  Py_XDECREF(scheduled);
 
   // https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.Handle.cancel
   PyObject *ret = PyObject_CallMethod(_handle, "cancel", NULL); // returns None
   Py_XDECREF(ret);
 }
 
+bool PyEventLoop::AsyncHandle::cancelled() {
+  // https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.Handle.cancelled
+  PyObject *ret = PyObject_CallMethod(_handle, "cancelled", NULL); // returns Python bool
+  bool cancelled = ret == Py_True;
+  Py_XDECREF(ret);
+  return cancelled;
+}
+
+bool PyEventLoop::AsyncHandle::_finishedOrCancelled() {
+  PyObject *scheduled = PyObject_GetAttrString(_handle, "_scheduled"); // this attribute only exists on asyncio.TimerHandle returned by loop.call_later
+                                                                       // NULL if no such attribute (on a strict asyncio.Handle returned by loop.call_soon)
+  bool notScheduled = scheduled && scheduled == Py_False; // not scheduled means the job function has already been executed or canceled
+  Py_XDECREF(scheduled);
+  return notScheduled;
+}
+
 void PyEventLoop::Future::setResult(PyObject *result) {
   // https://docs.python.org/3/library/asyncio-future.html#asyncio.Future.set_result
   PyObject *ret = PyObject_CallMethod(_future, "set_result", "O", result); // returns None

src/internalBinding/timers.cc

@@ -23,17 +23,19 @@ static bool enqueueWithDelay(JSContext *cx, unsigned argc, JS::Value *vp) {
   JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
   JS::HandleValue jobArgVal = args.get(0);
   double delaySeconds = args.get(1).toNumber();
+  bool repeat = args.get(2).toBoolean();
 
   // Convert to a Python function
   JS::RootedValue jobArg(cx, jobArgVal);
   PyObject *job = pyTypeFactory(cx, jobArg)->getPyObject();
   // Schedule job to the running Python event-loop
   PyEventLoop loop = PyEventLoop::getRunningLoop();
   if (!loop.initialized()) return false;
-  PyEventLoop::AsyncHandle::id_ptr_pair handler = loop.enqueueWithDelay(job, delaySeconds);
+  PyEventLoop::AsyncHandle::id_t handleId = loop.enqueueWithDelay(job, delaySeconds, repeat);
+  Py_DECREF(job);
 
   // Return the `timeoutID` to use in `clearTimeout`
-  args.rval().setNumber(handler.first);
+  args.rval().setNumber(handleId);
   return true;
 }