Skip to content

Fix externref/anyref ownership in C/C++ API#11799

Merged
alexcrichton merged 2 commits intobytecodealliance:mainfrom
alexcrichton:fix-c-api-onwership
Oct 7, 2025
Merged

Fix externref/anyref ownership in C/C++ API#11799
alexcrichton merged 2 commits intobytecodealliance:mainfrom
alexcrichton:fix-c-api-onwership

Conversation

@alexcrichton
Copy link
Member

This commit is a follow-up to #11514 which was discovered through failing tests in the wasmtime-py repository when updating to Wasmtime 37.0.0. Effectively a combination of bugs in the Rust API meant that it wasn't possible to use externref or anyref bindings correctly. The Rust changes in this commit are:

  • wasmtime_val_unroot correctly drops the value now as opposed to effectively being a noop from before (typo of using as_externref vs from_externref).
  • wasmtime_{anyref,externref,val}_t now have a Drop implementation in Rust to correctly drop them if a value in Rust is dropped. This is required to correctly manage memory in the wasmtime_func_{call,new} implementations, for example.
  • wasmtime_{anyref,externref,val}_clone no longer have an unnecessary context parameter.
  • wasmtime_{anyref,externref,val}_unroot no longer have an unnecessary context parameter.

Changes in the C/C++ APIs are:

  • Result::{ok,err}_ref APIs were added in addition to the preexisting rvalue accessors.
  • Loading/storing typed arguments now has an overload for const T& and T&& which behaves differently. Notably transferring ownership for T&& and not for const T&. This means that passing parameters when calling a wasm function uses const T&, but passing results from a host import uses T&&.
  • TypedFunc::call now uses const Params& instead of Params to explicitly specify it doesn't modify the parameters and forces using the const T& store method.
  • Store::gc is now a convenience method for store.context().gc()
  • ExternRef, AnyRef, and Val now have ownership semantics and destructors. This matches the spirit of GC: replace ManuallyRooted with OwnedRooted. #11514 for Rust but models it in C++ as well. This required filling out move/copy constructors/assignments.
  • The explicit ExternRef now takes std::any instead of T.
  • Minor issues related to ownership are fixed in Val bindings.

Valgrind was used to ensure that there were no leaks for the test suite which additionally resulted in a number of *_delete calls being added to tests using the C API (accidental omissions).

The original goal of this change was to be a patch release for 37.0.1 to enable updating wasmtime-py to the 37.0.x releases of Wasmtime. In the end though the changes here were broad enough that I no longer feel that this is a good idea, so wasmtime-py will be skipping the 37 version of Wasmtime.

This commit is a follow-up to bytecodealliance#11514 which was discovered through
failing tests in the wasmtime-py repository when updating to Wasmtime
37.0.0. Effectively a combination of bugs in the Rust API meant that it
wasn't possible to use `externref` or `anyref` bindings correctly. The
Rust changes in this commit are:

* `wasmtime_val_unroot` correctly drops the value now as opposed to
  effectively being a noop from before (typo of using `as_externref` vs
  `from_externref`).
* `wasmtime_{anyref,externref,val}_t` now have a `Drop` implementation
  in Rust to correctly drop them if a value in Rust is dropped. This is
  required to correctly manage memory in the `wasmtime_func_{call,new}`
  implementations, for example.
* `wasmtime_{anyref,externref,val}_clone` no longer have an unnecessary
  context parameter.
* `wasmtime_{anyref,externref,val}_unroot` no longer have an unnecessary
  context parameter.

Changes in the C/C++ APIs are:

* `Result::{ok,err}_ref` APIs were added in addition to the preexisting
  rvalue accessors.
* Loading/storing typed arguments now has an overload for `const T&` and
  `T&&` which behaves differently. Notably transferring ownership for
  `T&&` and not for `const T&`. This means that passing parameters when
  calling a wasm function uses `const T&`, but passing results from a
  host import uses `T&&`.
* `TypedFunc::call` now uses `const Params&` instead of `Params` to
  explicitly specify it doesn't modify the parameters and forces using
  the `const T&` store method.
* `Store::gc` is now a convenience method for `store.context().gc()`
* `ExternRef`, `AnyRef`, and `Val` now have ownership semantics and
  destructors. This matches the spirit of bytecodealliance#11514 for Rust but models it
  in C++ as well. This required filling out move/copy
  constructors/assignments.
* The explicit `ExternRef` now takes `std::any` instead of `T`.
* Minor issues related to ownership are fixed in `Val` bindings.

Valgrind was used to ensure that there were no leaks for the test suite
which additionally resulted in a number of `*_delete` calls being added
to tests using the C API (accidental omissions).

