-
Notifications
You must be signed in to change notification settings - Fork 1.6k
consistently create thread and task when entering component instance #12379
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
base: main
Are you sure you want to change the base?
Conversation
Label Messager: wasmtime:configIt looks like you are changing Wasmtime's configuration options. Make sure to
DetailsTo modify this label's message, edit the To add new label messages or remove existing label messages, edit the |
Previously, we weren't creating a new thread or task in all cases when entering a component instance, even when component model async features were enabled. In particular, entering an instance via a sync-to-sync, guest-to-guest adapter, via `Linker::instantiate[_async]`, or via `[Typed]Func::call` all skipped creating a thread or task, creating panics and/or instance mismatches in certain cases. This commit addresses all those cases and also adds assertions to all CM async intrinsics to verify that the caller instance matches the most-recently-pushed task. Note that we still skip pushing and popping threads and tasks if no CM async features are enabled in the `Config`. In order to populate the `GuestTask::instance` field for tasks created as part of `Linker::instantiate[_async]` calls, I had to add a `RuntimeComponentInstanceIndex` field to `GlobalInitializer::InstantiateModule` and friends so it would be available when needed. While testing this, I uncovered and fixed a couple of related issues: - We weren't checking the `may_leave` flag when guest-to-guest calling a resource destructor - We weren't checking whether a subtask was ready to delete (e.g. that no threads were still running) before attempting to delete it while deleting its supertask
While rebasing bytecodealliance/wasmtime#12379 onto Wasmtime's `main` branch, I found I needed to tweak the expected resource handle values and trap messages due to subtle changes to when Wasmtime allocates a thread handle. Once WebAssembly#600 lands and is implemented in Wasmtime, we should be able to clean all this up once and for all; for now we just muddle along.
|
I've rebased this onto main and addressed the feedback so far locally. Once WebAssembly/component-model#601 is merged, I'll push my updates. |
While rebasing bytecodealliance/wasmtime#12379 onto Wasmtime's `main` branch, I found I needed to tweak the expected resource handle values and trap messages due to subtle changes to when Wasmtime allocates a thread handle. Once #600 lands and is implemented in Wasmtime, we should be able to clean all this up once and for all; for now we just muddle along.
ea0b3fb to
200c24a
Compare
alexcrichton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Two other possible spots worth scrutinizing:
cabi_reallocinvocations - probably don't need special treatment? Unsure.ResourceAny::resource_drop- I think this'll need task-related treatment?
Although given the litany of entrypoints into wasm I'm finding it really hard to keep them all in sync, so it's probably also worth refactoring in theory to only have one entrypoint somewhere
Previously, we weren't creating a new thread or task in all cases when entering a component instance, even when component model async features were enabled. In particular, entering an instance via a sync-to-sync, guest-to-guest adapter, via
Linker::instantiate[_async], or via[Typed]Func::callall skipped creating a thread or task, leading to panics and/or instance mismatches in certain cases.This commit addresses all those cases and also adds assertions to all CM async intrinsics to verify that the caller instance matches the most-recently-pushed task. Note that we still skip pushing and popping threads and tasks if no CM async features are enabled in the
Config.In order to populate the
GuestTask::instancefield for tasks created as part ofLinker::instantiate[_async]calls, I had to add aRuntimeComponentInstanceIndexfield toGlobalInitializer::InstantiateModuleand friends so it would be available when needed.While testing this, I uncovered and fixed a couple of related issues:
may_leaveflag when guest-to-guest calling a resource destructor