Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wasmtime: Pop GC LIFO roots even when there is no GC heap #8899

Merged
merged 2 commits into from
Jul 8, 2024

Conversation

fitzgen
Copy link
Member

@fitzgen fitzgen commented Jul 3, 2024

We can create and root i31refs without ever allocating a GC heap for the store, so we can't guard popping LIFO roots on the presence of a GC heap or else we risk unbounded growth of the LIFO root set.

We can create and root `i31ref`s without ever allocating a GC heap for the
store, so we can't guard popping LIFO roots on the presence of a GC heap or else
we risk unbounded growth of the LIFO root set.
@fitzgen fitzgen requested a review from a team as a code owner July 3, 2024 19:46
@fitzgen fitzgen requested review from elliottt and removed request for a team July 3, 2024 19:46
@github-actions github-actions bot added wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:ref-types Issues related to reference types and GC in Wasmtime labels Jul 3, 2024
Copy link

github-actions bot commented Jul 3, 2024

Subscribe to Label Action

cc @fitzgen

This issue or pull request has been labeled: "wasmtime:api", "wasmtime:ref-types"

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

  • fitzgen: wasmtime:ref-types

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

Learn more.

@alexcrichton alexcrichton added this pull request to the merge queue Jul 8, 2024
Merged via the queue into bytecodealliance:main with commit b8872ed Jul 8, 2024
36 checks passed
@fitzgen fitzgen deleted the leaking-rooted-i31refs branch July 8, 2024 17:44
alexcrichton pushed a commit to alexcrichton/wasmtime that referenced this pull request Jul 9, 2024
…liance#8899)

* Wasmtime: Pop GC LIFO roots even when there is no GC heap

We can create and root `i31ref`s without ever allocating a GC heap for the
store, so we can't guard popping LIFO roots on the presence of a GC heap or else
we risk unbounded growth of the LIFO root set.

* Fix build with the gc feature disabled
fitzgen added a commit that referenced this pull request Jul 9, 2024
* Add custom-pages-sizes to CLI flags (#8917)

* Update dependency on `wit-bindgen` (#8911)

* Update dependency on `wit-bindgen`

Updating to the latest released version.

* Add vets

* Fix build of test-programs

* Fix panic with invalid DWARF file indices (#8913)

I'm not entirely sure what causes this but Wasmtime shouldn't panic with
invalid DWARF. In general (#5537) Wasmtime's support for DWARF needs to
be rewritten, but in the meantime let's play whack-a-mole with panics
and try to paper over issues.

Closes #8884
Closes #8904

* Wasmtime: Pop GC LIFO roots even when there is no GC heap (#8899)

* Wasmtime: Pop GC LIFO roots even when there is no GC heap

We can create and root `i31ref`s without ever allocating a GC heap for the
store, so we can't guard popping LIFO roots on the presence of a GC heap or else
we risk unbounded growth of the LIFO root set.

* Fix build with the gc feature disabled

---------

Co-authored-by: Nick Fitzgerald <fitzgen@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:ref-types Issues related to reference types and GC in Wasmtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants