Skip to content

Commit 7aadec4

Browse files
zollqirzollqir
authored
Merge pull request #86 from Distributive-Network/Xmader/feat/better-setTimeout
Better `setTimeout` APIs
2 parents 50d3587 + e805c93 commit 7aadec4

File tree

10 files changed

+173
-114
lines changed

10 files changed

+173
-114
lines changed

‎.eslintrc.js‎

Lines changed: 10 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -25,26 +25,23 @@ module.exports = {
2525
},
2626
rules: {
2727
'indent': [ 'warn', 2, {
28-
SwitchCase: 1,
29-
ignoredNodes: ['CallExpression', 'ForStatement'],
30-
}
31-
],
28+
SwitchCase: 1,
29+
ignoredNodes: ['CallExpression', 'ForStatement'],
30+
}],
3231
'linebreak-style': [ 'error', 'unix' ],
33-
'quotes': [ 'warn', 'single' ],
3432
'func-call-spacing': [ 'off', 'never' ],
3533
'no-prototype-builtins': 'off',
36-
'quotes': ['warn', 'single', 'avoid-escape'],
34+
'quotes': [ 'warn', 'single', 'avoid-escape' ],
3735
'no-empty': [ 'warn' ],
3836
'no-multi-spaces': [ 'off' ],
3937
'prettier/prettier': [ 'off' ],
4038
'vars-on-top': [ 'error' ],
4139
'no-var': [ 'off' ],
4240
'spaced-comment': [ 'warn' ],
43-
'brace-style': [ 'off' ],
41+
'brace-style': [ 'warn', 'allman' ],
4442
'no-eval': [ 'error' ],
4543
'object-curly-spacing': [ 'warn', 'always' ],
4644
'eqeqeq': [ 'warn', 'always' ],
47-
'no-dupe-keys': [ 'warn' ],
4845
'no-constant-condition': [ 'warn' ],
4946
'no-extra-boolean-cast': [ 'warn' ],
5047
'no-sparse-arrays': [ 'off' ],
@@ -60,16 +57,15 @@ module.exports = {
6057
'no-unused-expressions': [ 'warn', { allowShortCircuit: true, allowTernary: true } ],
6158
'prefer-promise-reject-errors': [ 'error' ],
6259
'no-throw-literal': [ 'error' ],
63-
'semi': [ 'off', { omitLastInOneLineBlock: true }], /* does not work right with exports.X = function allmanStyle */
60+
'semi': [ 'warn' ],
6461
'semi-style': [ 'warn', 'last' ],
65-
'semi-spacing': [ 'error', {'before': false, 'after': true}],
62+
'semi-spacing': [ 'error', { 'before': false, 'after': true }],
6663
'no-extra-semi': [ 'warn' ],
6764
'no-tabs': [ 'error' ],
6865
'symbol-description': [ 'error' ],
6966
'operator-linebreak': [ 'warn', 'before' ],
7067
'new-cap': [ 'warn' ],
7168
'consistent-this': [ 'error', 'that' ],
72-
'no-use-before-define': [ 'error', { functions: false, classes: false } ],
7369
'no-shadow': [ 'error' ],
7470
'no-label-var': [ 'error' ],
7571
'radix': [ 'error' ],
@@ -89,10 +85,9 @@ module.exports = {
8985
allow: ['!!'] /* really only want to allow if(x) and if(!x) but not if(!!x) */
9086
}],
9187
'no-trailing-spaces': [ 'off', {
92-
skipBlankLines: true,
93-
ignoreComments: true
94-
}
95-
],
88+
skipBlankLines: true,
89+
ignoreComments: true
90+
}],
9691
'no-unused-vars': ['warn', {
9792
vars: 'all',
9893
args: 'none',

‎include/internalBinding.hh‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414

1515
namespace InternalBinding {
1616
extern JSFunctionSpec utils[];
17+
extern JSFunctionSpec timers[];
1718
}
1819

1920
JSObject *createInternalBindingsForNamespace(JSContext *cx, JSFunctionSpec *methodSpecs);

‎python/pythonmonkey/__init__.py‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,10 @@
88
__version__= importlib.metadata.version(__name__)
99
del importlib
1010

11-
# Load the module by default to make `console`/`atob`/`btoa` globally available
11+
# Load the module by default to expose global APIs
1212
require("console")
1313
require("base64")
14+
require("timers")
1415

1516
# Add the `.keys()` method on `Object.prototype` to get JSObjectProxy dict() conversion working
1617
# Conversion from a dict-subclass to a strict dict by `dict(subclass)` internally calls the .keys() method to read the dictionary keys,

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,4 +30,19 @@ declare function internalBinding(namespace: "utils"): {
3030
getProxyDetails<T extends object>(proxy: T): undefined | [target: T, handler: ProxyHandler<T>];
3131
};
3232

33+
declare function internalBinding(namespace: "timers"): {
34+
/**
35+
* internal binding helper for the `setTimeout` global function
36+
*
37+
* **UNSAFE**, does not perform argument type checks
38+
* @return timeoutId
39+
*/
40+
enqueueWithDelay(handler: Function, delaySeconds: number): number;
41+
42+
/**
43+
* internal binding helper for the `clearTimeout` global function
44+
*/
45+
cancelByTimeoutId(timeoutId: number): void;
46+
};
47+
3348
export = internalBinding;

‎python/pythonmonkey/builtin_modules/timers.js‎

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/**
2+
* @file timers.js
3+
* @author Tom Tang <[email protected]>
4+
* @date May 2023
5+
*/
6+
const internalBinding = require('internal-binding');
7+
8+
const {
9+
enqueueWithDelay,
10+
cancelByTimeoutId
11+
} = internalBinding('timers');
12+
13+
/**
14+
* Implement the `setTimeout` global function
15+
* @see https://developer.mozilla.org/en-US/docs/Web/API/setTimeout and
16+
* @see https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-settimeout
17+
* @param {Function | string} handler
18+
* @param {number} delayMs timeout milliseconds, use value of 0 if this is omitted
19+
* @param {any[]} args additional arguments to be passed to the `handler`
20+
* @return {number} timeoutId
21+
*/
22+
function setTimeout(handler, delayMs = 0, ...args)
23+
{
24+
// Ensure the first parameter is a function
25+
// We support passing a `code` string to `setTimeout` as the callback function
26+
if (typeof handler !== 'function')
27+
handler = new Function(handler);
28+
29+
// `setTimeout` allows passing additional arguments to the callback, as spec-ed
30+
// FIXME (Tom Tang): the spec doesn't allow additional arguments to be passed if the original `handler` is not a function
31+
const thisArg = globalThis; // HTML spec requires `thisArg` to be the global object
32+
// Wrap the job function into a bound function with the given additional arguments
33+
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/bind
34+
/** @type {Function} */
35+
const boundHandler = handler.bind(thisArg, ...args);
36+
37+
// Get the delay time in seconds
38+
// JS `setTimeout` takes milliseconds, but Python takes seconds
39+
delayMs = Number(delayMs) || 0; // ensure the `delayMs` is a `number`, explicitly do type coercion
40+
if (delayMs < 0)
41+
delayMs = 0; // as spec-ed
42+
const delaySeconds = delayMs / 1000; // convert ms to s
43+
44+
return enqueueWithDelay(boundHandler, delaySeconds);
45+
}
46+
47+
/**
48+
* Implement the `clearTimeout` global function
49+
* @see https://developer.mozilla.org/en-US/docs/Web/API/clearTimeout and
50+
* @see https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-cleartimeout
51+
* @param {number} timeoutId
52+
* @return {void}
53+
*/
54+
function clearTimeout(timeoutId)
55+
{
56+
return cancelByTimeoutId(timeoutId);
57+
}
58+
59+
if (!globalThis.setTimeout)
60+
globalThis.setTimeout = setTimeout;
61+
if (!globalThis.clearTimeout)
62+
globalThis.clearTimeout = clearTimeout;
63+
64+
exports.setTimeout = setTimeout;
65+
exports.clearTimeout = clearTimeout;

‎python/pythonmonkey/global.d.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ declare var console: import("console").Console;
5151
declare var atob: typeof import("base64").atob;
5252
declare var btoa: typeof import("base64").btoa;
5353

54+
// Expose `setTimeout`/`clearTimeout` APIs
55+
declare var setTimeout: typeof import("timers").setTimeout;
56+
declare var clearTimeout: typeof import("timers").clearTimeout;
57+
5458
// Keep this in sync with both https://hg.mozilla.org/releases/mozilla-esr102/file/a03fde6/js/public/Promise.h#l331
5559
// and https://github.com/nodejs/node/blob/v20.2.0/deps/v8/include/v8-promise.h#L30
5660
declare enum PromiseState { Pending = 0, Fulfilled = 1, Rejected = 2 }

‎src/internalBinding.cc‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ JSObject *createInternalBindingsForNamespace(JSContext *cx, JSFunctionSpec *meth
2323
JSObject *getInternalBindingsByNamespace(JSContext *cx, JSLinearString *namespaceStr) {
2424
if (JS_LinearStringEqualsLiteral(namespaceStr, "utils")) {
2525
return createInternalBindingsForNamespace(cx, InternalBinding::utils);
26+
} else if (JS_LinearStringEqualsLiteral(namespaceStr, "timers")) {
27+
return createInternalBindingsForNamespace(cx, InternalBinding::timers);
2628
} else { // not found
2729
return nullptr;
2830
}

‎src/internalBinding/timers.cc‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
/**
2+
* @file timers.cc
3+
* @author Tom Tang ([email protected])
4+
* @brief Implement functions in `internalBinding("timers")`
5+
*/
6+
7+
#include "include/internalBinding.hh"
8+
#include "include/pyTypeFactory.hh"
9+
#include "include/PyEventLoop.hh"
10+
11+
#include <jsapi.h>
12+
13+
/**
14+
* See function declarations in python/pythonmonkey/builtin_modules/internal-binding.d.ts :
15+
* `declare function internalBinding(namespace: "timers")`
16+
*/
17+
18+
static bool enqueueWithDelay(JSContext *cx, unsigned argc, JS::Value *vp) {
19+
JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
20+
JS::HandleValue jobArgVal = args.get(0);
21+
double delaySeconds = args.get(1).toNumber();
22+
23+
// Convert to a Python function
24+
// FIXME (Tom Tang): memory leak, not free-ed
25+
JS::RootedObject *thisv = new JS::RootedObject(cx, nullptr);
26+
JS::RootedValue *jobArg = new JS::RootedValue(cx, jobArgVal);
27+
PyObject *job = pyTypeFactory(cx, thisv, jobArg)->getPyObject();
28+
29+
// Schedule job to the running Python event-loop
30+
PyEventLoop loop = PyEventLoop::getRunningLoop();
31+
if (!loop.initialized()) return false;
32+
PyEventLoop::AsyncHandle handle = loop.enqueueWithDelay(job, delaySeconds);
33+
34+
// Return the `timeoutID` to use in `clearTimeout`
35+
args.rval().setDouble((double)PyEventLoop::AsyncHandle::getUniqueId(std::move(handle)));
36+
return true;
37+
}
38+
39+
// TODO (Tom Tang): move argument checks to the JavaScript side
40+
static bool cancelByTimeoutId(JSContext *cx, unsigned argc, JS::Value *vp) {
41+
using AsyncHandle = PyEventLoop::AsyncHandle;
42+
JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
43+
JS::HandleValue timeoutIdArg = args.get(0);
44+
45+
args.rval().setUndefined();
46+
47+
// silently does nothing when an invalid timeoutID is passed in
48+
if (!timeoutIdArg.isInt32()) {
49+
return true;
50+
}
51+
52+
// Retrieve the AsyncHandle by `timeoutID`
53+
int32_t timeoutID = timeoutIdArg.toInt32();
54+
AsyncHandle *handle = AsyncHandle::fromId((uint32_t)timeoutID);
55+
if (!handle) return true; // does nothing on invalid timeoutID
56+
57+
// Cancel this job on Python event-loop
58+
handle->cancel();
59+
60+
return true;
61+
}
62+
63+
JSFunctionSpec InternalBinding::timers[] = {
64+
JS_FN("enqueueWithDelay", enqueueWithDelay, /* nargs */ 2, 0),
65+
JS_FN("cancelByTimeoutId", cancelByTimeoutId, 1, 0),
66+
JS_FS_END
67+
};

‎src/modules/pythonmonkey/pythonmonkey.cc‎

Lines changed: 0 additions & 89 deletions
Original file line numberDiff line numberDiff line change
@@ -305,90 +305,6 @@ struct PyModuleDef pythonmonkey =
305305

306306
PyObject *SpiderMonkeyError = NULL;
307307

308-
// Implement the `setTimeout` global function
309-
// https://developer.mozilla.org/en-US/docs/Web/API/setTimeout
310-
// https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-settimeout
311-
static bool setTimeout(JSContext *cx, unsigned argc, JS::Value *vp) {
312-
JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
313-
314-
// Ensure the first parameter is a function
315-
// We don't support passing a `code` string to `setTimeout` (yet)
316-
JS::HandleValue jobArgVal = args.get(0);
317-
bool jobArgIsFunction = jobArgVal.isObject() && js::IsFunctionObject(&jobArgVal.toObject());
318-
if (!jobArgIsFunction) {
319-
JS_ReportErrorNumberASCII(cx, nullptr, nullptr, JSErrNum::JSMSG_NOT_FUNCTION, "The first parameter to setTimeout()");
320-
return false;
321-
}
322-
323-
// Get the function to be executed
324-
// FIXME (Tom Tang): memory leak, not free-ed
325-
JS::RootedObject *thisv = new JS::RootedObject(cx, JS::GetNonCCWObjectGlobal(&args.callee())); // HTML spec requires `thisArg` to be the global object
326-
JS::RootedValue *jobArg = new JS::RootedValue(cx, jobArgVal);
327-
// `setTimeout` allows passing additional arguments to the callback, as spec-ed
328-
if (args.length() > 2) { // having additional arguments
329-
// Wrap the job function into a bound function with the given additional arguments
330-
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/bind
331-
JS::RootedVector<JS::Value> bindArgs(cx);
332-
(void)bindArgs.append(JS::ObjectValue(**thisv)); /** @todo XXXwg handle return value */
333-
for (size_t j = 2; j < args.length(); j++) {
334-
(void)bindArgs.append(args[j]); /** @todo XXXwg handle return value */
335-
}
336-
JS::RootedObject jobArgObj = JS::RootedObject(cx, &jobArgVal.toObject());
337-
JS_CallFunctionName(cx, jobArgObj, "bind", JS::HandleValueArray(bindArgs), jobArg); // jobArg = jobArg.bind(thisv, ...bindArgs)
338-
}
339-
// Convert to a Python function
340-
PyObject *job = pyTypeFactory(cx, thisv, jobArg)->getPyObject();
341-
342-
// Get the delay time
343-
// JS `setTimeout` takes milliseconds, but Python takes seconds
344-
double delayMs = 0; // use value of 0 if the delay parameter is omitted
345-
if (args.hasDefined(1)) { JS::ToNumber(cx, args[1], &delayMs); } // implicitly do type coercion to a `number`
346-
if (delayMs < 0) { delayMs = 0; } // as spec-ed
347-
double delaySeconds = delayMs / 1000; // convert ms to s
348-
349-
// Schedule job to the running Python event-loop
350-
PyEventLoop loop = PyEventLoop::getRunningLoop();
351-
if (!loop.initialized()) return false;
352-
PyEventLoop::AsyncHandle handle = loop.enqueueWithDelay(job, delaySeconds);
353-
354-
// Return the `timeoutID` to use in `clearTimeout`
355-
args.rval().setDouble((double)PyEventLoop::AsyncHandle::getUniqueId(std::move(handle)));
356-
357-
return true;
358-
}
359-
360-
// Implement the `clearTimeout` global function
361-
// https://developer.mozilla.org/en-US/docs/Web/API/clearTimeout
362-
// https://html.spec.whatwg.org/multipage/timers-and-user-prompts.html#dom-cleartimeout
363-
static bool clearTimeout(JSContext *cx, unsigned argc, JS::Value *vp) {
364-
using AsyncHandle = PyEventLoop::AsyncHandle;
365-
JS::CallArgs args = JS::CallArgsFromVp(argc, vp);
366-
JS::HandleValue timeoutIdArg = args.get(0);
367-
368-
args.rval().setUndefined();
369-
370-
// silently does nothing when an invalid timeoutID is passed in
371-
if (!timeoutIdArg.isInt32()) {
372-
return true;
373-
}
374-
375-
// Retrieve the AsyncHandle by `timeoutID`
376-
int32_t timeoutID = timeoutIdArg.toInt32();
377-
AsyncHandle *handle = AsyncHandle::fromId((uint32_t)timeoutID);
378-
if (!handle) return true; // does nothing on invalid timeoutID
379-
380-
// Cancel this job on Python event-loop
381-
handle->cancel();
382-
383-
return true;
384-
}
385-
386-
static JSFunctionSpec jsGlobalFunctions[] = {
387-
JS_FN("setTimeout", setTimeout, /* nargs */ 2, 0),
388-
JS_FN("clearTimeout", clearTimeout, 1, 0),
389-
JS_FS_END
390-
};
391-
392308
PyMODINIT_FUNC PyInit_pythonmonkey(void)
393309
{
394310
if (!PyDateTimeAPI) { PyDateTime_IMPORT; }
@@ -440,11 +356,6 @@ PyMODINIT_FUNC PyInit_pythonmonkey(void)
440356

441357
autoRealm = new JSAutoRealm(GLOBAL_CX, *global);
442358

443-
if (!JS_DefineFunctions(GLOBAL_CX, *global, jsGlobalFunctions)) {
444-
PyErr_SetString(SpiderMonkeyError, "Spidermonkey could not define global functions.");
445-
return NULL;
446-
}
447-
448359
JS_SetGCCallback(GLOBAL_CX, handleSharedPythonMonkeyMemory, NULL);
449360
JS_DefineProperty(GLOBAL_CX, *global, "debuggerGlobal", debuggerGlobal, JSPROP_READONLY);
450361

‎tests/python/test_event_loop.py‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,15 +48,13 @@ def to_raise(msg):
4848
pm.eval("clearTimeout(undefined)")
4949
pm.eval("clearTimeout()")
5050

51-
# should throw a TypeError when the first parameter to `setTimeout` is not a function
52-
with pytest.raises(pm.SpiderMonkeyError, match="TypeError: The first parameter to setTimeout\\(\\) is not a function"):
53-
pm.eval("setTimeout()")
54-
with pytest.raises(pm.SpiderMonkeyError, match="TypeError: The first parameter to setTimeout\\(\\) is not a function"):
55-
pm.eval("setTimeout(undefined)")
56-
with pytest.raises(pm.SpiderMonkeyError, match="TypeError: The first parameter to setTimeout\\(\\) is not a function"):
57-
pm.eval("setTimeout(1)")
58-
with pytest.raises(pm.SpiderMonkeyError, match="TypeError: The first parameter to setTimeout\\(\\) is not a function"):
59-
pm.eval("setTimeout('a', 100)")
51+
# passing a `code` string to `setTimeout` as the callback function
52+
assert "code string" == await pm.eval("""
53+
new Promise((resolve) => {
54+
globalThis._resolve = resolve
55+
setTimeout("globalThis._resolve('code string'); delete globalThis._resolve", 100)
56+
})
57+
""")
6058

6159
# making sure the async_fn is run
6260
return True

0 commit comments

Comments
 (0)