-
Notifications
You must be signed in to change notification settings - Fork 62
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
Segfault when evaluating modules that throws an error #198
Comments
I think i found the issue and it might be a complicated one to solve. I'm trying to implement sync imports (similar to require)
I don't know if this is even a bug in quickjs (that we shouldn't try to free the module currently under evaluation) There is no workaround for now. The simplest thing would be to patch quickjs to expose Whats your take @DelSkayn? :) |
Opened up https://github.com/DelSkayn/rquickjs/pull/199/files that will solve the issue. It takes a different approach and patches quickjs to allow for |
Hmm, I didn't think of evaluating modules while evaluating another module when designing the Module API. This is definitely a problem. Working around quickjs freeing all modules which aren't properly evaluated is difficult. As the library is now I think you are onto something using the builtin dynamic But quickjs must support dynamic imports and looking at the code it solves this by making dynamic imports which error only free all unresolved modules. If we patch quickjs such that we can evaluated modules like a dynamic import does it would fix this issue and we can make holding onto unevaluated modules safe again at the cost of some modules not being freed when an error happens, which I think is an acceptable tradeoff. |
The changes to the library will probably just be changing freeing modules in import code from freeing all unevaluated modules to all unresolved modules and adding a unsafe function to We could also add a safe version of that function to |
I've opened up a PR for the second approach which basically uses the same code that |
I want to keep this issue open for now since That must first be resolved before this issue can be closed. |
When trying to either define or evaluate a model that throws an error, quickjs segfauls.
I see in the docs that holding on to a unevaluated module is unsupported because quickjs will free it on craches. But i'm not doing that. Check this reproducable:
main.rs
index.js
hello.js
It doesn't matter if i throw or if its a type error. Only errors that are handled correctly are parsing errors.
Callstack
frame #0: 0x0000000100769ce7 llrt`list_del(el=0x000004f560166e10) at list.h:75:16 thread backtrace * thread #1, name = 'main', queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x8) * frame #0: 0x0000000100769ce7 llrt`list_del(el=0x000004f560166e10) at list.h:75:16 frame #1: 0x000000010077064c llrt`__JS_FreeValueRT(rt=0x000004f5601d0308, v=JSValue @ 0x00007ff7bfef8ca0) at quickjs.c:5513:17 frame #2: 0x0000000100770a99 llrt`__JS_FreeValue(ctx=0x000004f5601b0608, v=JSValue @ 0x00007ff7bfef8cd0) at quickjs.c:5555:5 frame #3: 0x0000000100769d5e llrt`JS_FreeValue(ctx=0x000004f5601b0608, v=JSValue @ 0x00007ff7bfef8d00) at quickjs.h:648:13 frame #4: 0x00000001007723e2 llrt`JS_CallFree(ctx=0x000004f5601b0608, func_obj=JSValue @ 0x00007ff7bfef8d70, this_obj=JSValue @ 0x00007ff7bfef8d60, argc=0, argv=0x0000000000000000) at quickjs.c:18732:5 frame #5: 0x00000001007b8daa llrt`js_evaluate_module(ctx=0x000004f5601b0608, m=0x000004f560270b48) at quickjs.c:28375:19 frame #6: 0x000000010078ea72 llrt`JS_EvalFunctionInternal(ctx=0x000004f5601b0608, fun_obj=JSValue @ 0x00007ff7bfef8f20, this_obj=JSValue @ 0x00007ff7bfef8f10, var_refs=0x0000000000000000, sf=0x0000000000000000) at quickjs.c:33574:19 frame #7: 0x000000010078e942 llrt`JS_EvalFunction(ctx=0x000004f5601b0608, fun_obj=JSValue @ 0x00007ff7bfef8f60) at quickjs.c:33589:12 frame #8: 0x0000000100731c4d llrt`rquickjs_core::value::module::Module::eval(self=0x00007ff7bfef9200) at module.rs:743:23 frame #9: 0x000000010013b416 llrt`llrt::vm::Vm::load_module_from_source(ctx=rquickjs_core::context::ctx::Ctx @ 0x00007ff7bfef91f8, name="index.mjs", source=") at vm.rs:409:24 frame #10: 0x000000010013bf2e llrt`llrt::vm::Vm::load_module(ctx=0x00007ff7bfef9de0, filename="index.mjs") at vm.rs:435:9 frame #11: 0x000000010005b5f3 llrt`llrt::vm::Vm::run_module::{{closure}}::{{closure}}(ctx=rquickjs_core::context::ctx::Ctx @ 0x00007ff7bfef9de0) at vm.rs:440:24 frame #12: 0x000000010005c1c8 llrt`llrt::vm::Vm::run_and_handle_exceptions::{{closure}}::{{closure}}(ctx=rquickjs_core::context::ctx::Ctx @ 0x00007ff7bfef9f60) at vm.rs:451:13 frame #13: 0x00000001002423aa llrt`rquickjs_core::context::async::AsyncContext::with::{{closure}}((null)=0x00007ff7bfefcb30) at async.rs:250:19 frame #14: 0x000000010005b925 llrt`llrt::vm::Vm::run_and_handle_exceptions::{{closure}}((null)=0x00007ff7bfefcb30) at vm.rs:455:10 frame #15: 0x000000010005b4c5 llrt`llrt::vm::Vm::run_module::{{closure}}((null)=0x00007ff7bfefcb30) at vm.rs:443:10 frame #16: 0x00000001000d6348 llrt`llrt::start_cli::{{closure}}((null)=0x00007ff7bfefcb30) at main.rs:123:44 frame #17: 0x00000001000db5b7 llrt`llrt::main::{{closure}}((null)=0x00007ff7bfefcb30) at main.rs:70:28 frame #18: 0x000000010030224e llrt`tokio::runtime::park::CachedParkThread::block_on::{{closure}} at park.rs:282:63 frame #19: 0x00000001003020ed llrt`tokio::runtime::park::CachedParkThread::block_on at coop.rs:107:5 frame #20: 0x000000010030207c llrt`tokio::runtime::park::CachedParkThread::block_on [inlined] tokio::runtime::coop::budget(f=tokio::runtime::park::{impl#4}::block_on::{closure_env#0} @ 0x00007ff7bfefcf50) at coop.rs:73:5 frame #21: 0x0000000100301ff8 llrt`tokio::runtime::park::CachedParkThread::block_on(self=0x00007ff7bfefcfe8, f=llrt::main::{async_block_env#0}::Unresumed @ 0x00007ff7bfefcff0) at park.rs:282:31 frame #22: 0x0000000100156db6 llrt`tokio::runtime::context::blocking::BlockingRegionGuard::block_on(self=0x00007ff7bfefd808, f=llrt::main::{async_block_env#0}::Unresumed @ 0x00007ff7bfefd3e8) at blocking.rs:66:9 frame #23: 0x000000010037d2e5 llrt`tokio::runtime::scheduler::multi_thread::MultiThread::block_on::{{closure}}(blocking=0x00007ff7bfefd808) at mod.rs:87:13 frame #24: 0x0000000100171614 llrt`tokio::runtime::context::runtime::enter_runtime(handle=0x00007ff7bfefebd0, allow_block_in_place=true, f=tokio::runtime::scheduler::multi_thread::{impl#0}::block_on::{closure_env#0} @ 0x00007ff7bfefdc40) at runtime.rs:65:16 frame #25: 0x000000010037d282 llrt`tokio::runtime::scheduler::multi_thread::MultiThread::block_on(self=0x00007ff7bfefeba8, handle=0x00007ff7bfefebd0, future=) at mod.rs:86:9 frame #26: 0x0000000100135276 llrt`tokio::runtime::runtime::Runtime::block_on(self=0x00007ff7bfefeba0, future=llrt::main::{async_block_env#0}::Unresumed @ 0x00007ff7bfefed08) at runtime.rs:349:45 frame #27: 0x0000000100243520 llrt`llrt::main at main.rs:74:5 frame #28: 0x000000010029d1de llrt`core::ops::function::FnOnce::call_once((null)=0x0000000100243430, (null)=()) at function.rs:250:5 frame #29: 0x00000001002934c1 llrt`std::sys_common::backtrace::__rust_begin_short_backtrace(f=0x0000000100243430) at backtrace.rs:135:18 frame #30: 0x0000000100288084 llrt`std::rt::lang_start::{{closure}} at rt.rs:166:18 frame #31: 0x00000001008457fa llrt`std::rt::lang_start_internal [inlined] core::ops::function::impls:: for &F>::call_once at function.rs:284:13 [opt] frame #32: 0x00000001008457ed llrt`std::rt::lang_start_internal [inlined] std::panicking::try::do_call at panicking.rs:500:40 [opt] frame #33: 0x00000001008457ed llrt`std::rt::lang_start_internal [inlined] std::panicking::try at panicking.rs:464:19 [opt] frame #34: 0x00000001008457ed llrt`std::rt::lang_start_internal [inlined] std::panic::catch_unwind at panic.rs:142:14 [opt] frame #35: 0x00000001008457ed llrt`std::rt::lang_start_internal [inlined] std::rt::lang_start_internal::{{closure}} at rt.rs:148:48 [opt] frame #36: 0x00000001008457ed llrt`std::rt::lang_start_internal [inlined] std::panicking::try::do_call at panicking.rs:500:40 [opt] frame #37: 0x00000001008457ed llrt`std::rt::lang_start_internal [inlined] std::panicking::try at panicking.rs:464:19 [opt] frame #38: 0x00000001008457ed llrt`std::rt::lang_start_internal [inlined] std::panic::catch_unwind at panic.rs:142:14 [opt] frame #39: 0x00000001008457ed llrt`std::rt::lang_start_internal at rt.rs:148:20 [opt] frame #40: 0x0000000100288057 llrt`std::rt::lang_start(main=0x0000000100243430, argc=1, argv=0x00007ff7bfeff398, sigpipe=0) at rt.rs:165:17
The text was updated successfully, but these errors were encountered: