Skip to content

test(coro): stabilize task group completion regression#1005

Merged
Coldwings merged 1 commit into
mainfrom
fix/ci-task-group-test-placement
Jul 21, 2026
Merged

test(coro): stabilize task group completion regression#1005
Coldwings merged 1 commit into
mainfrom
fix/ci-task-group-test-placement

Conversation

@Coldwings

@Coldwings Coldwings commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Description

Make the task_group completion-order regression establish deterministic worker placement before it pauses child completion. The inner owner coroutine explicitly migrates to worker 0, and the child explicitly migrates to worker 1 before it is allowed to fail.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature
  • Breaking change
  • Performance improvement
  • Documentation
  • Refactoring
  • Tests
  • Build/CI

Related Issues

Closes #1003

Changes Made

Core Changes

  • Set affinity inside the actual owner coroutine instead of relying only on the outer scheduler wrapper.
  • Assert that the owner runs on worker 0 and that its child runs on worker 1.
  • Gate child failure until the test thread has observed worker-1 execution.
  • Preserve completion-pause, pending-join, post-release destruction, shutdown, and exception assertions.

API Changes (if applicable)

None.

Migration Guide (if breaking change)

Not applicable.

Testing

Unit Tests

  • Added new tests for the changes
  • Updated existing tests if needed
  • All tests pass locally

Integration Tests

  • Tested with existing examples
  • Tested in real-world scenarios (if applicable)

Sanitizer Testing

  • Tested with ASAN
  • Tested with TSAN
  • No new warnings or errors

Test Results

Local compilation was intentionally not performed. The patch passed git diff --check; GitHub Actions provides executable x64/arm64 and sanitizer validation.

Checklist

Code Quality

  • My code follows the project style
  • The placement invariant is explicit and asserted
  • I removed no unrelated code
  • Static review found no production behavior change

Documentation

  • Documentation update (not applicable: test harness only)
  • Examples (not applicable)
  • API documentation (not applicable)

Testing

  • The ownership-boundary assertions are retained
  • New and existing unit tests pass locally
  • ASAN and TSAN validation is complete

Compatibility

  • Backward compatible
  • No public behavior change
  • CHANGELOG update (not applicable)

Performance

  • No production performance impact
  • Benchmarks (not applicable)

Screenshots / Diagrams

Not applicable.

Additional Notes

The original failure was caused by a test hook blocking whichever worker happened to run the child while an externally scheduled owner continuation could land in that same worker inbox. No production task_group behavior is changed.

Reviewer Guidance

Areas requiring special attention:

  • Affinity belongs to the inner owner coroutine, not merely the go_joinable_to callable wrapper.
  • The owner and child placement assertions match the workers required by the pause scenario.
  • The completion-pause guard releases the test hook on every scope exit.

Copilot AI review requested due to automatic review settings July 21, 2026 23:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Coldwings
Coldwings force-pushed the fix/ci-task-group-test-placement branch from d34920d to 8be290e Compare July 21, 2026 23:14
@Coldwings
Coldwings force-pushed the fix/ci-task-group-test-placement branch from 8be290e to 001f516 Compare July 21, 2026 23:21
@Coldwings

Copy link
Copy Markdown
Owner Author

Independent static review completed for exact head 001f5168086fc05c4c40698cb9693ec599f72cc5 against the PR base: approved with no actionable findings. The review specifically verified that affinity is now set inside the actual owner coroutine, so owner wakeups remain on worker 0 while the gated child completion pauses worker 1. It also checked hook cleanup, atomic synchronization, capture lifetimes, exception propagation, owner destruction, and shutdown ordering. No local compilation was performed; the exact-head GitHub Actions matrix remains the executable validation gate.

@Coldwings
Coldwings merged commit 4ae105b into main Jul 21, 2026
10 checks passed
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.

[CI] task_group completion-order regression flakes on paused worker placement

2 participants