refactor: rewrite setTimeout/clearTimeout in JS using internal bindings

This reverts commit 3b4478c and 06ac480.

Author: Xmader

include/internalBinding.hh

@@ -14,6 +14,7 @@
 
 namespace InternalBinding {
   extern JSFunctionSpec utils[];
+  extern JSFunctionSpec timers[];
 }
 
 JSObject *createInternalBindingsForNamespace(JSContext *cx, JSFunctionSpec *methodSpecs);

python/pythonmonkey/__init__.py

@@ -6,6 +6,7 @@
 import importlib.metadata
 __version__= importlib.metadata.version(__name__)
 
-# Load the module by default to make `console`/`atob`/`btoa` globally available
+# Load the module by default to expose global APIs
 require("console")
 require("base64")
+require("timers")

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

@@ -30,4 +30,19 @@ declare function internalBinding(namespace: "utils"): {
   getProxyDetails<T extends object>(proxy: T): undefined | [target: T, handler: ProxyHandler<T>];
 };
 
+declare function internalBinding(namespace: "timers"): {
+  /**
+   * internal binding helper for the `setTimeout` global function
+   * 
+   * **UNSAFE**, does not perform argument type checks
+   * @return timeoutId
+   */
+  enqueueWithDelay(handler: Function, delaySeconds: number): number;
+
+  /**
+   * internal binding helper for the `clearTimeout` global function
+   */
+  cancelByTimeoutId(timeoutId: number): void;
+};
+
 export = internalBinding;

python/pythonmonkey/builtin_modules/timers.js

@@ -0,0 +1,64 @@
+/**
+ * @file     timers.js
+ * @author   Tom Tang <[email protected]>
+ * @date     May 2023
+ */
+const internalBinding = require("internal-binding")
+
+const { 
+  enqueueWithDelay,
+  cancelByTimeoutId
+} = internalBinding("timers")
+
+/**
+ * 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 {number} timeoutId
+ */
+function setTimeout(handler, delayMs = 0, ...args) {
+  // Ensure the first parameter is a function
+  if (typeof handler !== "function") {
+    throw new TypeError("The first parameter to setTimeout() is not a function")
+  }
+
+  // `setTimeout` allows passing additional arguments to the callback, as spec-ed
+  // FIXME (Tom Tang): the spec doesn't allow additional arguments to be passed if the original `handler` is not a function
+  const thisArg = globalThis // HTML spec requires `thisArg` to be the global object
+  // 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)
+
+  // Get the delay time in seconds
+  //  JS `setTimeout` takes milliseconds, but Python takes seconds
+  delayMs = Number(delayMs) || 0 // ensure the `delayMs` is a `number`, explicitly do type coercion
+  if (delayMs < 0) { delayMs = 0 } // as spec-ed
+  const delaySeconds = delayMs / 1000 // convert ms to s
+
+  return enqueueWithDelay(boundHandler, delaySeconds)
+}
+
+/**
+ * Implement the `clearTimeout` global function
+ * @see https://developer.mozilla.org/en-US/docs/Web/API/clearTimeout and
+ * @see https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-cleartimeout
+ * @param {number} timeoutId
+ * @return {void}
+ */
+function clearTimeout(timeoutId) {
+  return cancelByTimeoutId(timeoutId)
+}
+
+if (!globalThis.setTimeout) {
+  globalThis.setTimeout = setTimeout;
+}
+if (!globalThis.clearTimeout) {
+  globalThis.clearTimeout = clearTimeout;
+}
+
+exports.setTimeout = setTimeout;
+exports.clearTimeout = clearTimeout;

src/internalBinding.cc

