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

Add an instance API for invoking the WASI start functions. #1516

Closed
wants to merge 4 commits into from

Conversation

sunfishcode
Copy link
Member

This adds support for the WASI reactor ABI proposed in
WebAssembly/WASI#256, as well as the existing
_start ABI for commands.

This also implements the semantics that command instances should not
persist after their _start function is called. As a safety measure,
this includes a check that the instance refcount is 1, so that other
references to the instance don't hold it live.

Implementing that prompted a change to how the main Instance API works.
Instead having instances eagerly compute a Vec of Externs, and bumping
the refcount for each Extern, compute Externs on demand.

This also means that the closure returned by get0 and friends now
holds an InstanceHandle to dynamically hold the instance live rather
than being scoped to a lifetime.

This is a draft PR for now while I think more about whether this is a good
approach.

This adds support for the WASI reactor ABI proposed in
WebAssembly/WASI#256, as well as the existing
_start ABI for commands.

This also implements the semantics that command instances should not
persist after their _start function is called. As a safety measure,
this includes a check that the instance refcount is 1, so that other
references to the instance don't hold it live.

Implementing that prompted a change to how the main Instance API works.
Instead having instances eagerly compute a Vec of Externs, and bumping
the refcount for each Extern, compute Externs on demand.

This also means that the closure returned by `get0` and friends now
holds an `InstanceHandle` to dynamically hold the instance live rather
than being scoped to a lifetime.
@github-actions github-actions bot added wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:c-api Issues pertaining to the C API. labels Apr 15, 2020
@github-actions
Copy link

Subscribe to Label Action

cc @peterhuene

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

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

  • peterhuene: wasmtime:api, wasmtime:c-api

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

Learn more.

Make `Module::exports` and `Module::imports` return iterators instead of
slices, and compute them on demand rather than eagerly.

Also, compute them from wasmtime_environ's data structures, rather than
re-decoding the wasm bytes.

To support this, change wasmtime_environ::Module hold a single imports
array rather than having per-type arrays.
@sunfishcode sunfishcode marked this pull request as ready for review April 16, 2020 05:38
@github-actions github-actions bot added the fuzzing Issues related to our fuzzing infrastructure label Apr 16, 2020
@sunfishcode
Copy link
Member Author

Ok, I've now updated Module's exports and imports to return their results on-demand too, which turned into a minor yak shave, but it significantly cleans up import/export decoding -- it eliminates the redundant decoding in Module::new, and tidies wasmtime_environ::Module.

@github-actions
Copy link

Subscribe to Label Action

cc @fitzgen

This issue or pull request has been labeled: "fuzzing"

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

  • fitzgen: fuzzing

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

Learn more.

@sunfishcode
Copy link
Member Author

Looking at where this PR ended up, there's a lot of refactoring here mixed in with a new feature, so I'm going to split this PR into pieces.

@sunfishcode sunfishcode deleted the reactor branch April 16, 2020 15:23
@sunfishcode sunfishcode mentioned this pull request Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fuzzing Issues related to our fuzzing infrastructure wasmtime:api Related to the API of the `wasmtime` crate itself wasmtime:c-api Issues pertaining to the C API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants