Resumable function invocation #110
Conversation
…start/resume_invocation
|
Added a simple test. And for resumable calls, I moved passing I think this PR is ready for review now. |
src/runner.rs
Outdated
|
|
||
| pub fn start_execution<'a, E: Externals + 'a>(&mut self, externals: &'a mut E) -> Result<Option<RuntimeValue>, Trap> { | ||
| // Ensure that the VM has not been executed. | ||
| assert!(self.state == InterpreterState::Initialized); |
There was a problem hiding this comment.
Not sure about all those asserts
This all supposed to be called in places with zero tolerance to panics
Can we at least write why this assert (and all others) will always pass?
There was a problem hiding this comment.
The public interface will not panic. In func mod's FuncInvocation::start_execution, we check whether the state is Initialized first before calling Interpreter::start_execution. But if there's an internal incorrect usage in our code, then panic do happen. Do we prefer to return additional errors here or is this panic okay?
src/runner.rs
Outdated
| }); | ||
|
|
||
| // Ensure that stack is empty after the execution. | ||
| assert!(self.value_stack.len() == 0); |
There was a problem hiding this comment.
What about this assert?
How is that guaranteed that there are no more values?
There was a problem hiding this comment.
sorry my bad. Yeah we do, it’s guaranteed by the validation properties. I should have add a comment
src/tests/host.rs
Outdated
| .expect("Failed to instantiate module") | ||
| .assert_no_start(); | ||
|
|
||
| let func_instance = match instance.export_by_name("test").unwrap() { |
There was a problem hiding this comment.
You can do the same thing a little bit cleaner (IMO) with ExternVal::as_func()
| } | ||
| } | ||
|
|
||
| /// Resume an execution if a previous trap of Host kind happened. |
There was a problem hiding this comment.
Can we add some more documentation here?
E.g. what happens if the user provides return_val of mismatched type?
src/func.rs
Outdated
| /// Trap happened. | ||
| Trap(Trap), | ||
| /// The invocation is not resumable. | ||
| NotResumable, |
There was a problem hiding this comment.
Could we expand a little bit more on why the invocation can be not resumable?
src/func.rs
Outdated
|
|
||
| /// Invoke the function, get a resumable handle. This handle can then be used to actually start the execution. If a | ||
| /// Host trap happens, caller can use `resume_execution` to feed the expected return value back in, and then | ||
| /// continue the execution. |
There was a problem hiding this comment.
I think it is a good idea to add a note that this is an experimental API. Also it might be a good idea to also warn the user that they might not be able to find anything like this API on other execution engines.
…umable_value_type`
Implements #85. If a trap of kind Host happens, we allow the caller to feed back in another return value for that host function, and then continue the execution.
Interpreterstruct itself. We drop the interpreter altogether for normal executions anyway.resume_execution, which takes the feeded-back return value, will continue the execution with the loop.invoke_resumableis added toFuncInstance. This function returns a handleFuncInvocation. The caller can then use this to start or resume an execution.