Stream: git-wasmtime

Topic: wasmtime / PR #7095 component HostFunc: add call hooks fo...

Wasmtime GitHub notifications bot (Sep 27 2023 at 04:06):

pchickey opened PR #7095 from bytecodealliance:pch/component_call_hooks to bytecodealliance:main:

<!--
Please make sure you include the following information:

• If this work has been discussed elsewhere, please include a link to that
  conversation. If it was discussed in an issue, just mention "issue #...".

• Explain why this change is needed. If the details are in an issue already,
  this can be brief.

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

Wasmtime GitHub notifications bot (Sep 27 2023 at 04:06):

pchickey requested alexcrichton for a review on PR #7095.

Wasmtime GitHub notifications bot (Sep 27 2023 at 04:08):

pchickey edited PR #7095:

This needs more fleshing out and testing.

Without this patch, @acfoltzer reported an unexpected trace when calling into a component:

#    7.148 p.v0 XQD| 2023-09-27T00:39:46.963691Z TRACE request{id=00000000-0000-0000-0000-000000000000}:service{sid=enabled}: xqd::guest::component: call_hook=CallingWasm\n
#    7.148 p.v0 XQD| 2023-09-27T00:39:46.964430Z TRACE request{id=00000000-0000-0000-0000-000000000000}:service{sid=enabled}: xqd::guest::component: call_hook=CallingWasm\n
#    7.148 p.v0 XQD| 2023-09-27T00:39:46.964485Z TRACE request{id=00000000-0000-0000-0000-000000000000}:service{sid=enabled}: xqd::guest::component: call_hook=ReturningFromWasm\n
#    7.148 p.v0 XQD| 2023-09-27T00:39:46.964932Z TRACE request{id=00000000-0000-0000-0000-000000000000}:service{sid=enabled}: xqd::guest::component: call_hook=ReturningFromWasm\n

Wasmtime GitHub notifications bot (Sep 27 2023 at 14:31):

alexcrichton submitted PR review:

Looks good!

I know there's a test for call hooks which verifies the transitions are all expected, and it might be nice to enable that for all component model tests via a debug_assert! in the store, but that's a bit of a refactoring to take on that I don't think is worth it just yet. The realloc bits won't be covered by these tests but those are a bit more difficult too to cover.

Wasmtime GitHub notifications bot (Sep 27 2023 at 14:31):

alexcrichton submitted PR review:

Looks good!

I know there's a test for call hooks which verifies the transitions are all expected, and it might be nice to enable that for all component model tests via a debug_assert! in the store, but that's a bit of a refactoring to take on that I don't think is worth it just yet. The realloc bits won't be covered by these tests but those are a bit more difficult too to cover.

Wasmtime GitHub notifications bot (Sep 27 2023 at 14:31):

alexcrichton created PR review comment:

The error semantics here are a bit subtle I think -- to match core wasm the ReturningFromHost hook should be called no matter what regardelss of whether closure returned an error or not.

Additionally the ReturningFromHost hook should be delayed to the end of the function because lowering may invoke realloc in the guest which is another instance of entering wasm and I think ReturningFromHost followed by CallWasm might be a bit unexpected.

Wasmtime GitHub notifications bot (Mar 07 2024 at 23:08):

pchickey closed without merge PR #7095.

Wasmtime GitHub notifications bot (Mar 07 2024 at 23:08):

pchickey commented on PR #7095:

I forgot about this and I don't really think its critical, we can let someone re-open it if they need to.

Last updated: Oct 23 2024 at 20:03 UTC