@@ -23,6 +23,8 @@ JSObject *createInternalBindingsForNamespace(JSContext *cx, JSFunctionSpec *meth
 JSObject *getInternalBindingsByNamespace(JSContext *cx, JSLinearString *namespaceStr) {
   if (JS_LinearStringEqualsLiteral(namespaceStr, "utils")) {
     return createInternalBindingsForNamespace(cx, InternalBinding::utils);
+  } else if (JS_LinearStringEqualsLiteral(namespaceStr, "timers")) {
+    return createInternalBindingsForNamespace(cx, InternalBinding::timers);
   } else { // not found
     return nullptr;
   }

src/internalBinding/timers.cc

@@ -0,0 +1,67 @@
+/**
+ * @file timers.cc
+ * @author Tom Tang ([email protected])
+ * @brief Implement functions in `internalBinding("timers")`
+ */
+
+#include "include/internalBinding.hh"
+#include "include/pyTypeFactory.hh"
+#include "include/PyEventLoop.hh"
+
+#include <jsapi.h>
+
+/**
+ * See function declarations in python/pythonmonkey/builtin_modules/internal-binding.d.ts :
+ *    `declare function internalBinding(namespace: "timers")`
+ */
+
+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();
+
+  // Convert to a Python function
+  // FIXME (Tom Tang): memory leak, not free-ed
+  JS::RootedObject *thisv = new JS::RootedObject(cx, nullptr);
+  JS::RootedValue *jobArg = new JS::RootedValue(cx, jobArgVal);
+  PyObject *job = pyTypeFactory(cx, thisv, jobArg)->getPyObject();
+
+  // Schedule job to the running Python event-loop
+  PyEventLoop loop = PyEventLoop::getRunningLoop();
+  if (!loop.initialized()) return false;
+  PyEventLoop::AsyncHandle handle = loop.enqueueWithDelay(job, delaySeconds);
+
+  // Return the `timeoutID` to use in `clearTimeout`
+  args.rval().setDouble((double)PyEventLoop::AsyncHandle::getUniqueId(std::move(handle)));
+  return true;
+}
+
+// TODO (Tom Tang): move argument checks to the JavaScript side
+static bool cancelByTimeoutId(JSContext *cx, unsigned argc, JS::Value *vp) {
+  using AsyncHandle = PyEventLoop::AsyncHandle;
+  JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
+  JS::HandleValue timeoutIdArg = args.get(0);
+
+  args.rval().setUndefined();
+
+  // silently does nothing when an invalid timeoutID is passed in
+  if (!timeoutIdArg.isInt32()) {
+    return true;
+  }
+
+  // Retrieve the AsyncHandle by `timeoutID`
+  int32_t timeoutID = timeoutIdArg.toInt32();
+  AsyncHandle *handle = AsyncHandle::fromId((uint32_t)timeoutID);
+  if (!handle) return true; // does nothing on invalid timeoutID
+
+  // Cancel this job on Python event-loop
+  handle->cancel();
+
+  return true;
+}
+
+JSFunctionSpec InternalBinding::timers[] = {
+  JS_FN("enqueueWithDelay", enqueueWithDelay, /* nargs */ 2, 0),
+  JS_FN("cancelByTimeoutId", cancelByTimeoutId, 1, 0),
+  JS_FS_END
+};

src/modules/pythonmonkey/pythonmonkey.cc

@@ -297,90 +297,6 @@ struct PyModuleDef pythonmonkey =
 
 PyObject *SpiderMonkeyError = NULL;
 
-// Implement the `setTimeout` global function
-//    https://developer.mozilla.org/en-US/docs/Web/API/setTimeout
-//    https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-settimeout
-static bool setTimeout(JSContext *cx, unsigned argc, JS::Value *vp) {
-  JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
-
-  // Ensure the first parameter is a function
-  // We don't support passing a `code` string to `setTimeout` (yet)
-  JS::HandleValue jobArgVal = args.get(0);
-  bool jobArgIsFunction = jobArgVal.isObject() && js::IsFunctionObject(&jobArgVal.toObject());
-  if (!jobArgIsFunction) {
-    JS_ReportErrorNumberASCII(cx, nullptr, nullptr, JSErrNum::JSMSG_NOT_FUNCTION, "The first parameter to setTimeout()");
-    return false;
-  }
-
-  // Get the function to be executed
-  // FIXME (Tom Tang): memory leak, not free-ed
-  JS::RootedObject *thisv = new JS::RootedObject(cx, JS::GetNonCCWObjectGlobal(&args.callee())); // HTML spec requires `thisArg` to be the global object
-  JS::RootedValue *jobArg = new JS::RootedValue(cx, jobArgVal);
-  // `setTimeout` allows passing additional arguments to the callback, as spec-ed
-  if (args.length() > 2) { // having additional arguments
-    // 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
-    JS::RootedVector<JS::Value> bindArgs(cx);
-    (void)bindArgs.append(JS::ObjectValue(**thisv)); /** @todo XXXwg handle return value */
-    for (size_t j = 2; j < args.length(); j++) {
-      (void)bindArgs.append(args[j]); /** @todo XXXwg handle return value */
-    }
-    JS::RootedObject jobArgObj = JS::RootedObject(cx, &jobArgVal.toObject());
-    JS_CallFunctionName(cx, jobArgObj, "bind", JS::HandleValueArray(bindArgs), jobArg); // jobArg = jobArg.bind(thisv, ...bindArgs)
-  }
-  // Convert to a Python function
-  PyObject *job = pyTypeFactory(cx, thisv, jobArg)->getPyObject();
-
-  // Get the delay time
-  //  JS `setTimeout` takes milliseconds, but Python takes seconds
-  double delayMs = 0; // use value of 0 if the delay parameter is omitted
-  if (args.hasDefined(1)) { JS::ToNumber(cx, args[1], &delayMs); } // implicitly do type coercion to a `number`
-  if (delayMs < 0) { delayMs = 0; } // as spec-ed
-  double delaySeconds = delayMs / 1000; // convert ms to s
-
-  // Schedule job to the running Python event-loop
-  PyEventLoop loop = PyEventLoop::getRunningLoop();
-  if (!loop.initialized()) return false;
-  PyEventLoop::AsyncHandle handle = loop.enqueueWithDelay(job, delaySeconds);
-
-  // Return the `timeoutID` to use in `clearTimeout`
-  args.rval().setDouble((double)PyEventLoop::AsyncHandle::getUniqueId(std::move(handle)));
-
-  return true;
-}
-
-// Implement the `clearTimeout` global function
-//    https://developer.mozilla.org/en-US/docs/Web/API/clearTimeout
-//    https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-cleartimeout
-static bool clearTimeout(JSContext *cx, unsigned argc, JS::Value *vp) {
-  using AsyncHandle = PyEventLoop::AsyncHandle;
-  JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
-  JS::HandleValue timeoutIdArg = args.get(0);
-
-  args.rval().setUndefined();
-
-  // silently does nothing when an invalid timeoutID is passed in
-  if (!timeoutIdArg.isInt32()) {
-    return true;
-  }
-
-  // Retrieve the AsyncHandle by `timeoutID`
-  int32_t timeoutID = timeoutIdArg.toInt32();
-  AsyncHandle *handle = AsyncHandle::fromId((uint32_t)timeoutID);
-  if (!handle) return true; // does nothing on invalid timeoutID
-
-  // Cancel this job on Python event-loop
-  handle->cancel();
-
-  return true;
-}
-
-static JSFunctionSpec jsGlobalFunctions[] = {
-  JS_FN("setTimeout", setTimeout, /* nargs */ 2, 0),
-  JS_FN("clearTimeout", clearTimeout, 1, 0),
-  JS_FS_END
-};
-
 PyMODINIT_FUNC PyInit_pythonmonkey(void)
 {
   PyDateTime_IMPORT;
@@ -425,11 +341,6 @@ PyMODINIT_FUNC PyInit_pythonmonkey(void)
 
   autoRealm = new JSAutoRealm(GLOBAL_CX, *global);
 
-  if (!JS_DefineFunctions(GLOBAL_CX, *global, jsGlobalFunctions)) {
-    PyErr_SetString(SpiderMonkeyError, "Spidermonkey could not define global functions.");
-    return NULL;
-  }
-
   JS_SetGCCallback(GLOBAL_CX, handleSharedPythonMonkeyMemory, NULL);
 
   // XXX: SpiderMonkey bug???