Skip to content

fix(coro): propagate targeted spawn affinity through task chains#1007

Merged
Coldwings merged 3 commits into
mainfrom
fix/spawn-affinity-propagation
Jul 22, 2026
Merged

fix(coro): propagate targeted spawn affinity through task chains#1007
Coldwings merged 3 commits into
mainfrom
fix/spawn-affinity-propagation

Conversation

@Coldwings

@Coldwings Coldwings commented Jul 22, 2026

Copy link
Copy Markdown
Owner

Description

Fix scheduler::go_to() and scheduler::go_joinable_to() so their documented worker affinity remains continuous across the directly awaited Elio task chain that implements the spawned vthread.

Previously, targeted spawn set affinity only on the scheduler-owned callable wrapper. The task returned by the callable had a separate execution context and therefore resumed with NO_AFFINITY after its first non-I/O suspension. Subsequent I/O could then bind to a different worker's io_context.

Direct Elio task awaits now inherit caller-requested affinity when the child has none. When the child finishes, its final caller-requested affinity is copied back to its Elio awaiter, so an explicit clear_affinity() or set_affinity() in a deeper frame remains authoritative for the rest of the same logical vthread. I/O ownership remains operation-local and continues to take precedence through effective_affinity().

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Performance improvement (optimization that improves speed/memory usage)
  • Documentation (changes to documentation, comments, or examples)
  • Refactoring (code changes that neither fix bugs nor add features)
  • Tests (adding or modifying tests)
  • Build/CI (changes to build system, CI configuration, or dependencies)

Related Issues

Closes #1006
Related to #1003

Changes Made

Core Changes

  • Propagate user affinity from an Elio awaiter into a directly awaited task<T> or task<void> when the child has no affinity.
  • Propagate the child's final user affinity back to the awaiter before returning a value or rethrowing an exception.
  • Preserve the existing symmetric-transfer and frame-ownership model; the fix does not enqueue borrowed child handles.
  • Remove the scheduler-wrapper-only propagation helper because task-level propagation now covers the complete directly awaited chain.
  • Add deterministic regressions for go_to() and go_joinable_to() through task_scope(), a value-returning channel::recv() task, a cross-worker wake, and subsequent real socket I/O.
  • Verify inherited affinity, clear_affinity(), and set_affinity(1, false) behavior.

API Changes (if applicable)

None. This restores the existing documented vthread and targeted-spawn behavior.

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

The regression waits on a bounded channel and uses the channel's waiter-publication test hook. It then runs a marker on worker 0 before waking from worker 1, proving that the original await_suspend() has returned before cross-worker resumption. One normal scheduler insertion is deliberately rejected so the test distinguishes affinity routing from local fallback. After the nested task chain returns, a cancellable socket receive verifies the owning io_context, completion worker, and post-I/O user affinity.

Integration Tests

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

Sanitizer Testing

  • Tested with ASAN (AddressSanitizer)
  • Tested with TSAN (ThreadSanitizer)
  • No new warnings or errors

Test Results

Local compilation and tests were not run; executable validation is delegated to the GitHub Actions matrix.
Static validation: git diff --check passed.
Independent read-only review: no unresolved actionable findings on the pushed design.
GitHub Actions at exact head 3514da6: all 10 checks passed, including x64/arm64 Debug sanitizer jobs.

Checklist

Code Quality

  • My code follows the project's code style
  • I have added/updated comments for complex logic where needed
  • I have removed any debug code, TODOs, or commented-out code
  • My changes generate no new warnings

Documentation

  • I have updated documentation (not needed; the public contract is unchanged)
  • I have added examples for new features (not applicable)
  • I have updated API documentation (not applicable)

Testing

  • I have added tests that prove my fix is effective or my feature works
  • New and existing unit tests pass locally with my changes
  • I have tested with ASAN and TSAN

Compatibility

  • My changes are backward compatible
  • I have considered the impact on existing users
  • I have updated CHANGELOG.md (not applicable; no API change)

Performance (if applicable)

  • I have considered the performance impact
  • I have added benchmarks for performance-critical changes (not applicable)

Screenshots / Diagrams

Not applicable.

Additional Notes

The fix intentionally does not alias execution contexts across coroutine frames. Cancellation continues to use the existing one-way direct-await link, while operation-local I/O pins remain owned by the frame and awaitable that registered the operation. Only caller-requested affinity is carried across the logical vthread chain.

The first regression implementation used a custom raw-handle gate. TSAN correctly identified that the test could resume and destroy the frame while its await_suspend() was still accessing the gate. The final test uses the channel publication hook plus a same-worker marker to establish the required happens-before ordering.

Reviewer Guidance

Areas requiring special attention:

  • Whether direct Elio task awaits are the correct boundary for vthread affinity continuity.
  • Whether downward inheritance and upward propagation preserve explicit clear_affinity() / set_affinity() behavior without copying I/O ownership.
  • Whether the bounded-channel publication and marker sequence excludes concurrent destruction of an active await_suspend() frame.
  • Whether every test failure path shuts down the scheduler before borrowed channel, observation, cancellation, and socket state is destroyed.

Questions for reviewers:

  • Are there any direct Elio task-await boundaries that should intentionally terminate user-affinity propagation, analogous to the existing foreign-coroutine cancellation boundary?

Copilot AI review requested due to automatic review settings July 22, 2026 02:37

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 changed the title fix(runtime): propagate targeted spawn affinity fix(coro): propagate targeted spawn affinity through task chains Jul 22, 2026
@Coldwings

Copy link
Copy Markdown
Owner Author

Independent read-only review completed against exact head 3514da6.

Review scope covered direct-Elio-await affinity propagation, symmetric-transfer/frame ownership, cancellation and exception paths, explicit affinity overrides, I/O-pin precedence, scheduler shutdown, and the sanitizer safety of the regression. No unresolved actionable findings remain.

Two issues found during earlier review iterations were corrected before this head: the fix no longer asynchronously queues a borrowed directly-awaited child handle, and the regression now uses bounded-channel waiter publication plus a same-worker marker instead of a raw-handle gate. The final exact-head review found no contract, lifetime, sanitizer, compile, hang, teardown, or portability blockers. GitHub Actions Debug sanitizer jobs are still running.

@Coldwings
Coldwings merged commit e54343c into main Jul 22, 2026
10 checks passed
@Coldwings
Coldwings deleted the fix/spawn-affinity-propagation branch July 22, 2026 04:14
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.

[Bug] go_to and go_joinable_to lose worker affinity after task suspension

2 participants