Stream: git-wasmtime

Topic: wasmtime / issue #3313 native hooks do some nonsense if t...

Wasmtime GitHub notifications bot (Sep 08 2021 at 02:13):

github-actions[bot] commented on issue #3313:

Subscribe to Label Action

cc @peterhuene

<details>
This issue or pull request has been labeled: "wasmtime:api"

Thus the following users have been cc'd because of the following labels:

• peterhuene: wasmtime:api

To subscribe or unsubscribe from this label, edit the <code>.github/subscribe-to-label.json</code> configuration file.

Learn more.
</details>

Wasmtime GitHub notifications bot (Sep 08 2021 at 15:48):

alexcrichton commented on issue #3313:

Thinking about this, the cases we're dealing with are:

1. When "exiting native" by calling wasm, if that traps we never call "entering native" or anything else
2. When "entering native" by entering a host function, if that traps we currently don't call "exiting native" as we exit with the trap.
3. When the host function returns a normal trap in the implementation, it looks like Func::new doesn't call "exiting native" correctly, but Func::wrap does.
4. When "exiting native" by returning from a host function, if that traps we still call "entering native" on the other side.
5. When "entering native" by returning from wasm execution, if that traps we don't call anything else and just bail out.

Ideally I want the hooks installed into the Store to be simple and we track most other things for you in Wasmtime itself. I think the best way we can interpret this is with guidelines of:

• If a trap happen when "entering" a state, then the state isn't changed. In this case the "entering" isn't defined by the callback names today, but rather entering into wasm by calling it or entering native by calling a host function.
• If a trap happens when "exiting" a state, then the state is still changed. (e.g. returning from a host call or returning from a wasm call).

To model this we probably want to remove the enter/exit callbacks and instead have something like one callback which receives an enum describing what's happening. The enum would have 4 values of 1/2/4/5 above, and perhaps other options as well like "I'm exiting the host from a hostcall but a trap was returned there anyway". With that sort of modeling the behavior we need to update is:

1. No change. By failing the "enter wasm" state you remain in "native" state, so no future callbacks necessary.
2. No change. By failing the "enter native" state we remain in "wasm", so no "exiting native" is necessary.
3. Needs a bugfix in Func::new
4. No change. By failing the "exit host" state we still force entry into the "wasm" state, which means it's correct.
5. No change. By failing the "exit wasm" state we're still forced back into the native state.

Does that make sense? I had to think about this for awhile... Basically I think we may want to change the interface from two callbacks to one-with-arguments where the arguments describe what's happening?

Wasmtime GitHub notifications bot (Sep 08 2021 at 17:18):

pchickey commented on issue #3313:

Yes, I really like the design change to one-with-arguments: the argument to describing whether the state transition is a call or a return fixes another ambiguity.

I also will change the function to take a Caller, instead of the state T.

Wasmtime GitHub notifications bot (Sep 08 2021 at 17:51):

alexcrichton commented on issue #3313:

I don't think we can pass a Caller<'_, T> without extra work to be done because the callback itself lives within the Caller, but is there a reason you think that the callback may need access to the whole store?

Wasmtime GitHub notifications bot (Sep 08 2021 at 18:31):

pchickey commented on issue #3313:

You're right, we dont have Caller available in all of these contexts. I thought it would be interesting to be able to inspect/modify fuel, but it doesn't make sense for this mechanism

Last updated: Nov 22 2024 at 16:03 UTC