Support select() and wait() on Group#551
Conversation
Add the select() future protocol to Group so a group's completion (all
tasks drained) can be raced against other futures, e.g.
select(.{ .group = &group, .recv = channel.asyncReceive() }).
Rather than add a waiter queue to Group (whose layout is pinned to
std.Io.Group's two words), the waiter parks on the same futex address
that unregisterGroupTask already wakes when the task counter reaches
zero. This needs no per-group storage and reuses the existing, already
stack-safe futex wake path.
To support this, FutexWaiter now references an externally-owned Waiter
instead of embedding one, and Futex gains a non-blocking prepareWait()/
cancelWait() pair (the async counterpart of wait()) used by the protocol.
Unlike Group.wait(), observing a group via select()/wait() does not close
it and does not participate in fail_fast: it purely waits for the counter
to reach zero, leaving the group reusable.
📝 WalkthroughWalkthroughGroup gains a select()/wait()-compatible future protocol (Result, WaitContext, getResult, asyncWait, asyncCancelWait) that parks on the group's futex state without closing it. Futex.zig's internal FutexWaiter is exported and refactored to hold a Waiter pointer instead of an owned value, with new prepareWait/cancelWait registration APIs backing this. ChangesFutex waiter refactor and Group select protocol
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/sync/Futex.zig (1)
248-272: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
prepareWait/cancelWaitlook correct; just note thewaiter = undefinedsharp edge.
node.* = .{ .waiter = waiter, .address = address }fully re-inits the node (queue links,in_list, padding) so reuse across register/cancel cycles is clean. The no-value-check contract is documented, andasyncWaitingroup.zigcloses the lost-wakeup race with its post-registration double check, so this half holds up.One thing worth keeping in the docstring:
cancelWaitderives the bucket fromnode.address, so it's only valid after a matchingprepareWait. Calling it on a node that never went throughprepareWait(e.g. anasyncWaitfast-path that returned "ready" without registering) would hash a garbage address and hitremove()on a node that was never in the list. The current protocol avoids that, but the field defaultwaiter: *Waiter = undefinedplusaddress: usize = 0makes it easy to trip later.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/sync/Futex.zig` around lines 248 - 272, `FutexWaiter`’s default `waiter = undefined` and `address = 0` make `cancelWait` fragile if it is ever called before `prepareWait`. Update the `FutexWaiter` initialization/contract so unprepared nodes are clearly invalid or safely initialized, and add a guard or assertion in `cancelWait`/`removeFromBucket` to reject nodes that have not been registered by `prepareWait`. Also keep the `prepareWait`/`cancelWait` doc comments explicit that `cancelWait` is only valid after a matching `prepareWait`.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/sync/Futex.zig`:
- Around line 248-272: `FutexWaiter`’s default `waiter = undefined` and `address
= 0` make `cancelWait` fragile if it is ever called before `prepareWait`. Update
the `FutexWaiter` initialization/contract so unprepared nodes are clearly
invalid or safely initialized, and add a guard or assertion in
`cancelWait`/`removeFromBucket` to reject nodes that have not been registered by
`prepareWait`. Also keep the `prepareWait`/`cancelWait` doc comments explicit
that `cancelWait` is only valid after a matching `prepareWait`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f3ea7896-b21a-40ad-aad9-32625b76eebe
📒 Files selected for processing (2)
src/group.zigsrc/sync/Futex.zig
What
Adds the
select()future protocol toGroup, so a group's completion (all tasks drained to zero) can be raced against other futures:Single-future
wait(&group)works too.Why this approach
Group's storage is pinned tostd.Io.Group's two words (token+state) via thefromStdpointer cast, so there is no room to add a waiter queue. Instead of splitting the type or stealing bits, the waiter parks on the same futex address thatunregisterGroupTaskalready wakes when the task counter hits zero.This means:
Group— layout stays identical tostd.Io.Group,fromStdis unchanged.Futex.wakeonly hashes the address (never dereferences the group) and is already stack-safe. The group can be freed the instant completion fires.Group.wait()er and aselectwaiter can coexist on the same address; the wake fans out to both.Changes
Futex:FutexWaiternow references an externally-ownedWaiter(was embedded), and a non-blockingprepareWait()/cancelWait()pair is added — the async counterpart ofwait(), used by the select protocol. The caller owns the predicate check.Group: implementsResult/WaitContext/getResult/asyncWait/asyncCancelWait.asyncWaitchecks the counter, parks, then re-checks to close the lost-wakeup race.Semantics
Unlike
Group.wait(), observing a group viaselect()/wait()does not close it and does not participate infail_fast— it purely waits for the task counter to reach zero, leaving the group reusable afterward.Tests
Added to
group.zig: group-wins, other-future-wins, already-drained fast path, and single-futurewait(&group)— all asserting the group remains usable after being observed. Full suite: 525/525 pass.