Skip to content

fix(http1): synchronize dispatch shutdown with sends - #4150

Open
fzlzjerry wants to merge 1 commit into
hyperium:masterfrom
fzlzjerry:fix/h1-dispatch-race
Open

fix(http1): synchronize dispatch shutdown with sends#4150
fzlzjerry wants to merge 1 commit into
hyperium:masterfrom
fzlzjerry:fix/h1-dispatch-race

Conversation

@fzlzjerry

@fzlzjerry fzlzjerry commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

Fixes #4122.

Summary

  • close the HTTP/1 dispatch receiver before teardown
  • continue polling the closed receiver until a concurrent synchronous send has either published its envelope or observed the closure
  • add a regression test that exercises the close/send race 10,000 times

Background

Tokio's unbounded channel reserves message capacity before it publishes the
envelope into the channel list. Receiver shutdown could close the channel and
observe a pending receive during that window. The send would then finish after
shutdown, leaving the request owned by the channel until the remaining
SendRequest was dropped.

After closing the receiver, Pending means that such a pre-close send is still
in flight. Retrying the existing single-poll receive until it yields the
envelope or reports the closed channel removes that window without introducing
an additional synchronization object. The HTTP/2 path remains unchanged.

Validation

  • cargo test --features full (309 passed, 10 ignored)
  • cargo +1.97.1 clippy --features full -- -D warnings
  • cargo fmt --all -- --check
  • cargo check --no-default-features --features client,http1
  • cargo check --no-default-features --features client,http2
  • cargo check --no-default-features --features client,http1,http2
  • minimal dependency resolution followed by cargo check --features full

@fzlzjerry
fzlzjerry marked this pull request as ready for review August 11, 2026 08:57
Copilot AI lite review requested due to automatic review settings August 11, 2026 08:57

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.

Comment thread src/client/dispatch.rs Outdated
@fzlzjerry
fzlzjerry force-pushed the fix/h1-dispatch-race branch from d5bcecb to 42dfc55 Compare August 21, 2026 15:08
Drain a closed HTTP/1 dispatch channel until any concurrent synchronous send has either published or observed closure, so the queued request cannot remain owned by the sender.

Closes hyperium#4122
@fzlzjerry
fzlzjerry force-pushed the fix/h1-dispatch-race branch from 42dfc55 to 333d33a Compare August 21, 2026 15:16
@fzlzjerry

Copy link
Copy Markdown
Contributor Author

Updated the implementation in 333d33a to remove the added Arc<Mutex>. Shutdown now closes the receiver and polls its existing receive future until a synchronous send that already reserved capacity either publishes its envelope or the channel terminates.

The new run passes the race regression, stable/beta/nightly tests, MSRV, minimal versions, Miri, features, docs, and the remaining checks. The sole red Linter job is Rust 1.98 reporting three new lints in untouched files (src/body/incoming.rs, src/client/conn/http1.rs, and src/client/conn/http2.rs); cargo +1.97.1 clippy --features full -- -D warnings passes locally. The PR diff remains limited to the dispatch shutdown path and its regression test.

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.

Body dropped too late when an h1 handshake hits an error

3 participants