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

Reserve handle index 0 and add handle index max in Canonical ABI #284

Merged
merged 1 commit into from
Dec 13, 2023

Conversation

lukewagner
Copy link
Member

As discussed in #282. In theory, this shouldn't be breaking since guest code shouldn't be depending on the precise index values they receive, treating them like opaque file-descriptor-like values.

@lukewagner lukewagner mentioned this pull request Dec 5, 2023
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Dec 8, 2023
This commit updates the allocation scheme for resources in the component
model to start at 1 instead of 0 when communicating with components.
This is an implementation of WebAssembly/component-model#284.

While this broke a number of tests we have this shouldn't actually break
any components in practice. The broken tests were all overly-precise in
their assertions and error messages and this shouldn't idiomatically
come up in any guest language, so this should not be a practically
breaking change.
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Dec 8, 2023
This commit updates the allocation scheme for resources in the component
model to start at 1 instead of 0 when communicating with components.
This is an implementation of WebAssembly/component-model#284.

While this broke a number of tests we have this shouldn't actually break
any components in practice. The broken tests were all overly-precise in
their assertions and error messages and this shouldn't idiomatically
come up in any guest language, so this should not be a practically
breaking change.

This change additionally places an upper limit on the maximum
allocatable index at `1 << 30` which is also specified in the above PR.
@alexcrichton
Copy link
Collaborator

I've posted bytecodealliance/wasmtime#7661 as the Wasmtime-side implementation change this correponds to

@lukewagner
Copy link
Member Author

Great! I'm thinking I'll wait another few days to see if there's any feedback and otherwise merge mid next week.

@lukewagner lukewagner merged commit da1cd55 into main Dec 13, 2023
2 checks passed
@lukewagner lukewagner deleted the reserve-handle-indices branch December 13, 2023 17:05
alexcrichton added a commit to alexcrichton/wasmtime that referenced this pull request Feb 21, 2024
This commit updates the allocation scheme for resources in the component
model to start at 1 instead of 0 when communicating with components.
This is an implementation of WebAssembly/component-model#284.

While this broke a number of tests we have this shouldn't actually break
any components in practice. The broken tests were all overly-precise in
their assertions and error messages and this shouldn't idiomatically
come up in any guest language, so this should not be a practically
breaking change.

This change additionally places an upper limit on the maximum
allocatable index at `1 << 30` which is also specified in the above PR.
github-merge-queue bot pushed a commit to bytecodealliance/wasmtime that referenced this pull request Feb 21, 2024
This commit updates the allocation scheme for resources in the component
model to start at 1 instead of 0 when communicating with components.
This is an implementation of WebAssembly/component-model#284.

While this broke a number of tests we have this shouldn't actually break
any components in practice. The broken tests were all overly-precise in
their assertions and error messages and this shouldn't idiomatically
come up in any guest language, so this should not be a practically
breaking change.

This change additionally places an upper limit on the maximum
allocatable index at `1 << 30` which is also specified in the above PR.
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