Stream: git-wasmtime

Topic: wasmtime / PR #14416 add (off-by-default) `task-group-hoo...

view this post on Zulip Wasmtime GitHub notifications bot (Sep 24 2026 at 23:29):

dicej requested alexcrichton for a review on PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 24 2026 at 23:29):

dicej opened PR #14416 from dicej:task-group-hook to bytecodealliance:main:

This addresses #14247 and roughly resembles the API sketch Alex posted as a comment on that issue.

Although the Component Model has no notion of a "task group", we define one here in order to enable embedders to associate guest->host calls with corresponding host->guest calls in a predictable way. See the doc comments on TaskGroupHook for details on what it means for a task to be part of a task group and what notifications a hook will receive for a given group.

Note that, unlike with CallHookHandler, the functions in TaskGroupHook do not have a StoreContextMut<T> parameter. Supporting such a parameter is technically possible, but would require converting a _lot_ of functions so that they accept &mut dyn VMStore parameters instead of &mut StoreOpaque parameters, including parts of the public API. This is something we can revisit later, if desired. Meanwhile, types implementing TaskGroupHook may need to use e.g. Arc to share state with the T in Store<T>.

This feature supersedes StoreContextMut::async_call_stack, which I've removed given that it is prone to use-after-free of a table slot as described in the aforementioned issue.

Fixes #14247

<!--
Please make sure you include the following information:

Our development process is documented in the Wasmtime book:
https://docs.wasmtime.dev/contributing-development-process.html

Please review the Bytecode Alliance's AI tool usage policy at
https://github.com/bytecodealliance/governance/blob/main/AI_TOOL_POLICY.md

Please ensure all communication follows the code of conduct:
https://github.com/bytecodealliance/wasmtime/blob/main/CODE_OF_CONDUCT.md
-->

view this post on Zulip Wasmtime GitHub notifications bot (Sep 24 2026 at 23:29):

dicej requested wasmtime-core-reviewers for a review on PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 24 2026 at 23:29):

dicej requested wasmtime-default-reviewers for a review on PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 01:11):

github-actions[bot] added the label wasmtime:api on PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 12:53):

:speech_balloon: lann created PR review comment:

I guess if you count the part about "semantic tail-calls" this really is just about the root task lifetime. Apart from maybe mentioning that here I still think "task group" makes sense, being analogous to a posix process group.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 12:53):

:memo: lann submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 13:21):

:memo: lann submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 13:21):

:speech_balloon: lann created PR review comment:

What happens when these return Err?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 13:33):

:speech_balloon: lann edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 14:34):

:memo: lann submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 14:34):

:speech_balloon: lann created PR review comment:

If this returns an error will it leak the TaskGroup? Or will the whole ConcurrentState get dropped immediately anyway?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 15:39):

:memo: alexcrichton submitted PR review:

Could this perhaps be something where a task_group_hook.rs submodule could be made to help split out code and reduce the #[cfg] traffic in concurrent.rs? (also in the spirit of keeping the file a bit smaller...)

Also, would it make sense to revert other work from #13510 such as the FuncCallConcurrent abstraction? (as that's sort of no longer relevant)

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 15:39):

:speech_balloon: alexcrichton created PR review comment:

In addition to having the method here, can the method also be on StoreContextMut?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 15:39):

:speech_balloon: alexcrichton created PR review comment:

Would it be possible to integrate this into GuestTask::new and perhaps HostTask::new?

I'm a bit wary to sprinkle incref/decref throughout the code in various locations because it feels like inevitably we'll forget to put one somewhere. By handling it once on construction and once on destruction however that feels more natural to me.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 15:39):

:speech_balloon: alexcrichton created PR review comment:

I think ideally this location would be the "one single location" that the decrement happens -- would it be possible to shuffle things around such that that's the case?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 15:39):

:speech_balloon: alexcrichton created PR review comment:

Could this perhaps get hooked into the deferred-host infrastructure? For example only incref'd when the host task is actually created?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 16:20):

:memo: lann submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 16:20):

:speech_balloon: lann created PR review comment:

It might be a little weird to attach the hook in the middle of task exection? :thinking:

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 22:23):

:memo: dicej submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 22:23):

:speech_balloon: dicej created PR review comment:

Yes, that will trap and poison the store. This was a good reminder to review the various ways a host can call into a component, cause these hooks to run, and return an error, in which case we need to poison the store even if the error happened without any guest code on the stack. I'll push an update which covers the cases I found that weren't already covered.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 22:24):

:memo: dicej submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 22:24):

:speech_balloon: dicej created PR review comment:

I'll push an update which ensures that TaskGroupHook::handle_finish is called on any TaskGroups still in the table when the store is dropped.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 22:46):

:speech_balloon: dicej edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 25 2026 at 22:46):

:speech_balloon: dicej edited PR review comment.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 15:08):

dicej requested pchickey for a review on PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 15:08):

dicej updated PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 15:08):

dicej requested wasmtime-wasi-reviewers for a review on PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 15:12):

:memo: dicej submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 15:12):

:speech_balloon: dicej created PR review comment:

I've added a footnote referencing the semantic tail-calls blurb.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 15:14):

:speech_balloon: dicej created PR review comment:

I've added a paragraph to the doc comment describing what happens when these functions return Err, and more generally what events will be delivered if the store traps for any reason.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 15:14):

:memo: dicej submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 15:17):

dicej updated PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 15:55):

dicej updated PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 16:51):

dicej updated PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 17:31):

dicej commented on PR #14416:

I believe I've addressed all the feedback so far.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 19:27):

:memo: alexcrichton submitted PR review:

A few minor things, mostly I think a *_disabled.rs file would work well here, but otherwise lgtm

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 19:27):

:speech_balloon: alexcrichton created PR review comment:

A #[cfg]'d method parameter in a publi-facing trait can sort of wreak havoc with dependencies -- could this be Option<TaskGroupId>?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 19:27):

:speech_balloon: alexcrichton created PR review comment:

Can you add inline docs to this feature here?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 19:27):

:speech_balloon: alexcrichton created PR review comment:

To clarify as well, I don't think that this file should have zero #[cfg] in it, but I think it could still be reduced quite a bit by having enabled/disabled files, and that I think is beneficial to have a file that contains 99%+ of the implementation as opposed to as-is where you've got to cross-reference a few file simultaneously to find the body of the implementation.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 19:27):

:speech_balloon: alexcrichton created PR review comment:

Could this be defined as a noop for task-group-hook-disabled to avoid the #[cfg] here?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 19:27):

:speech_balloon: alexcrichton created PR review comment:

FWIW this file still has quite a lot of #[cfg] which IMO defeats most of the purpose of outlining it into task_group_hook.rs as the goal is to generally have everything in one location with little #[cfg] necessary. That being said I might recommend some changes here to assist with that and reduce the #[cfg] this file. Notably a task_group_hook_disabled.rs would be a location to have stubs that do nothing, and that would enable this file to ideally have far less #[cfg] to assume that task group management should always be done. If the feature is disabled the functions would be noops, but when enabled it'd route into task_group_hooks.rs.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 22:04):

dicej updated PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 22:05):

:memo: dicej submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 22:05):

:speech_balloon: dicej created PR review comment:

I just pushed an update which makes TaskGroupId a unit type when the task-group-hook feature is disabled, so now this parameter is present unconditionally, with no Option<_> needed.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 22:06):

:memo: dicej submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 22:06):

:speech_balloon: dicej created PR review comment:

I just pushed an update; please let me know what you think.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 22:08):

:memo: dicej submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 22:08):

:speech_balloon: dicej created PR review comment:

Err, hold on, let me address the CI issues first.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 22:13):

dicej updated PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 22:18):

dicej updated PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 22:28):

dicej updated PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 22:42):

:memo: dicej submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 29 2026 at 22:42):

:speech_balloon: dicej created PR review comment:

Ok, really this time.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 19:26):

:thumbs_up: alexcrichton submitted PR review:

Just some minor nits, but lgtm :+1:

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 19:26):

:speech_balloon: alexcrichton created PR review comment:

For this can you add { _priv: () } to ensure that this isn't publicly-visible/constructible as an empty struct?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 19:26):

:speech_balloon: alexcrichton created PR review comment:

Bikeshed on this a bit: hook_switch_threads? (switch_threads feels a bit too powerful/generic for what it's doing)

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 19:26):

:speech_balloon: alexcrichton created PR review comment:

Unnecessary cfg now I think?

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 19:26):

:speech_balloon: alexcrichton created PR review comment:

Could this be moved down to mod task_group_hook_disabled? This sort of renaming stylistically typically happens near the mod declaration vs imports from other crates/modules.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 19:26):

:speech_balloon: alexcrichton created PR review comment:

This might be able to pub use task_group_hook::* to avoid individually listing each name

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 19:26):

:speech_balloon: alexcrichton created PR review comment:

indentation a bit off

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 22:41):

:memo: dicej submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 22:41):

:speech_balloon: dicej created PR review comment:

True, but I tend to avoid wildcard imports; they make it harder for the reader to determine where a given symbol is defined.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 22:44):

:memo: dicej submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 22:44):

:speech_balloon: dicej created PR review comment:

I know, but rustfmt insists :shrug:

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 22:45):

dicej updated PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 22:46):

:memo: dicej submitted PR review.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 22:46):

:speech_balloon: dicej created PR review comment:

I changed it to handle_thread_switch.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 22:47):

dicej has enabled auto merge for PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 22:56):

dicej added PR #14416 add (off-by-default) task-group-hook feature to the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Sep 30 2026 at 23:23):

github-merge-queue[bot] removed PR #14416 add (off-by-default) task-group-hook feature from the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 01 2026 at 00:33):

dicej updated PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 01 2026 at 00:34):

dicej has enabled auto merge for PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 01 2026 at 00:44):

dicej added PR #14416 add (off-by-default) task-group-hook feature to the merge queue.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 01 2026 at 01:08):

:check: dicej merged PR #14416.

view this post on Zulip Wasmtime GitHub notifications bot (Oct 01 2026 at 01:08):

dicej removed PR #14416 add (off-by-default) task-group-hook feature from the merge queue.

Last updated: Oct 11 2026 at 04:10 UTC