Skip to content

Migrate wasmtime to wasmtime-cpp-api#503

Merged
leonm1 merged 4 commits into
proxy-wasm:mainfrom
leonm1:api/wasmtime
May 20, 2026
Merged

Migrate wasmtime to wasmtime-cpp-api#503
leonm1 merged 4 commits into
proxy-wasm:mainfrom
leonm1:api/wasmtime

Conversation

@leonm1
Copy link
Copy Markdown
Contributor

@leonm1 leonm1 commented Mar 2, 2026

Hides many of the implementation details of the wasm-c-api.

Required for implementing execution limits, as those are under the wasmtime_ c api, which is fully incompatible with stores created using the wasm_ c api. Execution limits implemented in #510.

Built off of #502.

@leonm1 leonm1 force-pushed the api/wasmtime branch 3 times, most recently from 40e3040 to 68ebe57 Compare March 2, 2026 15:34
Copy link
Copy Markdown
Member

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implements memory limiting and execution termination for wasmtime.

Could you split this off into a separate PR? I think it's just a few lines, so it should be doable, and I think those changes warrant a proper review instead of being bundled in this mass refactor. Thanks!

@leonm1 leonm1 force-pushed the api/wasmtime branch 2 times, most recently from 680dc57 to 5982aca Compare March 2, 2026 20:15
@leonm1 leonm1 force-pushed the api/wasmtime branch 2 times, most recently from 356f7e2 to dbc1f3e Compare March 12, 2026 02:36
@leonm1 leonm1 requested a review from PiotrSikora March 12, 2026 03:13
Comment thread BUILD
Copy link
Copy Markdown
Member

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uhm, looking at the passed arguments in failed tests, it seems like some endianness conversions are broken.

@leonm1 leonm1 marked this pull request as ready for review March 13, 2026 23:19
@leonm1 leonm1 requested a review from PiotrSikora March 13, 2026 23:19
Comment thread include/proxy-wasm/word.h
Comment thread src/wasmtime/wasmtime.cc Outdated
Copy link
Copy Markdown
Contributor

@martijneken martijneken left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for correctness based on testing added in #525 and also a rigorous external e2e test suite. Please address style concerns when possible.

Comment thread bazel/external/wasmtime.BUILD
Comment thread src/wasmtime/wasmtime.cc Outdated
Comment thread src/wasmtime/wasmtime.cc
Comment thread bazel/external/wasmtime.BUILD
Comment thread src/wasmtime/wasmtime.cc
leonm1 added 4 commits May 19, 2026 10:54
Hides many of the implementation details of the wasm-c-api.

Note: adds `wat` feature to wasmtime c headers to fix the following build error, but note that `wat` support is not enabled in the wasmtime build, this just adds the headers to allow the cpp api to compile.

```
external/com_github_bytecodealliance_wasmtime/crates/c-api/include/wasmtime/module.hh:39:17: error: use of undeclared identifier 'wat2wasm'
   39 |     auto wasm = wat2wasm(wat);
      |                 ^
1 error generated.
```

Signed-off-by: Matt Leon <mattleon@google.com>
Signed-off-by: Matt Leon <mattleon@google.com>
Signed-off-by: Matt Leon <mattleon@google.com>
Signed-off-by: Matt Leon <mattleon@google.com>
Copy link
Copy Markdown
Member

@PiotrSikora PiotrSikora left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@leonm1 leonm1 merged commit a7cfd87 into proxy-wasm:main May 20, 2026
51 of 52 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants