Allow finer control of start function on instantiation #4072
Conversation
sjamesr
requested review from
loganek,
lum1n0us,
no1wudi,
TianlongLiang,
wenyongh,
xujuntwt95329 and
yamt
as code owners
sjamesr
force-pushed
the
instantiate_timeout
branch
2 times, most recently
from
ee6002e to
cd6cd60
Compare
Tries to address bytecodealliance#4047. Before this change, wasm_runtime_instantiate would execute the start function unconditionally, correctly implementing the spec. However, if there is an infinite loop in the start function, there was no way to interrupt execution. This change introduces a parameter to InstantiationArgs indicating whether the start function should be immediately invoked. If not, wasm_runtime_instantiate returns a module instance that is initialized except for the start function. This change adds a wasm_runtime_instantiate_run_start_func function which performs this last instantiation step. The user may prepare a timeout in advance of calling this to guard against expensive/infinite loops. This change also demonstrates a possible technique for doing this in iwasm.c.
sjamesr
force-pushed
the
instantiate_timeout
branch
from
cd6cd60 to
b8e57c6
Compare
|
I'm pretty confident that this is a workable patch. However, from my understanding, it seems more like a workaround rather than a general solution. Ideally, the termination should originate from the runtime itself, whether it's the interpreter, JIT, or AOT, rather than from iwasm. I'm considering a solution that's more intrinsic to the runtime, such as using a counter (which might be less precise) to estimate the remaining fuel or time left after each opcode or |
please note that the timeout thing is NOT the sole motivation of wasm_runtime_terminate. fuel-like approaches would work for my use cases only if it allows to resume terminated instance. it likely makes host functions very tricky. (i have implemented a similar approach in the other runitme: https://github.com/yamt/toywasm/blob/master/doc/check_interrupt.md. my gut feeling is that it isn't suitable for wamr.) |
| @@ -2333,6 +2333,25 @@ wasm_set_running_mode(WASMModuleInstance *module_inst, RunningMode running_mode) | |||
| return set_running_mode(module_inst, running_mode, false); | |||
| } | |||
|
|
|||
| bool | |||
| wasm_runtime_instantiate_run_start_func(wasm_module_inst_t module_inst, | |||
There was a problem hiding this comment.
this name is a bit confusing because it doesn't run wasi's entry point, _start.
how about wasm_runtime_execute_instantiate_functions?
| is_sub_inst, exec_env)) { | ||
| set_error_buf(error_buf, error_buf_size, | ||
| ((WASMModuleInstance *)module_inst)->cur_exception); | ||
| wasm_runtime_deinstantiate(module_inst); |
There was a problem hiding this comment.
as wasm_runtime_instantiate succeeded at this point,
isn't it simpler to leave wasm_runtime_deinstantiate to api users?
Tries to address #4047.
Before this change, wasm_runtime_instantiate would execute the start function unconditionally, correctly implementing the spec. However, if there is an infinite loop in the start function, there was no way to interrupt execution.
This change introduces a parameter to InstantiationArgs indicating whether the start function should be immediately invoked. If not, wasm_runtime_instantiate returns a module instance that is initialized except for the start function.
This change adds a wasm_runtime_instantiate_run_start_func function which performs this last instantiation step. The user may prepare a timeout in advance of calling this to guard against expensive/infinite loops.
This change also demonstrates a possible technique for doing this in iwasm.c.