The original goal of this change was to be a patch release for 37.0.1 to
enable updating wasmtime-py to the 37.0.x releases of Wasmtime. In the
end though the changes here were broad enough that I no longer feel that
this is a good idea, so wasmtime-py will be skipping the 37 version of
Wasmtime.
@alexcrichton alexcrichton requested a review from a team as a code owner October 7, 2025 14:26
@alexcrichton alexcrichton requested review from pchickey and removed request for a team October 7, 2025 14:26
prtest:full
@alexcrichton alexcrichton enabled auto-merge October 7, 2025 14:46
@alexcrichton
Copy link
Member Author

Once this lands on main I'll backport the merged commit to the release-38.0.0 branch as well

alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Oct 7, 2025
A primary mitigation for bytecodealliance#11799 recurring in one form or another in the
future.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Oct 7, 2025
A primary mitigation for bytecodealliance#11799 recurring in one form or another in the
future.

prtest:capi
Copy link
Member

@cfallin cfallin left a comment

Choose a reason for hiding this comment

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

Carrying over my review from the GHSA -- thanks again for catching this and fixing it!

@alexcrichton alexcrichton added this pull request to the merge queue Oct 7, 2025
Merged via the queue into bytecodealliance:main with commit adff9d9 Oct 7, 2025
170 checks passed
@alexcrichton alexcrichton deleted the fix-c-api-onwership branch October 7, 2025 17:03
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Oct 7, 2025
A primary mitigation for bytecodealliance#11799 recurring in one form or another in the
future.

prtest:capi
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Oct 7, 2025
* Fix externref/anyref ownership in C/C++ API

This commit is a follow-up to bytecodealliance#11514 which was discovered through
failing tests in the wasmtime-py repository when updating to Wasmtime
37.0.0. Effectively a combination of bugs in the Rust API meant that it
wasn't possible to use `externref` or `anyref` bindings correctly. The
Rust changes in this commit are:

* `wasmtime_val_unroot` correctly drops the value now as opposed to
  effectively being a noop from before (typo of using `as_externref` vs
  `from_externref`).
* `wasmtime_{anyref,externref,val}_t` now have a `Drop` implementation
  in Rust to correctly drop them if a value in Rust is dropped. This is
  required to correctly manage memory in the `wasmtime_func_{call,new}`
  implementations, for example.
* `wasmtime_{anyref,externref,val}_clone` no longer have an unnecessary
  context parameter.
* `wasmtime_{anyref,externref,val}_unroot` no longer have an unnecessary
  context parameter.

Changes in the C/C++ APIs are:

* `Result::{ok,err}_ref` APIs were added in addition to the preexisting
  rvalue accessors.
* Loading/storing typed arguments now has an overload for `const T&` and
  `T&&` which behaves differently. Notably transferring ownership for
  `T&&` and not for `const T&`. This means that passing parameters when
  calling a wasm function uses `const T&`, but passing results from a
  host import uses `T&&`.
* `TypedFunc::call` now uses `const Params&` instead of `Params` to
  explicitly specify it doesn't modify the parameters and forces using
  the `const T&` store method.
* `Store::gc` is now a convenience method for `store.context().gc()`
* `ExternRef`, `AnyRef`, and `Val` now have ownership semantics and
  destructors. This matches the spirit of bytecodealliance#11514 for Rust but models it
  in C++ as well. This required filling out move/copy
  constructors/assignments.
* The explicit `ExternRef` now takes `std::any` instead of `T`.
* Minor issues related to ownership are fixed in `Val` bindings.

Valgrind was used to ensure that there were no leaks for the test suite
which additionally resulted in a number of `*_delete` calls being added
to tests using the C API (accidental omissions).

The original goal of this change was to be a patch release for 37.0.1 to
enable updating wasmtime-py to the 37.0.x releases of Wasmtime. In the
end though the changes here were broad enough that I no longer feel that
this is a good idea, so wasmtime-py will be skipping the 37 version of
Wasmtime.

* Run `clang-format`

prtest:full
alexcrichton added a commit that referenced this pull request Oct 7, 2025
* Fix externref/anyref ownership in C/C++ API

This commit is a follow-up to #11514 which was discovered through
failing tests in the wasmtime-py repository when updating to Wasmtime
37.0.0. Effectively a combination of bugs in the Rust API meant that it
wasn't possible to use `externref` or `anyref` bindings correctly. The
Rust changes in this commit are:

* `wasmtime_val_unroot` correctly drops the value now as opposed to
  effectively being a noop from before (typo of using `as_externref` vs
  `from_externref`).
* `wasmtime_{anyref,externref,val}_t` now have a `Drop` implementation
  in Rust to correctly drop them if a value in Rust is dropped. This is
  required to correctly manage memory in the `wasmtime_func_{call,new}`
  implementations, for example.
