Skip to content

Duplicate page size determination in wasmtime-fiber #10803

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

Merged

Conversation

alexcrichton
Copy link
Member

@alexcrichton alexcrichton commented May 19, 2025

Currently Wasmtime has a function crate::runtime::vm::host_page_size
but this isn't reachable from the wasmtime-fiber crate and instead tha
crate uses rustix::param::page_size to determine the host page size.
It looks like this usage of rustix is causing a panic in #10802.
Ideally wasmtime-fiber would be able to use the same function but the
crate separation does not currently make that feasible. For now
duplicate the logic of wasmtime into wasmtime-fiber as it's modest
enough to ensure that this does not panic.

Closes #10802

@alexcrichton alexcrichton requested review from a team as code owners May 19, 2025 12:52
@alexcrichton alexcrichton requested review from pchickey and removed request for a team May 19, 2025 12:52
@github-actions github-actions bot added the wasmtime:api Related to the API of the `wasmtime` crate itself label May 19, 2025
@alexcrichton alexcrichton force-pushed the remove-rustix-page-size branch from 2f4bbc6 to 37e8216 Compare May 20, 2025 12:18
@alexcrichton alexcrichton enabled auto-merge May 20, 2025 12:19
@alexcrichton alexcrichton added this pull request to the merge queue May 20, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 20, 2025
Currently Wasmtime has a function `crate::runtime::vm::host_page_size`
but this isn't reachable from the `wasmtime-fiber` crate and instead tha
crate uses `rustix::param::page_size` to determine the host page size.
It looks like this usage of `rustix` is causing a panic in bytecodealliance#10802.
Ideally `wasmtime-fiber` would be able to use the same function but the
crate separation does not currently make that feasible. For now
duplicate the logic of `wasmtime` into `wasmtime-fiber` as it's modest
enough to ensure that this does not panic.

Closes bytecodealliance#10802
@alexcrichton alexcrichton force-pushed the remove-rustix-page-size branch from 37e8216 to 258c0e6 Compare May 20, 2025 13:50
@alexcrichton alexcrichton changed the title Reduce number of locations host page size comes from Duplicate page size determination in wasmtime-fiber May 20, 2025
@alexcrichton alexcrichton requested review from a team and dicej and removed request for a team May 20, 2025 13:51
@alexcrichton
Copy link
Member Author

I don't believe the CI failure is easily addressable with the previous design so I opted to duplicate the definition of host_page_size from the wasmtime crate into the wasmtime-fiber crate instead of passing it in as a parameter. That I think is regrettable duplication but "at least it's now commented duplication" and also fixes the panic for now.

I've also re-rolled review since Pat is on vacation right now since this has now changed substantially enough that I don't want to carry over the previous approval.

@alexcrichton
Copy link
Member Author

I'm actually going to go ahead and flag this for merge. Apparently using this syscall prevents using rr with Wasmtime (rr crashes locally) so I'd like to get that fixed. If others have further comments on this I'm happy to handle them in follow-ups.

@alexcrichton alexcrichton added this pull request to the merge queue May 21, 2025
Merged via the queue into bytecodealliance:main with commit b3fa9bf May 22, 2025
160 checks passed
@alexcrichton alexcrichton deleted the remove-rustix-page-size branch May 22, 2025 00:11
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Panic during component instantiation
2 participants