Skip to content

test(http): synchronize client cancellation regressions#1004

Merged
Coldwings merged 1 commit into
mainfrom
fix/ci-client-cancel-test-ordering
Jul 21, 2026
Merged

test(http): synchronize client cancellation regressions#1004
Coldwings merged 1 commit into
mainfrom
fix/ci-client-cancel-test-ordering

Conversation

@Coldwings

@Coldwings Coldwings commented Jul 21, 2026

Copy link
Copy Markdown
Owner

Description

Make the HTTP, WebSocket, and SSE connect-cancellation regressions wait until the client has successfully staged its cancellable response recv before cancellation. This removes the fixed 100 ms setup race that could cancel before TCP connection and strand fake-server I/O under ASAN.

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 #1002

Changes Made

Core Changes

  • Add a test-build-only one-shot observer that is armed by the HTTP, WebSocket, or SSE response path and published only after cancellable recv preparation succeeds.
  • Drive cancellation from the test thread after observing the staged recv, so the regression exercises backend cancellation rather than an earlier token precheck.
  • Give each fake server an independent cancellation source covering accept, header drain, and its intentionally stalled read.
  • Verify each fake server received a complete request header block and finished before scheduler shutdown.
  • Remove the fixed-delay and event-waiter cancellation tasks while preserving errno, latency, and graceful-shutdown assertions.

API Changes (if applicable)

None. The synchronization hook exists only when ELIO_RUNTIME_TEST_HOOKS is enabled.

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
  • I documented the test-only staged-I/O synchronization contract
  • I removed obsolete event waiters and timers
  • Static review found no unrelated production behavior change

Documentation

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

Testing

  • The tests prove cancellable response recv staging
  • Failure paths independently cancel and drain fake-server I/O
  • New and existing unit tests pass locally
  • ASAN and TSAN validation is complete

Compatibility

  • Backward compatible
  • No behavior or ABI change outside test-hook builds
  • CHANGELOG update (not applicable)

Performance

  • No runtime cost outside test-hook builds
  • Benchmarks (not applicable)

Screenshots / Diagrams

Not applicable.

Additional Notes

The LeakSanitizer report in the failing run was secondary fallout from forced scheduler shutdown with fake-server I/O still pending. This patch does not suppress leaks or weaken shutdown assertions.

Reviewer Guidance

Areas requiring special attention:

  • The one-shot observer is published only after prepare succeeds for the intended cancellable recv.
  • Server accept and reads share an independent cleanup token and publish bounded completion.
  • HTTP, WebSocket, and SSE tests pin server/client work to separate workers so the thread-local hook cannot be consumed by server I/O.

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-client-cancel-test-ordering branch 2 times, most recently from fdb5040 to 29ae3d0 Compare July 21, 2026 23:27
@Coldwings
Coldwings force-pushed the fix/ci-client-cancel-test-ordering branch from 29ae3d0 to 4650478 Compare July 21, 2026 23:29
@Coldwings

Copy link
Copy Markdown
Owner Author

Independent static review completed for exact head 4650478a590b956510a15e2fe108a9234fa47f53 against the PR base: approved with no actionable findings. The review verified that the one-shot observer is published only after cancellable recv preparation succeeds, client/server affinity prevents the server recv from consuming the client hook, and independent server cancellation covers accept, header drain, and the stalled read before shutdown. It also confirmed the direct affinity-header dependency and cleanup/capture lifetimes. No local compilation was performed; the exact-head GitHub Actions matrix remains the executable validation gate.

@Coldwings
Coldwings merged commit b43ca92 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] fixed-delay client cancellation tests can strand fake-server I/O

2 participants