* `wasmtime_{anyref,externref,val}_clone` no longer have an unnecessary
  context parameter.
* `wasmtime_{anyref,externref,val}_unroot` no longer have an unnecessary
  context parameter.

Changes in the C/C++ APIs are:

* `Result::{ok,err}_ref` APIs were added in addition to the preexisting
  rvalue accessors.
* Loading/storing typed arguments now has an overload for `const T&` and
  `T&&` which behaves differently. Notably transferring ownership for
  `T&&` and not for `const T&`. This means that passing parameters when
  calling a wasm function uses `const T&`, but passing results from a
  host import uses `T&&`.
* `TypedFunc::call` now uses `const Params&` instead of `Params` to
  explicitly specify it doesn't modify the parameters and forces using
  the `const T&` store method.
* `Store::gc` is now a convenience method for `store.context().gc()`
* `ExternRef`, `AnyRef`, and `Val` now have ownership semantics and
  destructors. This matches the spirit of #11514 for Rust but models it
  in C++ as well. This required filling out move/copy
  constructors/assignments.
* The explicit `ExternRef` now takes `std::any` instead of `T`.
* Minor issues related to ownership are fixed in `Val` bindings.

Valgrind was used to ensure that there were no leaks for the test suite
which additionally resulted in a number of `*_delete` calls being added
to tests using the C API (accidental omissions).

The original goal of this change was to be a patch release for 37.0.1 to
enable updating wasmtime-py to the 37.0.x releases of Wasmtime. In the
end though the changes here were broad enough that I no longer feel that
this is a good idea, so wasmtime-py will be skipping the 37 version of
Wasmtime.

* Run `clang-format`

prtest:full
github-merge-queue bot pushed a commit that referenced this pull request Oct 7, 2025
* Run ASAN with C/C++ tests in CI

A primary mitigation for #11799 recurring in one form or another in the
future.

prtest:capi

* Fix threaded interactions with ASAN
bongjunj pushed a commit to prosyslab/wasmtime that referenced this pull request Oct 20, 2025
* Fix externref/anyref ownership in C/C++ API

This commit is a follow-up to bytecodealliance#11514 which was discovered through
failing tests in the wasmtime-py repository when updating to Wasmtime
37.0.0. Effectively a combination of bugs in the Rust API meant that it
wasn't possible to use `externref` or `anyref` bindings correctly. The
Rust changes in this commit are:

* `wasmtime_val_unroot` correctly drops the value now as opposed to
  effectively being a noop from before (typo of using `as_externref` vs
  `from_externref`).
* `wasmtime_{anyref,externref,val}_t` now have a `Drop` implementation
  in Rust to correctly drop them if a value in Rust is dropped. This is
  required to correctly manage memory in the `wasmtime_func_{call,new}`
  implementations, for example.
* `wasmtime_{anyref,externref,val}_clone` no longer have an unnecessary
  context parameter.
* `wasmtime_{anyref,externref,val}_unroot` no longer have an unnecessary
  context parameter.

Changes in the C/C++ APIs are:

* `Result::{ok,err}_ref` APIs were added in addition to the preexisting
  rvalue accessors.
* Loading/storing typed arguments now has an overload for `const T&` and
  `T&&` which behaves differently. Notably transferring ownership for
  `T&&` and not for `const T&`. This means that passing parameters when
  calling a wasm function uses `const T&`, but passing results from a
  host import uses `T&&`.
* `TypedFunc::call` now uses `const Params&` instead of `Params` to
  explicitly specify it doesn't modify the parameters and forces using
  the `const T&` store method.
* `Store::gc` is now a convenience method for `store.context().gc()`
* `ExternRef`, `AnyRef`, and `Val` now have ownership semantics and
  destructors. This matches the spirit of bytecodealliance#11514 for Rust but models it
  in C++ as well. This required filling out move/copy
  constructors/assignments.
* The explicit `ExternRef` now takes `std::any` instead of `T`.
* Minor issues related to ownership are fixed in `Val` bindings.

Valgrind was used to ensure that there were no leaks for the test suite
which additionally resulted in a number of `*_delete` calls being added
to tests using the C API (accidental omissions).

The original goal of this change was to be a patch release for 37.0.1 to
enable updating wasmtime-py to the 37.0.x releases of Wasmtime. In the
end though the changes here were broad enough that I no longer feel that
this is a good idea, so wasmtime-py will be skipping the 37 version of
Wasmtime.

* Run `clang-format`

prtest:full
bongjunj pushed a commit to prosyslab/wasmtime that referenced this pull request Oct 20, 2025
* Run ASAN with C/C++ tests in CI

A primary mitigation for bytecodealliance#11799 recurring in one form or another in the
future.

prtest:capi

* Fix threaded interactions with ASAN
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.

2 participants