-
Notifications
You must be signed in to change notification settings - Fork 101
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Feature Request: Interrupt handler (or equivalent) for contexts (not just runtimes) #84
Comments
Commenting on my own Issue to add a possible bug and alter my feature request a bit accordingly. Essentially, after making this ticket I went ahead and put in the work to make our system use brand new Runtimes for each execution group, but noticed something wasn't working with the interrupt handlers. For some reason, the interrupt handlers set by I can set up timers to periodically call back to JS-land, but unfortunately it appears as though the |
Maybe the interrupt handler needs to be reinstalled after firing once? I don’t have free time right now to debug this issue. |
There are tests in the repo that check the basics of quickjs-emscripten/ts/quickjs.test.ts Line 448 in 04498f9
Can you open a PR with a failing test that reproduces the issue you're experiencing? That would go a long way towards fixing the issue.
It's possible to change QuickJS to pass through the JSContext pointer to the runtime. Here's one way to do it:
After that, the interruptHandler callback you write can consider the context that's being interrupted, so you can implement context-specific behavior. |
Thanks @justjake! I appreciate you taking a look at this. I am working on a minimal repro as we speak. I noticed in the unit tests that the interrupt is only checked to have been called once, which does fit what I've seen in our implementation. The interrupt handler is called once, as soon as the runtime is created, but never again. I suspect the issue is related to our use of promises (perhaps the interrupt handler isn't called while the VM is waiting on a promise to resolve?) but am hoping to determine that with the repo case. Those changes seem doable, so I'll take a stab at a PR in a bit. |
I created a "simple" repo (#90) and it shows what I have been seeing. 0 calls to the interrupt handler when running async code via the async context. Please take a look, and let me know if I am just doing something wrong here. We've got a pretty complex setup in our project and everything works as expected except the interrupts |
36911f0d regexp: fix non greedy quantizers with zero length matches d86aaf0b updated test262.patch adec7343 fixed test of test262 directory d378a9f3 Improve `js_os_exec` (#295) 97be5a32 Add `js_resolve_proxy` (#293) f3f2f427 Add `JS_StrictEq()`, `JS_SameValue()`, and `JS_SameValueZero()` (#264) 6f9d05fd Expose `JS_SetUncatchableError()` (#262) d53aafe0 Add the missing fuzz_common.c (#292) db9dbd0a Add `JS_HasException()` (#265) 6c430131 Add `JS_NewTypedArray()` (#272) 01454caf OSS-Fuzz targets improvements (#267) 0c8fecab Improve class parser (#289) d9c699f5 fix class method with name get (#258) 7a2c6f42 Improve libunicode and libregexp headers (#288) 1402478d Improve unicode table handling (#286) 3b45d155 Fix endianness handling in `js_dataview_getValue` / `js_dataview_setValue` 653b2276 Improve error handling 203fe2d5 Improve `JSON.stringify` ce6b6dca Use more explicit magic values for array methods c0e67c47 Simplify redundant initializers for `JS_NewBool()` 06651314 Fix compilation with -DCONFIG_BIGNUM 65ecb0b0 Improve Date.parse, small fixes 6a89d7c2 Add CI targets, fix test_std.js (#247) ebe7496d Fix build: use LRE_BOOL in libunicode.h (#244) 1a5333bc prevent 0 length allocation in `js_worker_postMessage` e17cb9fc Add github CI tests 06c100c9 Prevent UB on memcpy and floating point conversions 3dd93eb4 fix microbench when microbench.txt is missing (#246) 35b7b3c3 Improve Date.parse 8d64731e Improve Number.prototype.toString for radix other than 10 a78d2cbf Improve repl regexp handling 8180d3dd Improve microbench.js 78db49cf Improve Date.parse 6428ce0c show readable representation of Date objects in repl 27928ce4 Fix Map hash bug b70e7644 Rewrite `set_date_fields` to match the ECMA specification b91a2aec Add C API function JS_GetClassID() 12c91df5 Improve surrogate handling readability 8d932deb Rename regex flag and field utf16 -> unicode 97ae6f39 Add benchmarks target c24a865a Improve run-test262 bbf36d5b Fix big endian serialization 530ba6a6 handle missing test262 gracefully 0a361b7c handle missing test262 gracefully 74bdb496 Improve tests 85fb2cae Fix UB signed integer overflow in js_math_imul 8df43275 Fix UB left shift of negative number 3bb2ca36 Remove unnecessary ssize_t posix-ism c06af876 Improve string concatenation hack 8e21b967 pass node-js command line arguments to microbench 95e0aa05 Reverse e140122202cc24728b394f8f90fa2f4a2d7c397e 1fe04149 Fix test262 error ef4e7b23 Fix compiler warnings 92e339d1 Simplify and clarify URL quoting js_std_urlGet 636c9465 FreeBSD QuickJS Patch (#203) ae6fa8d3 Fix shell injection bug in std.urlGet (#61) 693449e3 add gitignore for build objects (#84) e1401222 Fix sloppy mode arguments uninitialized value use 6dbf01bb Remove unsafe sprintf() and strcat() calls 65350645 Fix undefined behavior (UBSAN) e53d6223 Fix UB in js_dtoa1 fd6e0397 Add UndefinedBehaviorSanitizer support 325ca194 Add MemorySanitizer support git-subtree-dir: vendor/quickjs git-subtree-split: 36911f0d3ab1a4c190a4d5cbe7c2db225a455389
I am working on a feature in our product to allow tracing "time in vm" by code execution, along with the ability to "cancel" running functions if they surpass some limit or if cancellation is manually requested. We currently use a single context and runtime with a singular
setInterruptHandler
to stop long-running executions, but that doesn't provide enough granularity for our usecase.I began the work to split our vm into multiple contexts under a singular runtime to benefit from the configured limits, only to discover the interrupt handler lives at the runtime level. Is there any chance that this method or something similar could be ported down to either the
evalCode
/evalCodeAsync
level, or at least the context?The text was updated successfully, but these errors were encountered: