Skip to content

feat: add task group - #308

Open
Colerar wants to merge 2 commits into
apache:mainfrom
Colerar:codex/task-group
Open

Colerar wants to merge 2 commits into
apache:mainfrom
Colerar:codex/task-group

Conversation

@Colerar

@Colerar Colerar commented Sep 13, 2026

Copy link
Copy Markdown

Summary

This PR implements the first TaskGroup proposed in the issue discussion.

Issue: #272

The design rationale and alternatives are discussed in this issue comment.

Main API

let (mut group, registrar) = TaskGroup::new();

let first = tokio::spawn(registrar.track(worker_a()).unwrap());
let second = tokio::spawn(registrar.track(worker_b()).unwrap());

group.close();

while let Some(output) = group.join_next().await {
    process(output);
}

first.await.unwrap();
second.await.unwrap();

Scope of this PR

This version intentionally does not provide:

  • cancellation or a cancellation token
  • forced interruption or task abortion
  • try_join(), race(), or race_ok() result policies
  • bounded concurrency or output backpressure
  • multiple result consumers

@Colerar

Colerar commented Sep 13, 2026

Copy link
Copy Markdown
Author

This version intentionally does not provide:
……

  • try_join(), race(), or race_ok() result policies

I am not sure whether this is the right time to add TaskGroup. It may also make sense to wait until the cancellation token proposed in #223 is available.

That said, this PR contains my current implementation so that we can evaluate the API and how it should eventually compose with cancellation. :D

@QwQBiG

QwQBiG commented Sep 14, 2026

Copy link
Copy Markdown
Member

Tested 2d7df57 locally: cargo x test, cargo x check, and cargo x miri passed. :P

Two points:

  • Rust 1.86 fails with E0658 on the let chains in Registration::drop. Could you use nested if let to preserve the declared MSRV?
  • I confirmed that wait() can return before the completed future is dropped if its Tracked wrapper remains alive. Is that intentional? If so, documenting this boundary would help callers distinguish result completion from resource cleanup.

Sorry if I misunderstood, or if this was already explained and I missed it.

@tisonkun

Copy link
Copy Markdown
Member

Rust 1.86 fails with E0658 on the let chains in Registration::drop. Could you use nested if let to preserve the declared MSRV?

While I don't insist on MSRV and agree that we can always bump the MSRV when needed, we may do it in a separate PR and decide the next MSRV with certain reason.

@Colerar

Colerar commented Sep 14, 2026

Copy link
Copy Markdown
Author

Tested 2d7df57 locally: cargo x test, cargo x check, and cargo x miri passed. :P

Two points:

* Rust 1.86 fails with E0658 on the let chains in `Registration::drop`. Could you use nested `if let` to preserve the declared MSRV?

* I confirmed that `wait()` can return before the completed future is dropped if its `Tracked` wrapper remains alive. Is that intentional? If so, documenting this boundary would help callers distinguish result completion from resource cleanup.

Sorry if I misunderstood, or if this was already explained and I missed it.

@QwQBiG Thanks for catching both issues!

  • MSRV fixed.
  • I agreed that wait() should also cover resource cleanup. This is consistent with similar APIs such as tokio_util::TaskTracker, which use wrapper destruction as the task-lifetime boundary.

@QwQBiG

QwQBiG commented Sep 14, 2026

Copy link
Copy Markdown
Member

Thanks for addressing both points and adding the regression test! Keeping the registration active until Tracked is dropped makes sense to me.

Agreed that any MSRV bump should be handled in a separate PR with a clear rationale. :P

@tisonkun tisonkun left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This doesn't seem like a common primitive. When would you need such a primitive?

I may consider this join-alike primitive a subset of https://docs.rs/futures-concurrency.

While I may consider to include some functions of futures-concurrency in asyncband, it doesn't look like in this form:

  1. Why close?
  2. Why all tasks must output the same type T?

I may expect some real world use cases for motivation and would consider if we can have a more generic/unified form as futures-concurrency has already designed.

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.

3 participants